Re: [RFC PATCH 02/12] futex: Switch PI futex to use p->pi_futex_lock instead of p->pi_lock.

[Suleiman Souhlal]

On Fri, Oct 16, 2026 at 5:16 PM K Prateek Nayak <kprateek.nayak@xxxxxxx> wrote:
>
> Hello Suleiman,
>
> On 10/16/2026 11:29 AM, Suleiman Souhlal wrote:
> >>> I suppose the alternatives would be to either figure out how to
> >>> un-nest pi_lock from wait_lock in futex code or un-nesting the
> >>> blocked_on lock from pi_lock in the scheduler.
> >>
> >> So the only time we try to grab pi_lock + blocked_lock is during
> >> wakeup so I think doing that should be (theoretically) possible.
> >>
> >> The ttwu_runnable() handling is extremely painful unless we block
> >> the delayed task fully. Since we have he new p->is_blocked state,
> >> we can safely retain the p->blocked_on while running the task -
> >> it is either cleared during unlock by previosu owner or will be
> >> same as the lock on which the task blocked on.
> >>
> >>
> >> Based on John's tree at commit 06ac43db4d8e ("[ANNOTATION] ===
> >> Needs confirmation of functionality past this point ===") on
> >> proxy-exec-v31-7.2-rc4 branch:
> >
> > Sorry for the slow reply and thanks for the very quick patch.
> >
> > Unfortunately, it doesn't seem like this approach works in the end,
> > because there is still a pi_lock -> rq lock -> blocked_lock transitive
> > relationship, which deadlocks when trying to get the pi_lock while
> > holding the blocked_lock (when fixing up the pi_state owner in the
> > futex code). :-(
>
> Ah! So what you are saying is, since we have:
>
>   p->pi_lock
>     rq_lock /* ttwu_runnable() */
>
> and
>
>   rq_lock
>    blocked_lock /* find_proxy_task() */
>
> relations, doing a:
>
>   blocked_lock
>     p->pi_lock
>
> would cause a deadlock via rq_lock. True that.

Yes, exactly that.

>
> >
> > On the other hand, Peter's suggestion of moving the scheduler's use of
> > pi_lock to a new lock, p->sched_lock, seems to work in my testing, but
> > is kind of a scary change to me.
> >
> > Maybe an acceptable alternative would be for the new futex to use a
> > new lock (similarly to what the original patch does), but keep regular
> > PI futexes using pi_lock, at least as long as they are still based on
> > rtmutexes.
>
> I guess that should do too, yes.
>
> On a different note, why do we need the p->pi_lock/p->pi_futex_lock to
> modify the ping_state->owner in ping_state_update_owner()? Isn't
> ping_state->ping_mutex.wait_lock + owner->blocked_on locks enough?

It's because ping_states need to be on a per-task list,
task->futex.ping_state_list, so that when a task exits, we can walk
all the ping_states it might still own and unlock/free them (this also
applies to pi_states with PI futexes).
See exit_ping_state_list() in patch 5 in this series or
exit_pi_state_list() for regular PI futexes.
(owner->blocked_on lock is just wait_lock)

-- Suleiman

>
> Note: I'm still wrapping my head around the PING implementation so it
> might take me a while to appreciate the need for the pi_futex_lock.
> I'm using proxy-exec-v32-7.3-rc-tip-sched-core from John's tree at the
> moment to jump around.
>
> --
> Thanks and Regards,
> Prateek
>