linux-kernel - Re: softlockups in multi_cpu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 7 Mar 2015 09:55:26 +0800
From:	Ming Lei <ming.lei@...onical.com>
To:	Davidlohr Bueso <dave@...olabs.net>
Cc:	Jason Low <jason.low2@...com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Ingo Molnar <mingo@...nel.org>,
	Sasha Levin <sasha.levin@...cle.com>,
	Peter Zijlstra <peterz@...radead.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Dave Jones <davej@...emonkey.org.uk>
Subject: Re: softlockups in multi_cpu_stop

On Fri, 06 Mar 2015 14:15:37 -0800
Davidlohr Bueso <dave@...olabs.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/