Re: [PATCH 2/3] sched/psi: Prevent stale timer rearm after rtpoll teardown

[Suren Baghdasaryan]

On Fri, Jul 17, 2026 at 2:14 AM Guopeng Zhang <guopeng.zhang@xxxxxxxxx> wrote:
>
> From: Guopeng Zhang <zhangguopeng@xxxxxxxxxx>
>
> psi_schedule_rtpoll_work() reads rtpoll_task under RCU before calling
> mod_timer(). Last-trigger teardown clears the pointer and deletes the
> timer before waiting for existing readers. A reader that saw the old task
> can therefore rearm the timer after timer_delete(), leaving a stale timer
> pending after trigger teardown.
>
> psi_cgroup_free() shuts down rtpoll_timer before freeing the group, so the
> pending timer cannot outlive the psi_group. It can still fire after the
> last trigger has been removed and wake the waitqueue when no worker is
> published, and trigger teardown does not leave the timer quiesced.

quiesced? Don't you just love these AI generated changelogs?

>
> After publishing NULL, wait for existing readers while holding
> rtpoll_trigger_lock, then use timer_delete_sync() to drain the callback.
> Holding the lock also prevents a new trigger from reusing the timer until
> teardown has finished with it.

I've seen a report of this problem generated by AI and I think it's
legitimate; however, so far I could not reproduce it even after
injecting delays to increase the possibility of this race. Have you
been able to reproduce it? If so, could you please share the
reproducer?

>
> Fixes: 8f91efd870ea ("psi: Fix race between psi_trigger_create/destroy")
> Signed-off-by: Guopeng Zhang <zhangguopeng@xxxxxxxxxx>
> ---
>  kernel/sched/psi.c | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
> index b9e2a93a757b..db9c56fa8923 100644
> --- a/kernel/sched/psi.c
> +++ b/kernel/sched/psi.c
> @@ -1488,18 +1488,22 @@ void psi_trigger_destroy(struct psi_trigger *t)
>                                                 group->rtpoll_task,
>                                                 lockdep_is_held(&group->rtpoll_trigger_lock));
>                                 rcu_assign_pointer(group->rtpoll_task, NULL);
> -                               timer_delete(&group->rtpoll_timer);
> +                               /*
> +                                * Wait for psi_schedule_rtpoll_work() to either
> +                                * observe the NULL task or finish rearming the timer.
> +                                * Keeping the mutex held also prevents a new trigger
> +                                * from installing a task before the old timer is gone.
> +                                */
> +                               synchronize_rcu();
> +                               timer_delete_sync(&group->rtpoll_timer);

Ok, poll_timer_fn() does not take rtpoll_trigger_lock, so I think this
is safe. I would like to double-check the code and run some tests
before approving this fix.

>                         }
>                 }
>                 mutex_unlock(&group->rtpoll_trigger_lock);
>         }
>
> -       /*
> -        * Wait for psi_schedule_rtpoll_work RCU to complete its read-side
> -        * critical section before destroying the trigger and optionally the
> -        * rtpoll_task.
> -        */
> -       synchronize_rcu();
> +       /* The last-trigger path has already waited for RCU readers above. */
> +       if (!task_to_destroy)
> +               synchronize_rcu();
>         /*
>          * Stop kthread 'psimon' after releasing rtpoll_trigger_lock to prevent
>          * a deadlock while waiting for psi_rtpoll_work to acquire
> --
> 2.43.0