Re: [PATCH sched_ext/for-7.1-fixes] sched_ext: Defer sub_kset base put to scx_sched_free_rcu_work

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Relying on sysfs_create_dir() failure triggers a user-triggerable kernel log spam (pr_err).
- [Medium] Inconsistent error handling paths for the same race condition.
--

commit e9f250877d2f7deea4cca723a738f6925005797e
Author: Tejun Heo <tj@kernel.org>

sched_ext: Defer sub_kset base put to scx_sched_free_rcu_work

This commit splits the sub_kset teardown by moving kobject_put() to
scx_sched_free_rcu_work() while keeping kobject_del() in the disable
path. This ensures the memory remains valid during concurrent disable
and enable operations, relying on sysfs creation failure to catch races.

> A racing child sees state_in_sysfs=0 with valid memory, sysfs_create_dir() fails, and
> the existing exit_kind gate in scx_link_sched() turns it away with -ENOENT.

Does this accurately describe the error path? If sysfs_create_dir() fails,
wouldn't scx_alloc_and_add_sched() return ERR_PTR(-ENOENT) and jump directly
to out_unlock in scx_sub_enable_workfn(), completely bypassing
scx_link_sched()?

Also, does this create an inconsistent API contract for userspace? If the
sysfs creation fails, userspace receives a synchronous -ENOENT. However, if
sysfs creation succeeds but the race is caught later by the exit_kind gate in
scx_link_sched(), the error path jumps to err_disable which sets cmd->ret = 0,
resulting in an asynchronous failure instead.

> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
[ ... ]
> @@ -5801,7 +5803,7 @@ static void scx_sub_disable(struct scx_sched *sch)
>  	if (sch->ops.exit)
>  		SCX_CALL_OP(sch, exit, NULL, sch->exit_info);
>  	if (sch->sub_kset)
> -		kset_unregister(sch->sub_kset);
> +		kobject_del(&sch->sub_kset->kobj);
>  	kobject_del(&sch->kobj);
>  }

Will relying on sysfs failure here cause user-triggerable kernel log spam?

When the parent's sysfs directory is removed via kobject_del(), its ->sd
pointer is set to NULL. When the racing child subsequently calls
kobject_init_and_add() and sysfs_create_dir_ns() fails with -ENOENT,
kobject_add_internal() will unconditionally print a pr_err() about the failure.

Since this is an expected race condition, is it safe to handle it by letting
sysfs fail and print an error to dmesg?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/2f2b7481ef217ad89c5d90fd56fc0c01@kernel.org?part=1