linux-kernel - Re: [PATCH -v11][RFC] mutex: implement adaptive spinning

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20090115004645.GB32044@wotan.suse.de>
Date:	Thu, 15 Jan 2009 01:46:45 +0100
From:	Nick Piggin <npiggin@...e.de>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Ingo Molnar <mingo@...e.hu>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Gregory Haskins <ghaskins@...ell.com>,
	Matthew Wilcox <matthew@....cx>,
	Andi Kleen <andi@...stfloor.org>,
	Chris Mason <chris.mason@...cle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>,
	linux-btrfs <linux-btrfs@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Morreale <pmorreale@...ell.com>,
	Sven Dietrich <SDietrich@...ell.com>,
	Dmitry Adamushko <dmitry.adamushko@...il.com>,
	Johannes Weiner <hannes@...xchg.org>
Subject: Re: [PATCH -v11][RFC] mutex: implement adaptive spinning

On Wed, Jan 14, 2009 at 06:22:36PM +0100, Peter Zijlstra wrote:
> On Wed, 2009-01-14 at 18:18 +0100, Nick Piggin wrote:
> 
> > > @@ -173,21 +237,21 @@ __mutex_lock_common(struct mutex *lock, 
> > >  			spin_unlock_mutex(&lock->wait_lock, flags);
> > >  
> > >  			debug_mutex_free_waiter(&waiter);
> > > +			preempt_enable();
> > >  			return -EINTR;
> > >  		}
> > >  		__set_task_state(task, state);
> > >  
> > >  		/* didnt get the lock, go to sleep: */
> > >  		spin_unlock_mutex(&lock->wait_lock, flags);
> > > -		schedule();
> > > +		__schedule();
> > 
> > Why does this need to do a preempt-disabled schedule? After we schedule
> > away, the next task can do arbitrary things or reschedule itself, so if
> > we have not anticipated such a condition here, then I can't see what
> > __schedule protects. At least a comment is in order?
> 
> From:
> http://programming.kicks-ass.net/kernel-patches/mutex-adaptive-spin/mutex-preempt.patch
> 
> Subject: mutex: preemption fixes
> From: Peter Zijlstra <a.p.zijlstra@...llo.nl>
> Date: Wed Jan 14 15:36:26 CET 2009
> 
> The problem is that dropping the spinlock right before schedule is a voluntary
> preemption point and can cause a schedule, right after which we schedule again.
> 
> Fix this inefficiency by keeping preemption disabled until we schedule, do this
> by explicitly disabling preemption and providing a schedule() variant that
> assumes preemption is already disabled.
> 
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@...llo.nl>
> 
> > Pity to add the call overhead to schedule just for this case.
> 
> Good point, seeing any way around that?

Hmm, well this is rather a slow path, I would say. I'd prefer not to
modify schedule in this way (if we just get scheduled back on after
being switched away, the subsequent call to schedule is going to be
cache hot and not do too much work).

preempt_enable_noresched maybe if you really care, would close up the
window even more. But is it really worthwhile? We'd want to see numbers
(when in doubt, keep it  simpler).
 

> >  BTW. __schedule shouldn't need to be asmlinkage?
> 
> TBH I've no clue, probably not, Ingo?

--
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/