locking/mutexes: don't spin on owner when wait list is not NULL.

kernel/locking/mutex.c static inline int mutex_can_spin_on_owner(struct mutex *lock) · line 261

Quoted 13 lines · Ding Tianhong

kernel/locking/mutex.c +1 -1

index 0551c21..596b341 100644
@@ -256,7 +256,7 @@ static inline int mutex_can_spin_on_owner(struct mutex *lock)
 	struct task_struct *owner;
 	int retval = 1;
 
-	if (need_resched())
+	if (need_resched() || atomic_read(&lock->count) == -1)
 		return 0;
 

One concern I have is this change will eliminate any optimistic spinning as long as there is a waiter. Is there a middle ground that we can allow only one spinner if there are waiters?

In other words, we allow spinning when atomic_read(&lock->count) == -1 but there is no one on the osq lock that queue up the spinners (i.e. no other process doing optimistic spinning).

This could allow a bit of spinning without starving out the waiters.

I did some testing, which exposed it to the 0day test robot, which did note some performance differences. I was hoping that it would clear up some instability from other patches, but no such luck. ;-)

Quoted 3 lines · Paul E. McKenney

I did some testing, which exposed it to the 0day test robot, which did note some performance differences. I was hoping that it would clear up some instability from other patches, but no such luck. ;-)

Oh, that explains why we got a performance regression report :)

I build a script to create several process for ioctl loop calling, the ioctl will calling the kernel function just like:
xx_ioctl {
...
rtnl_lock();
function();
rtnl_unlock();
...
}
The function may sleep several ms, but will not halt, at the same time another user service may calling ifconfig to change the state of the ethernet, and after several hours, the hung task thread report this problem:

========================================================================
149738.039038] INFO: task ifconfig:11890 blocked for more than 120 seconds.

[149738.040597] "echo 0>  /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[149738.042280] ifconfig D ffff88061ec13680 0 11890 11573 0x00000080
[149738.042284] ffff88052449bd40 0000000000000082 ffff88053a33f300 ffff88052449bfd8
[149738.042286] ffff88052449bfd8 ffff88052449bfd8 ffff88053a33f300 ffffffff819e6240
[149738.042288] ffffffff819e6244 ffff88053a33f300 00000000ffffffff ffffffff819e6248
[149738.042290] Call Trace:
[149738.042300] [<ffffffff8160d219>] schedule_preempt_disabled+0x29/0x70
[149738.042303] [<ffffffff8160af65>] __mutex_lock_slowpath+0xc5/0x1c0
[149738.042305] [<ffffffff8160a3cf>] mutex_lock+0x1f/0x2f
[149738.042309] [<ffffffff8150d945>] rtnl_lock+0x15/0x20
[149738.042311] [<ffffffff81514e3a>] dev_ioctl+0xda/0x590
[149738.042314] [<ffffffff816121cc>] ? __do_page_fault+0x21c/0x560
[149738.042318] [<ffffffff814e42c5>] sock_do_ioctl+0x45/0x50
[149738.042320] [<ffffffff814e49d0>] sock_ioctl+0x1f0/0x2c0
[149738.042324] [<ffffffff811dc9b5>] do_vfs_ioctl+0x2e5/0x4c0
[149738.042327] [<ffffffff811e6a00>] ? fget_light+0xa0/0xd0

================================ cut here ================================

I got the vmcore and found that the ifconfig is already in the wait_list of the rtnl_lock for 120 second, but my process could get and release the rtnl_lock normally several times in one second, so it means that my process jump the queue and the ifconfig couldn't get the rtnl all the time, I check the mutex lock slow path and found that the mutex may spin on owner ignore whether the wait list is empty, it will cause the task in the wait list always be cut in line, so add test for wait list in the mutex_can_spin_on_owner and avoid this problem.

Signed-off-by Ding Tianhong<[email protected]>

Cc Ingo Molnar<[email protected]>

Cc Peter Zijlstra<[email protected]>

Cc Davidlohr Bueso<[email protected]>

Cc Linus Torvalds<[email protected]>

Cc Paul E. McKenney<[email protected]>

Cc Thomas Gleixner<[email protected]>

Cc Will Deacon<[email protected]>

