Reset the inherited cpu affinity - #1891
Merged
Merged
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR updates libcrun_reset_cpu_affinity_mask to explicitly build and apply a full CPU affinity mask instead of relying on the kernel’s default behavior. Class diagram for updated libcrun_reset_cpu_affinity_mask functionclassDiagram
class libcrun_reset_cpu_affinity_mask {
+int libcrun_reset_cpu_affinity_mask(pid_t pid, libcrun_error_t *err)
-int ret
-cpu_set_t mask
-long cpu_num
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/libcrun/scheduler.c:85-88` </location>
<code_context>
libcrun_reset_cpu_affinity_mask (pid_t pid, libcrun_error_t *err)
{
int ret;
+ cpu_set_t mask;
+
+ long cpu_num = sysconf(_SC_NPROCESSORS_CONF);
+ for (int i = 0; i < cpu_num; i++) {
+ CPU_SET(i, &mask);
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** mask is not initialized before use, which may lead to undefined behavior.
Initialize mask with CPU_ZERO(&mask) before using CPU_SET to avoid unintended affinity settings.
</issue_to_address>
### Comment 2
<location> `src/libcrun/scheduler.c:87` </location>
<code_context>
int ret;
+ cpu_set_t mask;
+
+ long cpu_num = sysconf(_SC_NPROCESSORS_CONF);
+ for (int i = 0; i < cpu_num; i++) {
+ CPU_SET(i, &mask);
</code_context>
<issue_to_address>
**issue:** sysconf(_SC_NPROCESSORS_CONF) can return -1 on error, which is not handled.
Check if cpu_num is -1 before using it in the loop to prevent undefined behavior.
</issue_to_address>
### Comment 3
<location> `src/libcrun/scheduler.c:104` </location>
<code_context>
See: https://issues.redhat.com/browse/OCPBUGS-15102 */
- ret = sched_setaffinity (pid, 0, NULL);
+ ret = sched_setaffinity (pid, sizeof(mask), &mask);
if (LIKELY (ret < 0))
{
if (LIKELY (errno == EINVAL || errno == ENODEV || errno == ENOSYS))
</code_context>
<issue_to_address>
**nitpick:** The logic for error checking on sched_setaffinity return value appears inverted.
The use of LIKELY with ret < 0 may reduce clarity, as errors are less likely. Please clarify or remove the macro to improve readability.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
ningmingxiao
force-pushed
the
dev_01
branch
2 times, most recently
from
October 9, 2025 07:12
13f5096 to
4f5916f
Compare
Member
|
@haircommander PTAL |
ningmingxiao
force-pushed
the
dev_01
branch
2 times, most recently
from
October 9, 2025 11:34
448282f to
0e76f72
Compare
kolyshkin
reviewed
Oct 10, 2025
Contributor
Author
|
can this pr be merged @giuseppe |
kolyshkin
reviewed
Oct 15, 2025
kolyshkin
requested changes
Oct 15, 2025
kolyshkin
left a comment
Collaborator
There was a problem hiding this comment.
just a nit about uneeded CPU_ZERO
Signed-off-by: ningmingxiao <ning.mingxiao@zte.com.cn>
Contributor
Author
done thanks. |
kolyshkin
approved these changes
Oct 16, 2025
kolyshkin
left a comment
Collaborator
There was a problem hiding this comment.
LGTM
@haircommander PTAL
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
since this patch https://lore.kernel.org/lkml/20231003205735.2921964-1-longman@redhat.com/ doesn't merge into kernel.
so it doesn't work on new kernel. @giuseppe