linux-kernel - Re: [PATCH v1 11/11] locks: give the blocked

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:	Tue, 4 Jun 2013 10:39:35 -0400
From:	Jeff Layton <jlayton@...hat.com>
To:	"Stefan (metze) Metzmacher" <metze@...ba.org>
Cc:	viro@...iv.linux.org.uk, matthew@....cx, bfields@...ldses.org,
	linux-cifs@...r.kernel.org, linux-nfs@...r.kernel.org,
	cluster-devel@...hat.com, sage@...tank.com,
	samba-technical@...ts.samba.org, Trond.Myklebust@...app.com,
	linux-kernel@...r.kernel.org, linux-afs@...ts.infradead.org,
	dhowells@...hat.com, smfrench@...il.com,
	linux-fsdevel@...r.kernel.org, ceph-devel@...r.kernel.org,
	akpm@...ux-foundation.org, swhiteho@...hat.com
Subject: Re: [PATCH v1 11/11] locks: give the blocked_hash its own spinlock

On Tue, 04 Jun 2013 16:19:53 +0200
"Stefan (metze) Metzmacher" <metze@...ba.org> wrote:

> Hi Jeff,
> 
> > There's no reason we have to protect the blocked_hash and file_lock_list
> > with the same spinlock. With the tests I have, breaking it in two gives
> > a barely measurable performance benefit, but it seems reasonable to make
> > this locking as granular as possible.
> 
> as file_lock_{list,lock} is only used for debugging (/proc/locks) after this
> change, I guess it would be possible to use RCU instead of a spinlock.
> 
> @others: this was the related discussion on IRC
> (http://irclog.samba.org/) about this:
> 
> 16:02 < metze> jlayton: do you have time to discuss your file_lock_lock
> changes?
> 16:02 < jlayton> metze: sure, what's up?
> 16:03 < jlayton> metze: note that it won't help vl's thundering herd
> problems...
> 16:03 < metze> is it correct that after your last patch file_lock_lock
> is only used for /proc/locks?
> 16:03 < jlayton> well, it's only used to protect the list that is used
> for /proc/locks
> 16:04 < jlayton> it still gets taken whenever a lock is acquired or
> released in order to manipulate that list
> 16:04 < metze> would it be a good idea to use rcu instead of a spin lock?
> 16:04 < jlayton> I tried using RCU, but it turned out to slow everything
> down
> 16:04 < jlayton> this is not a read-mostly workload unfortunately
> 16:04 < jlayton> so doing it with mutual exclusion turns out to be faster
> 16:04 < metze> ok
> 16:05 < jlayton> I might play around with it again sometime, but I don't
> think it really helps. What we need to ensure is
>                  that we optimize the code that manipulates that list,
> and RCU list manipulations have larger overhead
> 16:06 < jlayton> metze: that's a good question though so if you want to
> ask it on the list, please do
> 16:06 < jlayton> others will probably be wondering the same thing
> 16:08 < metze> maybe it's worth a comment in commit message and the code


I'm not sure it's worth commenting about RCU in the code. In the future
it may be possible to do this -- who knows. It does seem to have been
consistently slower in my testing here though.

> 16:08 < metze> btw, why don't you remove the ' /* Protects the
> file_lock_list and the blocked_hash */' comment?
> 

Removed in my git tree -- thanks for pointing that out.

-- 
Jeff Layton <jlayton@...hat.com>

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)