Cc Jason Low<[email protected]>

Cc Tim Chen<[email protected]>

Cc Waiman Long<[email protected]>

---
  kernel/locking/mutex.c | 11 ++++++-----
  1 file changed, 6 insertions(+), 5 deletions(-)

kernel/locking/mutex.c +1 -3

index 0551c21..596b341 100644
@@ -256,7 +256,7 @@ static inline int mutex_can_spin_on_owner(struct mutex *lock)
  	struct task_struct *owner;
  	int retval = 1;
 
-	if (need_resched())
+	if (need_resched() || atomic_read(&lock->count) == -1)
  		return 0;
 
  	rcu_read_lock();

@@ -283,10 +283,11 @@ static inline bool mutex_try_to_acquire(struct mutex *lock)
  /*
   * Optimistic spinning.
   *
- * We try to spin for acquisition when we find that the lock owner
- * is currently running on a (different) CPU and while we don't

Quoted 7 lines · Ding Tianhong

I got the vmcore and found that the ifconfig is already in the wait_list of the rtnl_lock for 120 second, but my process could get and release the rtnl_lock normally several times in one second, so it means that my process jump the queue and the ifconfig couldn't get the rtnl all the time, I check the mutex lock slow path and found that the mutex may spin on owner ignore whether the wait list is empty, it will cause the task in the wait list always be cut in line, so add test for wait list in the mutex_can_spin_on_owner and avoid this problem.

Quoted 1 line

[...]

From: Waiman Long <[email protected]>
Date: Thu, 21 Jan 2016 17:53:14 -0500
Subject: [PATCH] locking/mutex: Enable optimistic spinning of woken task in wait list

Ding Tianhong reported a live-lock situation where a constant stream of incoming optimistic spinners blocked a task in the wait list from getting the mutex.

This patch attempts to fix this live-lock condition by enabling the a woken task in the wait list to enter optimistic spinning loop itself with precedence over the ones in the OSQ. This should prevent the live-lock
condition from happening.

Quoted 9 lines · Waiman Long

Quoted 7 lines · Ding Tianhong

I got the vmcore and found that the ifconfig is already in the wait_list of the rtnl_lock for 120 second, but my process could get and release the rtnl_lock normally several times in one second, so it means that my process jump the queue and the ifconfig couldn't get the rtnl all the time, I check the mutex lock slow path and found that the mutex may spin on owner ignore whether the wait list is empty, it will cause the task in the wait list always be cut in line, so add test for wait list in the mutex_can_spin_on_owner and avoid this problem.

So this has been somewhat always known, at least in theory, until now. It's the cost of spinning without going through the wait-queue, unlike other locks.

Quoted 10 lines

Quoted 1 line

[...]

From: Waiman Long <[email protected]>
Date: Thu, 21 Jan 2016 17:53:14 -0500
Subject: [PATCH] locking/mutex: Enable optimistic spinning of woken task in wait list

Ding Tianhong reported a live-lock situation where a constant stream of incoming optimistic spinners blocked a task in the wait list from getting the mutex.

Quoted 5 lines

This patch attempts to fix this live-lock condition by enabling the a woken task in the wait list to enter optimistic spinning loop itself with precedence over the ones in the OSQ. This should prevent the live-lock
condition from happening.

And one of the reasons why we never bothered 'fixing' things was the additional branching out in the slowpath (and lack of real issue, although this one being so damn pathological). I fear that your approach is one of those scenarios where the code ends up being bloated, albeit most of it is actually duplicated and can be refactored *sigh*. So now we'd spin, then sleep, then try spinning then sleep again... phew. Not to mention the performance implications, ie loosing the benefits of osq over waiter spinning in scenarios that would otherwise have more osq spinners as opposed to waiter spinners, or in setups where it is actually best to block instead of spinning.

The patch that I sent out is just a proof of concept to make sure that it can fix that particular case. I do plan to refactor it if I decide to go ahead with an official one. Unlike the OSQ, there can be no more than one waiter spinner as the wakeup function is directed to only the first task in the wait list and the spinning won't happen until the task is first woken up. In the worst case scenario, there are only 2 spinners spinning on the lock and the owner field, one from OSQ and one from the wait list. That shouldn't put too much cacheline contention traffic to the system.

Make use of wake_q and enable the wakeup to occur after releasing the wait_lock. This is similar to what we do with rtmutex top waiter, slightly shortening the critical region and allow other waiters to acquire the wait_lock sooner. In low contention cases it can also help the recently woken waiter to find the wait_lock available (fastpath) when it continues execution.

Signed-off-by Davidlohr Bueso <[email protected]>

---
 kernel/locking/mutex.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

kernel/locking/mutex.c +3 -2

index 0551c21..e364b42 100644
@@ -716,6 +716,7 @@ static inline void
 __mutex_unlock_common_slowpath(struct mutex *lock, int nested)
 {
     unsigned long flags;
+    WAKE_Q(wake_q);
 
     /*
      * As a performance measurement, release the lock before doing other

@@ -743,11 +744,11 @@ __mutex_unlock_common_slowpath(struct mutex 
 *lock, int nested)
                        struct mutex_waiter, list);
 
         debug_mutex_wake_waiter(lock, waiter);
-
-        wake_up_process(waiter->task);
+        wake_q_add(&wake_q, waiter->task);
     }
 
     spin_unlock_mutex(&lock->wait_lock, flags);
+    wake_up_q(&wake_q);
 }
 

/*

This patch attempts to fix this live-lock condition by enabling the a woken task in the wait list to enter optimistic spinning loop itself with precedence over the ones in the OSQ. This should prevent the live-lock
condition from happening.

Quoted 5 lines · Waiman Long

This patch attempts to fix this live-lock condition by enabling the a woken task in the wait list to enter optimistic spinning loop itself with precedence over the ones in the OSQ. This should prevent the live-lock
condition from happening.

So I think having the top waiter going back in to contend on the OSQ is an excellent idea, but I'm not sure the wlh_spinning thing is important.

The OSQ itself is FIFO fair, and the waiters retain the wait_list position. So having the top wait_list entry contending on the OSQ ensures we cannot starve (I think).

kernel/locking/mutex.c +2 -0

@@ -543,6 +543,8 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
 	lock_contended(&lock->dep_map, ip);
 
 	for (;;) {
+		bool acquired = false;
+
 		/*
 		 * Lets try to take the lock again - this is needed even if
 		 * we get here for the first time (shortly after failing to

@@ -577,7 +579,12 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
 		/* didn't get the lock, go to sleep: */

