Re: [PATCH v3 sched_ext/for-7.3 4/4] sched_ext: Split sub-scheduler implementation into sub.c

[Tejun Heo <tj@kernel.org>]

Hello.

> The sub-scheduler is removed from the scx_sched_hash via scx_unlink_sched()
> after the RCU grace period completes. Because it remains in the hash during
> and after the grace period, couldn't a concurrent BPF program on the parent
> scheduler call scx_bpf_sub_dispatch() and find the sub-scheduler?

It can, but that's fine. The expedited synchronize is there to drain
in-flight ops after all tasks have been moved off, not to make the sched
unreachable. The sched is freed through another RCU grace period after
unlinking and scx_bpf_sub_dispatch() runs under RCU read lock, so the
racing lookup can't see the sched freed under it. Such a dispatch is also
inert as the child is already bypassed and has no tasks left.

> If a BPF program attaches a sub-scheduler to a newly created cgroup that
> hasn't completed its ONLINE phase yet, cgrp->scx_sched might be NULL. The
> code dereferences it immediately.

The cgroup can only come from cgroup_get_from_id() which fails on kernfs
nodes which haven't been activated, and cgroup_mkdir() activates the node
after CGROUP_LIFETIME_ONLINE has fired and ->scx_sched has been inherited
from the parent. The inherited pointer is non-NULL whenever the attach can
get this far. The root enable path sets ->scx_sched on all live cgroups
before __scx_enabled is turned on, under the same scx_enable_mutex that
scx_sub_enable_workfn() holds across the scx_enabled() check. The
CSS_ONLINE check afterwards guards against destruction, not incomplete
onlining.

> This is a pre-existing issue, but do we need to call scx_error() in these
> failure paths?

Will be dealt with separately.

Thanks.

-- 
tejun