linux-kernel - Re: [announce] "kill the Big Kernel Lock (BKL)" tree

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]

Date:	Thu, 15 May 2008 22:45:55 +0200
From:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
To:	Arjan van de Ven <arjan@...radead.org>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Ingo Molnar <mingo@...e.hu>, linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Alexander Viro <viro@....linux.org.uk>
Subject: Re: [announce] "kill the Big Kernel Lock (BKL)" tree

On Thu, 2008-05-15 at 13:27 -0700, Arjan van de Ven wrote:
> On Thu, 15 May 2008 10:41:54 -0700 (PDT)
> Linus Torvalds <torvalds@...ux-foundation.org> wrote:
> 
> > 
> > So looking a bit more at your trivial fixups, I'd suggest strongly
> > that they be re-organized a bit.
> 
> > >
> > > diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
> > > index 6eab9bf..e12e571 100644
> > > --- a/net/sunrpc/sched.c
> > > +++ b/net/sunrpc/sched.c
> > > @@ -224,9 +224,15 @@ EXPORT_SYMBOL_GPL(rpc_destroy_wait_queue);
> > >  
> > >  static int rpc_wait_bit_killable(void *word)
> > >  {
> > > +	int bkl = kernel_locked();
> > > +
> > >  	if (fatal_signal_pending(current))
> > >  		return -ERESTARTSYS;
> > > +	if (bkl)
> > > +		unlock_kernel();
> > >  	schedule();
> > > +	if (bkl)
> > > +		lock_kernel();
> > >  	return 0;
> > >  }
> > 
> > The above doesn't even work in general. It depends on having just a
> > single level of locking, and is ugly to boot. So wow about we just
> > expose some version of
> > 
> > 	depth = release_kernel_lock()
> > 	..
> > 	reacquire_kernel_lock(depth);
> > 
> > to existing BKL users as a way to safely release and re-aquire it 
> > regardless of depth. That makes the code more generic, but it *also*
> > makes it more readable than that "if (bkl) [un]lock_kernel()"
> > sequence.
> 
> 
> can we make this even more specific/restricted? Like having something
> like
> 
> call_bkl_unlocked(function_pointer, argument);
> 
> or something that will internally do the full unlock and then the
> function call. The last thing we need is another nailgun that BKL using
> code can use to staple themselves to something big and fast moving.
> By having a more restricted interface... less likely.
> Maybe we can even get away with only a
> 
> drop_bkl_and_schedule();
> 
> and nothing else.

No, that would defeat the whole purpose of the exercise. This drop on
schedule property makes it possible to have inverse lock order and not
deadlock.

That also makes the manual re-acquire on a different level pretty ugly
and deadlock prone.

The whole purpose of this patch series was to get rid of this exact
problem so that the BKL turns into something that resembles a normal
lock within the regular locking hierarchy.

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