There might be other details, but this is the one that stood out.

Quoted 1 line · Peter Zijlstra

There might be other details, but this is the one that stood out.

I think this also does the wrong thing for use_ww_ctx.

Quoted 4 lines · Peter Zijlstra

Quoted 1 line · Peter Zijlstra

There might be other details, but this is the one that stood out.

I think this also does the wrong thing for use_ww_ctx.

Something like so?

---
  kernel/locking/mutex.c | 15 +++++++++++++++
  1 file changed, 15 insertions(+)

kernel/locking/mutex.c +14 -0

index 0551c219c40e..070a0ac34aa7 100644
@@ -512,6 +512,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
  	struct task_struct *task = current;
  	struct mutex_waiter waiter;
  	unsigned long flags;
+	bool acquired;
  	int ret;
 
  	preempt_disable();

@@ -543,6 +544,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
  	lock_contended(&lock->dep_map, ip);
 
  	for (;;) {
+		acquired = false;
  		/*
  		 * Lets try to take the lock again - this is needed even if
  		 * we get here for the first time (shortly after failing to

@@ -577,7 +579,16 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
  		/* didn't get the lock, go to sleep: */
  		spin_unlock_mutex(&lock->wait_lock, flags);
  		schedule_preempt_disabled();
+
+		if (mutex_is_locked(lock))
+			acquired = mutex_optimistic_spin(lock, ww_ctx, use_ww_ctx);
+
  		spin_lock_mutex(&lock->wait_lock, flags);
+
+		if (acquired) {
+			atomic_set(&lock->count, -1);
+			break;
+		}
  	}
  	__set_task_state(task, TASK_RUNNING);
 

@@ -587,6 +598,9 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
  		atomic_set(&lock->count, 0);
  	debug_mutex_free_waiter(&waiter);
 
