Re: [PATCH 01/14] sched: Employ sched_change guards

K Prateek Nayak
2025-09-11T09:06:33Z

Hello Peter,

On 9/10/2025 9:14 PM, Peter Zijlstra wrote:
> @@ -9240,8 +9213,9 @@ static void sched_change_group(struct ta
> */
> void sched_move_task(struct task_struct *tsk, bool for_autogroup)
> {
> - int queued, running, queue_flags =
> + unsigned int queue_flags =
> DEQUEUE_SAVE | DEQUEUE_MOVE | DEQUEUE_NOCLOCK;

nit.

Since we don't do a complete dequeue for delayed task in
sched_move_task(), can we get rid of that DEQUEUE_NOCLOCK and ...

> + bool resched = false;
> struct rq *rq;
>
> CLASS(task_rq_lock, rq_guard)(tsk);
> @@ -9249,28 +9223,12 @@ void sched_move_task(struct task_struct
>
> update_rq_clock(rq);

... this clock update and instead rely on sched_change_begin() to
handle it within the guard?

>
> - running = task_current_donor(rq, tsk);
> - queued = task_on_rq_queued(tsk);
> -
> - if (queued)
> - dequeue_task(rq, tsk, queue_flags);
> - if (running)
> - put_prev_task(rq, tsk);
> -
> - sched_change_group(tsk);
> - if (!for_autogroup)
> - scx_cgroup_move_task(tsk);
> -
> - if (queued)
> - enqueue_task(rq, tsk, queue_flags);
> - if (running) {
> - set_next_task(rq, tsk);
> - /*
> - * After changing group, the running task may have joined a
> - * throttled one but it's still the running task. Trigger a
> - * resched to make sure that task can still run.
> - */
> - resched_curr(rq);
> + scoped_guard (sched_change, tsk, queue_flags) {
> + sched_change_group(tsk);
> + if (!for_autogroup)
> + scx_cgroup_move_task(tsk);
> + if (scope->running)
> + resched = true;
> }

Also, are we missing a:

if (resched)
resched_curr(rq);

here after the guard? I don't see anything in sched_change_end() at this
point that would trigger a resched.

> }
--
Thanks and Regards,
Prateek