The tryget_task_struct() calls in scx_sub_disable(), scx_root_enable_workfn() and scx_sub_enable_workfn() can never fail at the points they're invoked: - scx_root_enable_workfn() iterates over scx_tasks under scx_tasks_lock and rq lock. sched_ext_dead() removes tasks from scx_tasks under the same scx_tasks_lock before put_task_struct_rcu_user() runs in finish_task_switch(). So any task observed in scx_tasks must have usage > 0; put_task_struct_rcu_user() hasn't been called and the delayed_put_task_struct() callback that decrements usage cannot have been queued. - scx_sub_disable() and scx_sub_enable_workfn() iterate via css_task_iter, which takes a reference on each task in css_task_iter_next() and holds it until the next iter_next() call, so usage > 0 is guaranteed by the iter itself. The actual filter for dead tasks is the SCX_TASK_DEAD check inside scx_task_iter_next_locked(), not tryget; tryget only fails on zero usage, a state that can't be reached for tasks visible to these iters. Commit b7d4b28db7da ("sched_ext: Use SCX_TASK_READY test instead of tryget_task_struct() during class switch") removed an analogous tryget in the class-switch loop. Convert the remaining tryget calls to plain get_task_struct() and update the comment in scx_root_enable_workfn() that suggested tasks could be observed with zero @usage waiting for an RCU grace period.
Unified · Split
| @@ -5933,14 +5933,11 @@ static void scx_sub_disable(struct scx_sched *sch) · 5932 unmodified lines | |||
| 5933 | WARN_ON_ONCE(!scx_task_on_sched(sch, p)); | 5933 | WARN_ON_ONCE(!scx_task_on_sched(sch, p)); |
| 5934 | 5934 | ||
| 5935 | /* | 5935 | /* |
| 5936 | * If $p is about to be freed, nothing prevents $sch from | 5936 | * @p is pinned by the iter: css_task_iter_next() takes a |
| 5937 | * unloading before $p reaches sched_ext_free(). Disable and | 5937 | * reference and holds it until the next iter_next() call, so |
| 5938 | * exit $p right away. | 5938 | * @p->usage is guaranteed > 0. |
| 5939 | */ | 5939 | */ |
| 5940 | if (!tryget_task_struct(p)) { | 5940 | get_task_struct(p); |
| 5941 | scx_disable_and_exit_task(sch, p); | ||
| 5942 | continue; | ||
| 5943 | } | ||
| 5944 | 5941 | ||
| 5945 | scx_task_iter_unlock(&sti); | 5942 | scx_task_iter_unlock(&sti); |
| 5946 | 5943 | ||
| @@ -7181,12 +7178,13 @@ static void scx_root_enable_workfn(struct kthread_work *work) · 1234 unmodified lines | |||
| 7181 | scx_task_iter_start(&sti, NULL); | 7178 | scx_task_iter_start(&sti, NULL); |
| 7182 | while ((p = scx_task_iter_next_locked(&sti))) { | 7179 | while ((p = scx_task_iter_next_locked(&sti))) { |
| 7183 | /* | 7180 | /* |
| 7184 | * @p may already be dead, have lost all its usages counts and | 7181 | * @p is in scx_tasks under scx_tasks_lock, and SCX_TASK_DEAD |
| 7185 | * be waiting for RCU grace period before being freed. @p can't | 7182 | * tasks are filtered by scx_task_iter_next_locked(). |
| 7186 | * be initialized for SCX in such cases and should be ignored. | 7183 | * sched_ext_dead() removes @p from scx_tasks under the same |
| 7184 | * lock before put_task_struct_rcu_user() runs, so @p->usage | ||
| 7185 | * is guaranteed > 0 here. | ||
| 7187 | */ | 7186 | */ |
| 7188 | if (!tryget_task_struct(p)) | 7187 | get_task_struct(p); |
| 7189 | continue; | ||
| 7190 | 7188 | ||
| 7191 | /* | 7189 | /* |
| 7192 | * Set %INIT_BEGIN under the iter's rq lock so that a concurrent | 7190 | * Set %INIT_BEGIN under the iter's rq lock so that a concurrent |
| @@ -7487,9 +7485,8 @@ static void scx_sub_enable_workfn(struct kthread_work *work) · 294 unmodified lines | |||
| 7487 | if (p->scx.flags & SCX_TASK_SUB_INIT) | 7485 | if (p->scx.flags & SCX_TASK_SUB_INIT) |
| 7488 | continue; | 7486 | continue; |
| 7489 | 7487 | ||
| 7490 | /* see scx_root_enable() */ | 7488 | /* @p is pinned by the iter; see scx_sub_disable() */ |
| 7491 | if (!tryget_task_struct(p)) | 7489 | get_task_struct(p); |
| 7492 | continue; | ||
| 7493 | 7490 | ||
| 7494 | if (!assert_task_ready_or_enabled(p)) { | 7491 | if (!assert_task_ready_or_enabled(p)) { |
| 7495 | ret = -EINVAL; | 7492 | ret = -EINVAL; |