+	if (acquired)
+		goto unlock;
+
  skip_wait:
  	/* got the lock - cleanup and rejoice! */
  	lock_acquired(&lock->dep_map, ip);

Quoted 6 lines · Peter Zijlstra

Quoted 4 lines · Peter Zijlstra

Quoted 1 line · Peter Zijlstra

There might be other details, but this is the one that stood out.

I think this also does the wrong thing for use_ww_ctx.

Something like so?

I think that should work. My only minor concern is that putting the waiter spinner at the end of the OSQ will take it longer to get the lock, but that shouldn't be a big issue.

Quoted 55 lines

---
  kernel/locking/mutex.c | 15 +++++++++++++++
  1 file changed, 15 insertions(+)

kernel/locking/mutex.c +14 -0

index 0551c219c40e..070a0ac34aa7 100644
@@ -512,6 +512,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
      struct task_struct *task = current;
      struct mutex_waiter waiter;
      unsigned long flags;
+    bool acquired;
      int ret;
 
      preempt_disable();

@@ -543,6 +544,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
      lock_contended(&lock->dep_map, ip);
 
      for (;;) {
+        acquired = false;
          /*
           * Lets try to take the lock again - this is needed even if
           * we get here for the first time (shortly after failing to

@@ -577,7 +579,16 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
          /* didn't get the lock, go to sleep: */
          spin_unlock_mutex(&lock->wait_lock, flags);
          schedule_preempt_disabled();
+
+        if (mutex_is_locked(lock))
+            acquired = mutex_optimistic_spin(lock, ww_ctx, use_ww_ctx);
+
          spin_lock_mutex(&lock->wait_lock, flags);
+
+        if (acquired) {
+            atomic_set(&lock->count, -1);
+            break;
+        }
      }
      __set_task_state(task, TASK_RUNNING);
 

@@ -587,6 +598,9 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
          atomic_set(&lock->count, 0);
      debug_mutex_free_waiter(&waiter);
 
+    if (acquired)
+        goto unlock;
+
  skip_wait:
      /* got the lock - cleanup and rejoice! */
      lock_acquired(&lock->dep_map, ip);

@@ -597,6 +611,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
          ww_mutex_set_context_slowpath(ww, ww_ctx);
      }
 

kernel/locking/mutex.c __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass, · line 617

Quoted 4 lines

kernel/locking/mutex.c +1 -0

+unlock:
      spin_unlock_mutex(&lock->wait_lock, flags);
      preempt_enable();
      return 0;

Cheers,
Longman

.

looks good to me, I will try this solution and report the result, thanks everyone.

Quoted 1 line · Ding Tianhong

looks good to me, I will try this solution and report the result, thanks everyone.

Did you get a change to run with this?

.

Quoted 6 lines · Peter Zijlstra

Quoted 1 line · Ding Tianhong

looks good to me, I will try this solution and report the result, thanks everyone.

Did you get a change to run with this?

Quoted 2 lines

.

I backport this patch to 3.10 lts kernel, and didn't change any logic, Till now, the patch works fine to me, and no need to change anything, So I think this patch is no problem, could you formal release this patch to the latest kernel? :)

Thanks.
Ding

Quoted 10 lines · Ding Tianhong

Quoted 8 lines · Peter Zijlstra

Quoted 1 line · Ding Tianhong

looks good to me, I will try this solution and report the result, thanks everyone.

Did you get a change to run with this?

.

Quoted 9 lines

I backport this patch to 3.10 lts kernel, and didn't change any
logic, Till now, the patch works fine to me, and no need to change
anything,
So I think this patch is no problem, could you formal release this patch to the latest kernel? :)

Thanks.
Ding

The original patch from Tianhong triggered a performance regression because the optimistic spinning is turned off in effect. I tested Peter's patch with same configuration and there show no regression. So I think the patch keep the optimistic spinning. Test result details will be in the next email.

Quoted 6 lines · Peter Zijlstra

Quoted 1 line · Ding Tianhong

looks good to me, I will try this solution and report the result, thanks everyone.

Did you get a change to run with this?

Quoted 2 lines

.

I backport this patch to 3.10 lts kernel, and didn't change any logic, Till now, the patch works fine to me, and no need to change anything, So I think this patch is no problem, could you formal release this patch to the latest kernel? :)

