Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scx_bpfland: small improvements #429

Merged
merged 6 commits into from
Jul 15, 2024
Merged

scx_bpfland: small improvements #429

merged 6 commits into from
Jul 15, 2024

Conversation

arighi
Copy link
Contributor

@arighi arighi commented Jul 14, 2024

Small improvements for the CPU hotplug support, time slice evaluation and in the logic to classify interactive/regular tasks. No major improvement in this PR, mostly small fixes/adjustments to enhance the quality of the code.

Always assign the maximum time slice if there are idle CPUs in the
system.

Otherwise, double the task's unused time slice to reward tasks that use
less CPU time and at the same time refill the time slice of the tasks
every time they're dispatched.

Signed-off-by: Andrea Righi <righi.andrea@gmail.com>
Refine the safeguard mechanism to avoid generating too many interactive
tasks in the system, which could nullify the effect of the
interactive/regular task classification.

The safeguard mechanism operates by pausing the promotion of new tasks
to interactive status during the task wake-up process, whenever the
number of interactive tasks in the priority queue exceeds a specific
limit (set to 4x the number of online CPUs).

Halting the promotion of additional interactive tasks allows to
prioritize those already classified as interactive, thereby preventing
potential "bursts" of excessive interactive tasks in the system.

This refines the mitigation already provided by commit 640bd56
("scx_bpfland: prevent tasks from abusing interactive priority boost").

Fixes: 640bd56 ("scx_bpfland: prevent tasks from abusing interactive priority boost")
Signed-off-by: Andrea Righi <righi.andrea@gmail.com>
Initialize the number of voluntary context switches metrics in the local
task storage.

Signed-off-by: Andrea Righi <righi.andrea@gmail.com>
Instead of constantly checking the need to drain tasks from the DSQs of
the offline CPUs, provide an atomic flag to notify when there are tasks
to be drained from the offline CPUs.

Signed-off-by: Andrea Righi <righi.andrea@gmail.com>
We can rely on scx_bpf_nr_cpu_ids() to create all the possible per-CPU
DSQs, eliminating the need for the hard-coded limit MAX_CPUS.

In this way scx_bpfland can support the same amount of CPUs that the
kernel can handle.

Signed-off-by: Andrea Righi <righi.andrea@gmail.com>
* valid.
*/
static u64 cpu_to_dsq(s32 cpu)
{
if (cpu < 0 || cpu >= MAX_CPUS) {
u64 cpu_max = scx_bpf_nr_cpu_ids();
Copy link
Contributor

Choose a reason for hiding this comment

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

Because this gets surprisingly confusing sometimes, I wonder whether it'd be beneficial if we all agree to use nr_cpu_ids consistently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good point, I'll change it to nr_cpu_ids, so it's more consistent with the rest of the code.

offline = offline_cpumask;
if (!offline)
return 0;
if (bpf_cpumask_test_cpu(cpu, cast_mask(offline))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm... this is a bit tricky. I think this can be a lot simpler if we had a way to wait a RCU grace period.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is quite ugly, also offline_cpumask is allocated once at init and it goes away only when the scheduler exits (it's never re-allocated), this is just to make the verifier happy...

We always use nr_cpu_ids to represent the maximum CPU id returned by
scx_bpf_nr_cpu_ids().

Replace cpu_max with nr_cpu_ids to be more consistent with the rest of
the code.

Signed-off-by: Andrea Righi <righi.andrea@gmail.com>
@arighi arighi merged commit e268c58 into main Jul 15, 2024
1 check passed
@arighi arighi deleted the bpfland-small-improvements branch July 15, 2024 18:08
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.

2 participants