Re: [PATCH v2] fs: try an opportunistic lookup for O_CREAT opens too

[Jeff Layton <jlayton@kernel.org>]

On Tue, 2024-08-06 at 12:11 -0700, Andi Kleen wrote:
> Mateusz Guzik <mjguzik@gmail.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@kernel.org>