Subject: locking/mutex: Avoid spinner vs waiter starvation
From: Peter Zijlstra <[email protected]>
Date: Fri, 22 Jan 2016 12:06:53 +0100

Ding Tianhong reported that under his load the optimistic spinners would totally starve a task that ended up on the wait list.

Fix this by ensuring the top waiter also partakes in the optimistic spin queue.

There are a few subtle differences between the assumed state of regular optimistic spinners and those already on the wait list, which result in the @acquired complication of the acquire path.

Most notable are:

- waiters are on the wait list and need to be taken off
- mutex_optimistic_spin() sets the lock->count to 0 on acquire even though there might be more tasks on the wait list.

Quoted 14 lines · Peter Zijlstra

Subject: locking/mutex: Avoid spinner vs waiter starvation
From: Peter Zijlstra <[email protected]>
Date: Fri, 22 Jan 2016 12:06:53 +0100

Ding Tianhong reported that under his load the optimistic spinners would totally starve a task that ended up on the wait list.

Fix this by ensuring the top waiter also partakes in the optimistic spin queue.

There are a few subtle differences between the assumed state of regular optimistic spinners and those already on the wait list, which result in the @acquired complication of the acquire path.

Quoted 5 lines

Most notable are:

- waiters are on the wait list and need to be taken off
- mutex_optimistic_spin() sets the lock->count to 0 on acquire even though there might be more tasks on the wait list.

Right, the main impact I see with these complications are that the window of when a waiter takes the lock via spinning and then acquires the wait_lock to remove itself from the list, will allow an unlock thread to set the lock as available in the fastpath which could in turn allow a third thread the steal the lock. With high contention, this window will be come obviously larger as we contend for the wait_lock.

CPU-0 CPU-1 CPU-3
__mutex_lock_common mutex_optimistic_spin
(->count now 0)

            __mutex_fastpath_unlock
            (->count now 1)                 __mutex_fastpath_lock
                                      (stolen)

spin_lock_mutex(&lock->wait_lock, flags);

But we've always been bad when it comes to counter and waiters.

Thanks,
Davidlohr

.

Agree, but this patch is going to help the waiter in the wait list to get the lock, your scene probability looks more too low and I don't think it is a problem.

Quoted 2 lines · Ding Tianhong

Agree, but this patch is going to help the waiter in the wait list to get the lock, your scene probability looks more too low and I don't think it is a problem.

Sure, I was in fact implying its not the end of the world, although it will be interesting to see the impact on different (non pathological) workloads, even if it only affects a single waiter. Also, technically this issue can also affect rwsems if only using writers, but that's obviously pretty idiotic, so I wouldn't worry about it.

Thanks,
Davidlohr

.

Quoted 11 lines · Davidlohr Bueso

Quoted 2 lines · Ding Tianhong

Agree, but this patch is going to help the waiter in the wait list to get the lock, your scene probability looks more too low and I don't think it is a problem.

Sure, I was in fact implying its not the end of the world, although it will be interesting to see the impact on different (non pathological) workloads, even if it only affects a single waiter. Also, technically this issue can also affect rwsems if only using writers, but that's obviously pretty idiotic, so I wouldn't worry about it.

Quoted 3 lines

Thanks,
Davidlohr

Hi Davidlohr, Peter:

According Davidlohr's suggestion, I use several VM to test Peter's patch, and sadly I found one VM still happen Hung Task for this problem, so I think we still need to think more about this solution.

Thanks.
Ding

Quoted 14 lines · Peter Zijlstra

Subject: locking/mutex: Avoid spinner vs waiter starvation
From: Peter Zijlstra <[email protected]>
Date: Fri, 22 Jan 2016 12:06:53 +0100

Ding Tianhong reported that under his load the optimistic spinners would totally starve a task that ended up on the wait list.

Fix this by ensuring the top waiter also partakes in the optimistic spin queue.

There are a few subtle differences between the assumed state of regular optimistic spinners and those already on the wait list, which result in the @acquired complication of the acquire path.

Quoted 5 lines

Most notable are:

