Re: [PATCH 2/2] sched: Consider task_struct::saved_state in wait_task_inactive().

From: Sebastian Andrzej Siewior
Date: Tue Jul 26 2022 - 02:22:54 EST


On 2022-07-25 18:47:58 [+0100], Valentin Schneider wrote:
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -3257,6 +3257,40 @@ int migrate_swap(struct task_struct *cur
> > }
> > #endif /* CONFIG_NUMA_BALANCING */
> >
> > +#ifdef CONFIG_PREEMPT_RT
>
> Would something like the below be useful?
>
> /*
> * If p->saved_state is anything else than TASK_RUNNING, then p blocked on an
> * rtlock *before* voluntarily calling into schedule() after setting its state
> * to X. For things like ptrace (X=TASK_TRACED), the task could have more work
> * to do upon acquiring the lock before whoever called wait_task_inactive()
> * should return. IOW, we have to wait for:
> *
> * p.saved_state = TASK_RUNNING
> * p.__state = X
> *
> * which implies the task isn't blocked on an RT lock and got to schedule() by
> * itself.
> *
> * Also see comments in ttwu_state_match().
> */

This sums up the code. I would s/schedule/schedule_rtlock/ since there
are two entrypoints.


> > @@ -3346,7 +3382,7 @@ unsigned long wait_task_inactive(struct
> > * running right now), it's preempted, and we should
> > * yield - it could be a while.
> > */
> > - if (unlikely(queued)) {
> > + if (unlikely(wait)) {
>
> We could be repeatedly doing this for as long as the task is blocked on the
> rtlock, but IIUC that's the same story on !PREEMPT_RT if it's just a queued
> task preempted by a higher prio task, it may take a while for it to
> schedule() and dequeue...

Yes.

> > ktime_t to = NSEC_PER_SEC / HZ;
> >
> > set_current_state(TASK_UNINTERRUPTIBLE);

Sebastian