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

Strip compat support #363

Merged
merged 14 commits into from
Jun 17, 2024
Merged

Strip compat support #363

merged 14 commits into from
Jun 17, 2024

Conversation

htejun
Copy link
Contributor

@htejun htejun commented Jun 16, 2024

In preparation of upstreaming, set the currently released v6.9 kernels as the minimum requirement and strip the compat features. The only remaining one is for dsq iters. This needs updates from BPF side and will be handled later.

htejun added 14 commits June 15, 2024 18:27
…tion

scx_ops_open!() and scx_ops_attach!() could return the calling function
after an error, which can be surprising. Forutnately, as all the current
callers are either unwrapping or returning on error, the surprising behavior
is currently not very noticeable.

Fix it by breaking out of the macro block on errors.
In preparation of upstreaming, let's set the min version requirement at the
released v6.9 kernels. Drop __COMPAT_scx_bpf_switch_call(). The open helper
macros now check the existence of SCX_OPS_SWITCH_PARTIAL and abort if not.
There's no guarantee that errno is set or contains relevant information when
SCX_BUG() is invoked. This sometimes leads to "task failed successfully"
messages:

  # ./scx_simple
  ../scheds/c/scx_simple.c:72 [scx panic]: Success
  SCX_OPS_SWITCH_PARTIAL missing, kernel too old?

While not critical, it's not great. Let's update it so that errno is printed
in parentheses when non-zero and match the tag to the macro name so that
what's printed is the following:

  # ./scx_simple
  [SCX_BUG] ../scheds/c/scx_simple.c:72
  SCX_OPS_SWITCH_PARTIAL missing, kernel too old?
In preparation of upstreaming, let's set the min version requirement at the
released v6.9 kernels. Drop __COMPAT_SCX_KICK_IDLE. The open helper macros
now check the existence of SCX_KICK_IDLE and abort if not.
In preparation of upstreaming, let's set the min version requirement at the
released v6.9 kernels. Drop __COMPAT_scx_bpf_exit(). The open helper macros
now check the existence of scx_bpf_exit_bstr() and abort if not.
In preparation of upstreaming, let's set the min version requirement at the
released v6.9 kernels. Drop __COMPAT_scx_bpf_dump(). The open helper macros
now check the existence of scx_bpf_dump_bstr() and abort if not.

While at it, reorder the min requirement checks so that newly added ones are
up top to make testing easier.
In preparation of upstreaming, let's set the min version requirement at the
released v6.9 kernels. Drop __COMPAT_HAS_CPUMASKS(). The open helper macros
now check the existence of scx_bpf_nr_cpu_ids() and abort if not.
In preparation of upstreaming, let's set the min version requirement at the
released v6.9 kernels. Drop __COMPAT_scx_bpf_cpuperf_*(). The open helper
macros now check the existence of scx_bpf_cpuperf_cap() and abort if not.
In preparation of upstreaming, let's set the min version requirement at the
released v6.9 kernels. Drop support for missing sched_ext_ops.hotplug_seq.
The open helper macros now check the existence of the field and abort if
missing.
In preparation of upstreaming, let's set the min version requirement at the
released v6.9 kernels. Drop support for missing sched_ext_ops.exit_dump_len.
The open helper macros now check the existence of the field and abort if
missing.
In preparation of upstreaming, let's set the min version requirement at the
released v6.9 kernels. Drop support for missing sched_ext_ops.tick(). The
open helper macros now check the existence of the field and abort if
missing.
In preparation of upstreaming, let's set the min version requirement at the
released v6.9 kernels. Drop support for missing sched_ext_ops.dump*(). The
open helper macros now check the existence of the fields and abort if
missing.
Let's check only the latest one.
Copy link
Contributor

@arighi arighi left a comment

Choose a reason for hiding this comment

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

LGTM, left few minor comments that can be also addressed later.

Moreover, we should also update the sched_ext-ci branch, or CI will fail.

@@ -246,7 +246,6 @@ impl<'cb> BpfScheduler<'cb> {

skel.bss_mut().usersched_pid = std::process::id();
skel.rodata_mut().slice_ns = slice_us * 1000;
skel.rodata_mut().switch_partial = partial;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also drop partial from BpfScheduler, update scx_rustland and scx_rlfifo, and drop the option --partial from scx_rustland), but it can be done later in a separate commit (this is just as a reminder).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

--partial still sets SCX_OPS_SWITCH_PARTIAL in sched_ext_ops.flags, so it should keep working the same.

@@ -314,7 +314,6 @@ impl<'a> Scheduler<'a> {
skel.rodata_mut().load_half_life = (opts.load_half_life * 1000000000.0) as u32;
skel.rodata_mut().kthreads_local = opts.kthreads_local;
skel.rodata_mut().fifo_sched = opts.fifo_sched;
skel.rodata_mut().switch_partial = opts.partial;
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto for scx_rusty about --partial.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ditto.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I was missing this part, that's right. So everything looks good, thanks!

Copy link
Contributor

@multics69 multics69 left a comment

Choose a reason for hiding this comment

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

Thanks @htejun . LGTM

@htejun htejun merged commit d18bb48 into main Jun 17, 2024
1 check failed
@htejun htejun deleted the htejun/compat-strip branch June 17, 2024 06:04
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