- waiters are on the wait list and need to be taken off
- mutex_optimistic_spin() sets the lock->count to 0 on acquire even though there might be more tasks on the wait list.

Right, the main impact I see with these complications are that the window of when a waiter takes the lock via spinning and then acquires the wait_lock to remove itself from the list, will allow an unlock thread to set the lock as available in the fastpath which could in turn allow a third thread the steal the lock. With high contention, this window will be come obviously larger as we contend for the wait_lock.

CPU-0 CPU-1 CPU-3
__mutex_lock_common mutex_optimistic_spin
(->count now 0)

            __mutex_fastpath_unlock
            (->count now 1)                 __mutex_fastpath_lock
                                      (stolen)

spin_lock_mutex(&lock->wait_lock, flags);

But we've always been bad when it comes to counter and waiters.

Thanks,
Davidlohr

Quoted 7 lines · Ding Tianhong

Quoted 6 lines · Peter Zijlstra

Quoted 1 line · Ding Tianhong

looks good to me, I will try this solution and report the result, thanks everyone.

Did you get a change to run with this?

Quoted 7 lines

Quoted 2 lines

.

I backport this patch to 3.10 lts kernel, and didn't change any logic, Till now, the patch works fine to me, and no need to change anything, So I think this patch is no problem, could you formal release this patch to the latest kernel? :)

Thanks for testing, I've queued the below patch.

---
Subject: locking/mutex: Avoid spinner vs waiter starvation
From: Peter Zijlstra <[email protected]>
Date: Fri, 22 Jan 2016 12:06:53 +0100

Ding Tianhong reported that under his load the optimistic spinners would totally starve a task that ended up on the wait list.

Fix this by ensuring the top waiter also partakes in the optimistic spin queue.

There are a few subtle differences between the assumed state of regular optimistic spinners and those already on the wait list, which result in the @acquired complication of the acquire path.

Most notable are:

- waiters are on the wait list and need to be taken off
- mutex_optimistic_spin() sets the lock->count to 0 on acquire even though there might be more tasks on the wait list.

Cc Jason Low <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Tim Chen <[email protected]>

Cc Linus Torvalds <[email protected]>

Cc Waiman Long <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc "Paul E. McKenney" <[email protected]>

Cc Davidlohr Bueso <[email protected]>

Cc Will Deacon <[email protected]>

Reported-by Ding Tianhong <[email protected]>

Tested-by Ding Tianhong <[email protected]>

Tested-by "Huang, Ying" <[email protected]>

Suggested-by Waiman Long <[email protected]>

Signed-off-by Peter Zijlstra (Intel) <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

I've done some testing with this patch with some of the AIM7 workloads and found that this reduced throughput by about 10%. The reduction in throughput is expected since spinning as a waiter is less efficient.

Another observation I made is that the top waiter spinners would often times require needing to reschedule before being able to acquire the lock from spinning when there was high contention. A waiter can go into the cycle of spin -> reschedule -> spin -> reschedule. So although the chance of starvation is reduced, this patch doesn't fully address the issue of waiter starvation.

Hi, Low,

Quoted 4 lines · Jason Low

I've done some testing with this patch with some of the AIM7 workloads and found that this reduced throughput by about 10%. The reduction in throughput is expected since spinning as a waiter is less efficient.

Quoted 6 lines

Another observation I made is that the top waiter spinners would often times require needing to reschedule before being able to acquire the lock from spinning when there was high contention. A waiter can go into the cycle of spin -> reschedule -> spin -> reschedule. So although the chance of starvation is reduced, this patch doesn't fully address the issue of waiter starvation.

Could you share your workload? I want to reproduce it in 0day/LKP+ environment.

Quoted 5 lines · Waiman Long

This patch attempts to fix this live-lock condition by enabling the a woken task in the wait list to enter optimistic spinning loop itself with precedence over the ones in the OSQ. This should prevent the live-lock
condition from happening.

So I think having the top waiter going back in to contend on the OSQ is an excellent idea, but I'm not sure the wlh_spinning thing is important.

The OSQ itself is FIFO fair, and the waiters retain the wait_list position. So having the top wait_list entry contending on the OSQ ensures we cannot starve (I think).

Also, as Davidlohr said, we cannot copy/paste this much code.