Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Meaningful code and commit message mismatch: In sub-schedulers, scx_sched_sysfs_add() is called before ops.init(), contradicting the root scheduler's behavior and the commit message's goal.
--
--- Patch [14]: [PATCH v4 sched_ext/for-7.3 14/40] sched_ext: Defer scx_sched kobj sysfs add into the enable workfns ---
commit 76fa4bf5abb621f0acd7524a77691d28f4fdf9b7
Author: Tejun Heo <[email protected]>
sched_ext: Defer scx_sched kobj sysfs add into the enable workfns
This commit splits kobject initialization and addition for schedulers, moving
the sysfs addition to the enable work functions. This ensures the scheduler
is fully built before becoming visible in sysfs.
> Prep so a future caps attribute can rely on @sch being fully built by
> the time it's sysfs-visible.
Does the implementation in scx_sub_enable_workfn() match this description?
The commit message states that sysfs addition happens once the scheduler is
fully built and its sysfs-visible state is initialized.
> diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
> index ce76ae141e0a9..9855a9a4e7095 100644
> --- a/kernel/sched/ext/sub.c
> +++ b/kernel/sched/ext/sub.c
[ ... ]
> @@ -363,6 +365,10 @@ void scx_sub_enable_workfn(struct kthread_work *work)
> if (ret)
> goto err_disable;
>
> + ret = scx_sched_sysfs_add(sch);
> + if (ret)
> + goto err_disable;
> +
> if (sch->level >= SCX_SUB_MAX_DEPTH) {
> scx_error(sch, "max nesting depth %d violated",
> SCX_SUB_MAX_DEPTH);
[Severity: Medium]
Is it intentional that scx_sched_sysfs_add() is called here in
scx_sub_enable_workfn(), before ops.init() is executed?
In scx_root_enable_workfn(), the sysfs addition is deferred until after
ops.init() runs and SCX_EFLAG_INITIALIZED is set. However, in this
sub-scheduler path, it is added to sysfs earlier, before ops.init() is
invoked.
Will this premature exposure cause issues for future caps attributes that
expect sub-schedulers to be fully initialized when visible in sysfs?
--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=14
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.