linux-kernel - Re: [PATCH v2] fs: try an opportunistic lookup for O

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: <44862ec7c85cdc19529e26f47176d0ecfc90d888.camel@kernel.org>
Date: Tue, 06 Aug 2024 15:26:28 -0400
From: Jeff Layton <jlayton@...nel.org>
To: Andi Kleen <ak@...ux.intel.com>, Mateusz Guzik <mjguzik@...il.com>
Cc: Alexander Viro <viro@...iv.linux.org.uk>, Christian Brauner
	 <brauner@...nel.org>, Jan Kara <jack@...e.cz>, Andrew Morton
	 <akpm@...ux-foundation.org>, Josef Bacik <josef@...icpanda.com>, 
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] fs: try an opportunistic lookup for O_CREAT opens too

On Tue, 2024-08-06 at 12:11 -0700, Andi Kleen wrote:
> Mateusz Guzik <mjguzik@...il.com> writes:
> > 
> > I would bench with that myself, but I temporarily don't have handy
> > access to bigger hw. Even so, the below is completely optional and
> > perhaps more of a suggestion for the future :)
> > 
> > I hacked up the test case based on tests/open1.c.
> 
> Don't you need two test cases? One where the file exists and one
> where it doesn't. Because the "doesn't exist" will likely be slower
> than before because it will do the lookups twice,
> and it will likely even slow single threaded.
> 
> I assume the penalty will also depend on the number of entries
> in the path.
> 
> That all seem to be an important considerations in judging the benefits
> of the patch.
> 

Definitely.

FWIW, I did test a single threaded (bespoke) test case that did a bunch
of O_CREAT opens, closes and then unlinks. I didn't measure any
discernable difference with this patch. My conclusion from that was
that the extra lockless lookup should be cheap.

That said, this could show a difference if you have rather long hash
chains that need to be walked completely, and you have to actually do
the create every time. In practice though, time spent under the
inode_lock and doing the create tends to dominate in that case, so I
*think* this should still be worthwhile.

I'll plan to add a test like that to will_it_scale unless Mateusz beats
me to it. A long soak in linux-next is probably also justified with
this patch.
-- 
Jeff Layton <jlayton@...nel.org>