Skip to content

Reset the inherited cpu affinity - #1891

Merged
giuseppe merged 1 commit into
containers:mainfrom
ningmingxiao:dev_01
Oct 16, 2025
Merged

Reset the inherited cpu affinity#1891
giuseppe merged 1 commit into
containers:mainfrom
ningmingxiao:dev_01

Conversation

@ningmingxiao

@ningmingxiao ningmingxiao commented Oct 9, 2025

Copy link
Copy Markdown
Contributor

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

@sourcery-ai

sourcery-ai Bot commented Oct 9, 2025

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This 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 function

classDiagram
    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
    }
Loading

File-Level Changes

Change Details Files
Explicitly set the CPU affinity mask to all available CPUs
  • Declare a cpu_set_t variable for the mask
  • Use sysconf(_SC_NPROCESSORS_CONF) to get the number of CPUs
  • Loop over CPU indices and add each to the mask
  • Call sched_setaffinity with sizeof(mask) and &mask instead of NULL
src/libcrun/scheduler.c

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/libcrun/scheduler.c Outdated
Comment thread src/libcrun/scheduler.c Outdated
Comment thread src/libcrun/scheduler.c Outdated
@ningmingxiao
ningmingxiao force-pushed the dev_01 branch 2 times, most recently from 13f5096 to 4f5916f Compare October 9, 2025 07:12
@giuseppe

giuseppe commented Oct 9, 2025

Copy link
Copy Markdown
Member

@haircommander PTAL

@ningmingxiao
ningmingxiao force-pushed the dev_01 branch 2 times, most recently from 448282f to 0e76f72 Compare October 9, 2025 11:34
Comment thread src/libcrun/scheduler.c Outdated
@ningmingxiao

Copy link
Copy Markdown
Contributor Author

can this pr be merged @giuseppe

@kolyshkin kolyshkin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

Comment thread src/libcrun/scheduler.c Outdated

@kolyshkin kolyshkin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a nit about uneeded CPU_ZERO

Signed-off-by: ningmingxiao <ning.mingxiao@zte.com.cn>
@ningmingxiao

ningmingxiao commented Oct 16, 2025

Copy link
Copy Markdown
Contributor Author

just a nit about uneeded CPU_ZERO

done thanks.

@kolyshkin kolyshkin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@haircommander PTAL

@giuseppe giuseppe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@giuseppe
giuseppe merged commit 64611d7 into containers:main Oct 16, 2025
48 checks passed
@ningmingxiao
ningmingxiao deleted the dev_01 branch October 16, 2025 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants