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

[K Prateek Nayak <kprateek.nayak@amd.com>]

Hello Suleiman,

On 9/18/2026 12:41 PM, Suleiman Souhlal wrote:
> On Fri, Sep 18, 2026 at 12:38 AM Peter Zijlstra <peterz@infradead.org> wrote:
>>
>> On Thu, Sep 17, 2026 at 04:33:26AM +0000, Suleiman Souhlal wrote:
>>> Switch PI futexes to use p->pi_futex_lock instead of p->pi_lock.
>>>
>>> When augmenting PING futexes with proxy execution, we get lock order
>>> inversions, due to the lock order being p->pi_lock -> mutex->wait_lock
>>> in the scheduler, but wait_lock -> p->pi_lock in futex code.
>>>
>>> So move the futex code to use a new lock, p->pi_futex_lock, to
>>> protect p->pi_state_list and pi_state->owner.
>>
>> This is of course horrible. Lets not do this.
> 
> 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:

  (Lightly tested)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index c54e9fedc9cd..a5a9d3655976 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4008,18 +4008,14 @@ static inline bool proxy_needs_return(struct rq *rq, struct task_struct *p)
 	if (task_cpu(p) == p->wake_cpu)
 		return false;
 
-	scoped_guard(raw_spinlock, &p->blocked_lock) {
-		/* Task is waking up; clear any blocked_on relationship */
-		__clear_task_blocked_on(p, NULL);
+	/* If already current, don't need to return migrate */
+	if (task_current(rq, p))
+		return false;
 
-		/* If already current, don't need to return migrate */
-		if (task_current(rq, p))
-			return false;
+	/* If we're return migrating the rq->donor, switch it out for idle */
+	if (task_current_donor(rq, p))
+		proxy_reset_donor(rq);
 
-		/* If we're return migrating the rq->donor, switch it out for idle */
-		if (task_current_donor(rq, p))
-			proxy_reset_donor(rq);
-	}
 	block_task(rq, p, TASK_WAKING);
 	return true;
 }
@@ -4106,9 +4102,10 @@ static int ttwu_runnable(struct task_struct *p, int wake_flags)
 	update_rq_clock(rq);
 	if (p->is_blocked) {
 		if (p->se.sched_delayed) {
-			proxy_remove_from_sleeping_owner(p);
-			enqueue_task(rq, p, ENQUEUE_NOCLOCK | ENQUEUE_DELAYED);
+			dequeue_task(rq, p, DEQUEUE_SLEEP | DEQUEUE_DELAYED);
+			return 0;
 		}
+
 		if (proxy_needs_return(rq, p))
 			return 0;
 	}
@@ -4305,20 +4302,6 @@ static bool ttwu_queue_wakelist(struct task_struct *p, int cpu, int wake_flags)
 	return false;
 }
 
-static void ttwu_queue(struct task_struct *p, int cpu, int wake_flags)
-{
-	struct rq *rq = cpu_rq(cpu);
-	struct rq_flags rf;
-
-	if (ttwu_queue_wakelist(p, cpu, wake_flags))
-		return;
-
-	rq_lock(rq, &rf);
-	update_rq_clock(rq);
-	ttwu_do_activate(rq, p, wake_flags, &rf);
-	rq_unlock(rq, &rf);
-}
-
 /*
  * Invoked from try_to_wake_up() to check whether the task can be woken up.
  *
@@ -4493,6 +4476,8 @@ int try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
 {
 	guard(preempt)();
 	int cpu, success = 0;
+	unsigned long flags;
+	struct rq *rq;
 
 	wake_flags |= WF_TTWU;
 
@@ -4524,17 +4509,19 @@ int try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
 		goto out;
 	}
 
+	local_irq_save(flags);
+
 	/*
 	 * If we are going to wake up a thread waiting for CONDITION we
 	 * need to ensure that CONDITION=1 done by the caller can not be
 	 * reordered with p->state check below. This pairs with smp_store_mb()
 	 * in set_current_state() that the waiting thread does.
 	 */
-	scoped_guard (raw_spinlock_irqsave, &p->pi_lock) {
+	scoped_guard (raw_spinlock, &p->pi_lock) {
 		smp_mb__after_spinlock();
 
 		if (!ttwu_state_match(p, state, &success))
-			break;
+			goto out_restore;
 
 		trace_sched_waking(p);
 
@@ -4562,7 +4549,7 @@ int try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
 		 */
 		smp_rmb();
 		if (READ_ONCE(p->on_rq) && ttwu_runnable(p, wake_flags))
-			break;
+			goto out_restore;
 
 		/*
 		 * Ensure we load p->on_cpu _after_ p->on_rq, otherwise it would be
@@ -4618,7 +4605,7 @@ int try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
 		 */
 		if (smp_load_acquire(&p->on_cpu) &&
 		    ttwu_queue_wakelist(p, task_cpu(p), wake_flags))
-			break;
+			goto out_restore;
 
 		/*
 		 * If the owning (remote) CPU is still in the middle of schedule() with
@@ -4653,8 +4640,25 @@ int try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
 			p->wake_cpu = cpu;
 		}
 
-		ttwu_queue(p, cpu, wake_flags);
+		if (ttwu_queue_wakelist(p, cpu, wake_flags))
+			goto out_restore;
+
+		/*
+		 * p->__state is set to TASK_WAKING. It is safe to drop
+		 * the p->pi_lock and wake up task outside holding
+		 * rq_lock() similar to the wakelist approach.
+		 */
+	}
+
+	rq = cpu_rq(cpu);
+
+	scoped_guard(rq_lock, rq) {
+		update_rq_clock(rq);
+		ttwu_do_activate(rq, p, wake_flags, &scope.rf);
 	}
+
+out_restore:
+	local_irq_restore(flags);
 out:
 	activate_blocked_waiters(cpu_rq(task_cpu(p)), p, wake_flags);
 	if (success)
---

I'm not sure if PREEMPT_RT needs that activate within pi_lock since
TTWU_QUEUE is disabled but I'll let Peter, John chime in.


> Either of them seemed more involved than creating a new lock, but
> maybe I was wrong.
> I will take another look.

-- 
Thanks and Regards,
Prateek