Sched_ext development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Tejun Heo" <tj@kernel.org>
Cc: sched-ext@lists.linux.dev
Subject: Re: [PATCH v4 sched_ext/for-7.3 14/40] sched_ext: Defer scx_sched kobj sysfs add into the enable workfns
Date: Wed, 08 Jul 2026 21:47:19 +0000	[thread overview]
Message-ID: <20260708214720.193391F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260708212429.3405787-15-tj@kernel.org>

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 <tj@kernel.org>

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/20260708212429.3405787-1-tj@kernel.org?part=14

  reply	other threads:[~2026-07-08 21:47 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-08 21:23 [PATCHSET v4 sched_ext/for-7.3] sched_ext: Capability-based CPU delegation for sub-schedulers Tejun Heo
2026-07-08 21:23 ` [PATCH v4 sched_ext/for-7.3 01/40] sched_ext: Record an error on errno-only sub-enable failure Tejun Heo
2026-07-09 20:37   ` Andrea Righi
2026-07-08 21:23 ` [PATCH v4 sched_ext/for-7.3 02/40] sched_ext: Fix premature ops->priv publication in scx_alloc_and_add_sched() Tejun Heo
2026-07-09 20:38   ` Andrea Righi
2026-07-08 21:23 ` [PATCH v4 sched_ext/for-7.3 03/40] tools/sched_ext: scx - Fix cmask_subset(), cmask_equal() and cmask_weight() Tejun Heo
2026-07-09 20:39   ` Andrea Righi
2026-07-08 21:23 ` [PATCH v4 sched_ext/for-7.3 04/40] sched_ext: Use READ_ONCE/WRITE_ONCE in cmask word ops and drop _RACY variants Tejun Heo
2026-07-09 20:39   ` Andrea Righi
2026-07-08 21:23 ` [PATCH v4 sched_ext/for-7.3 05/40] sched_ext: Add SCX_CALL_CID_OP_TASK() for cid-form op dispatch Tejun Heo
2026-07-09 20:41   ` Andrea Righi
2026-07-08 21:23 ` [PATCH v4 sched_ext/for-7.3 06/40] sched_ext: Rename extra_enq_flags to remote_activate_enq_flags Tejun Heo
2026-07-09 20:41   ` Andrea Righi
2026-07-08 21:23 ` [PATCH v4 sched_ext/for-7.3 07/40] tools/sched_ext: scx_qmap - Use bare u64/u32/s32 integer types Tejun Heo
2026-07-09 20:42   ` Andrea Righi
2026-07-08 21:23 ` [PATCH v4 sched_ext/for-7.3 08/40] sched_ext: Reject direct slice and dsq_vtime writes for cid-form schedulers Tejun Heo
2026-07-08 21:41   ` sashiko-bot
2026-07-09 22:28     ` Tejun Heo
2026-07-09 20:56   ` Andrea Righi
2026-07-08 21:23 ` [PATCH v4 sched_ext/for-7.3 09/40] sched_ext: Make scx_bpf_kick_cid() return void Tejun Heo
2026-07-09 20:57   ` Andrea Righi
2026-07-08 21:23 ` [PATCH v4 sched_ext/for-7.3 10/40] sched_ext: Make the kick machinery per-sched Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 11/40] sched_ext: Add ops.init_cids() to finalize the cid layout before init Tejun Heo
2026-07-08 21:54   ` sashiko-bot
2026-07-09 22:28     ` Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 12/40] sched_ext: Add CID sharding Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 13/40] sched_ext: Add shard boundaries to scx_bpf_cid_override() Tejun Heo
2026-07-08 21:47   ` sashiko-bot
2026-07-09 22:28     ` Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 14/40] sched_ext: Defer scx_sched kobj sysfs add into the enable workfns Tejun Heo
2026-07-08 21:47   ` sashiko-bot [this message]
2026-07-09 22:28     ` Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 15/40] sched_ext: Add per-shard scx_sched storage scaffolding Tejun Heo
2026-07-08 21:44   ` sashiko-bot
2026-07-09 22:28     ` Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 16/40] sched_ext: Add scx_cmask_ref for validated arena cmask access Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 17/40] sched_ext: Build the set_cmask scratch from trusted geometry Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 18/40] sched_ext: RCU-protect the sub-sched tree's children/sibling lists Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 19/40] sched_ext: Add scx_skip_subtree_pre() Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 20/40] sched_ext: Stop resolving a disabled scheduler's programs Tejun Heo
2026-07-08 21:55   ` sashiko-bot
2026-07-09 22:28     ` Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 21/40] sched_ext: Add per-shard cap delegation for sub-schedulers Tejun Heo
2026-07-08 22:00   ` sashiko-bot
2026-07-09 22:28     ` Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 22/40] sched_ext: Add coalescing sub_caps_updated() notifier " Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 23/40] sched_ext: Maintain per-cpu effective cap copies for single-read checks Tejun Heo
2026-07-08 22:03   ` sashiko-bot
2026-07-09 22:28     ` Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 24/40] sched_ext: Add sub_ecaps_updated() effective-cap change notifier Tejun Heo
2026-07-08 22:02   ` sashiko-bot
2026-07-09 22:28     ` Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 25/40] sched_ext: Generalize local-DSQ handling to rq-owned DSQs Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 26/40] sched_ext: Add reject DSQ for cap-rejected dispatches Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 27/40] sched_ext: Add the SCX_CAP_ENQ_IMMED cap Tejun Heo
2026-07-08 21:56   ` sashiko-bot
2026-07-09 22:28     ` Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 28/40] sched_ext: Assign a unique id to each scheduler instance Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 29/40] sched_ext: Route task slice writes through set_task_slice() Tejun Heo
2026-07-08 21:53   ` sashiko-bot
2026-07-09 22:28     ` Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 30/40] sched_ext: Track the cpu a task is runnable on Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 31/40] sched_ext: Tie cpu occupancy to SCX_CAP_BASE through the task slice Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 32/40] sched_ext: Add the SCX_CAP_ENQ cap Tejun Heo
2026-07-08 22:00   ` sashiko-bot
2026-07-09 22:28     ` Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 33/40] sched_ext: Gate kicks on SCX_CAP_BASE and preemption on SCX_CAP_PREEMPT Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 34/40] sched_ext: Authorize remote-move inserts against the placing scheduler Tejun Heo
2026-07-08 22:04   ` sashiko-bot
2026-07-09 22:28     ` Tejun Heo
2026-07-09 20:09   ` Andrea Righi
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 35/40] sched_ext: Route ops.update_idle() to sub-schedulers and re-notify owed scheds Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 36/40] sched_ext: Replay ecaps notifications suppressed by bypass Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 37/40] sched_ext: Add scx_bpf_sub_kill() to evict a child sub-scheduler Tejun Heo
2026-07-08 22:11   ` sashiko-bot
2026-07-09 22:28     ` Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 38/40] tools/sched_ext: Add three-mask cmask intersection iterator Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 39/40] tools/sched_ext: scx_qmap - Expand hierarchical sub-scheduling Tejun Heo
2026-07-08 22:03   ` sashiko-bot
2026-07-09 22:28     ` Tejun Heo
2026-07-08 21:24 ` [PATCH v4 sched_ext/for-7.3 40/40] tools/sched_ext: scx_qmap - Add sub-sched cap fault injection Tejun Heo
2026-07-09 21:18 ` [PATCHSET v4 sched_ext/for-7.3] sched_ext: Capability-based CPU delegation for sub-schedulers Tejun Heo
2026-07-09 22:13 ` Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260708214720.193391F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=sched-ext@lists.linux.dev \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox