Skip to content

Conversation

cloehle
Copy link
Contributor

@cloehle cloehle commented Aug 11, 2025

scx_bpf_cpu_rq() may return NULL so check for it.
Additionally also add a NULL check for ->curr.

scx_bpf_cpu_rq() may return NULL so check for it.
Additionally also add a NULL check for ->curr.

Fixes: a937a24 ("scx_cosmos: Introduce flat idle CPU scan")
Reported-by: Jake Hillion <[email protected]>
Closes: https://lore.kernel.org/lkml/y23etey3foin5nrxgj6e4g373b3ap6oxqa5rrvuvwyus3umw5s@bgh3d6uuga5t/
Signed-off-by: Christian Loehle <[email protected]>
@cloehle cloehle force-pushed the cloehle/scx_cosmos-null-idle-check branch from 1559a4b to c0d1e10 Compare August 11, 2025 15:04
Copy link
Contributor

@JakeHillion JakeHillion left a comment

Choose a reason for hiding this comment

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

This will unblock #2584 to update the sched_ext/for-next kernel, and this does return NULL sometimes so should be checked. As mentioned in https://lore.kernel.org/sched-ext/aJIWHk_4IuOxyh5d@gpd4/T/#m8e663f569b713377557976be11c4ce20ff88be07 though, I think we should implement these changes in a way that doesn't break existing scheduler binaries for a couple of kernel versions.

@JakeHillion JakeHillion requested a review from arighi August 11, 2025 15:54
@JakeHillion
Copy link
Contributor

@cloehle does this change still make sense with the differences in the kernel patches? Happy to merge if so.

@arighi
Copy link
Contributor

arighi commented Aug 12, 2025

Hm.. this would still break the logic in the scheduler, because it would consider CPUs as busy (in the NULL case) while they're not.

What we should do instead is introduce a compat helper to check if the new API to get the curr task is available and rely on that.

And if we get NULL I'd rather error the scheduler or drop this logic completely.

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