Re: softlockups in multi_cpu_stop

[Ming Lei <ming.lei@canonical.com>]

On Fri, 06 Mar 2015 14:15:37 -0800
Davidlohr Bueso <dave@stgolabs.net> wrote:

> On Fri, 2015-03-06 at 13:12 -0800, Jason Low wrote:
> > In owner_running() there are 2 conditions that would make it return
> > false: if the owner changed or if the owner is not running. However,
> > that patch continues spinning if there is a "new owner" but it does not
> > take into account that we may want to stop spinning if the owner is not
> > running (due to getting rescheduled).
> 
> So you're rationale is that we're missing this need_resched:
> 
> 	while (owner_running(sem, owner)) {
> 		/* abort spinning when need_resched */
> 		if (need_resched()) {
> 			rcu_read_unlock();
> 			return false;
> 		}
> 	}
> 
> Because the owner_running() would return false, right? Yeah that makes
> sense, as missing a resched is a bug, as opposed to our heuristics being
> so painfully off.
> 
> Sasha, Ming (Cc'ed), does this address the issues you guys are seeing?

For the xfstest lockup, what matters is that the owner isn't running, since
the following simple change does fix the issue:

diff --git a/kernel/locking/rwsem-xadd.c b/kernel/locking/rwsem-xadd.c
index 06e2214..5e08705 100644
--- a/kernel/locking/rwsem-xadd.c
+++ b/kernel/locking/rwsem-xadd.c
@@ -358,8 +358,9 @@ bool rwsem_spin_on_owner(struct rw_semaphore *sem, struct task_struct *owner)
 	}
 	rcu_read_unlock();
 
-	if (READ_ONCE(sem->owner))
-		return true; /* new owner, continue spinning */
+	owner = READ_ONCE(sem->owner);
+	if (owner && owner->on_cpu)
+		return true;
 
 	/*
 	 * When the owner is not set, the lock could be free or


Thanks,
Ming Lei