Re: [PATCH v3 07/19] VFS: Add LOOKUP_SHARED flag.

NeilBrown
2026-04-27T08:47:19Z

On Mon, 27 Apr 2026, Amir Goldstein wrote:
> On Mon, Apr 27, 2026 at 6:06 AM NeilBrown <neilb@ownmail.net> wrote:
> >
> > From: NeilBrown <neil@brown.name>
> >
> > Some ->lookup handlers will need to drop and retake the parent lock, so
> > they can safely use d_alloc_parallel().
> >
> > ->lookup can be called with the parent lock either exclusive or shared.
> >
> > A new flag, LOOKUP_SHARED, tells ->lookup how the parent is locked.
> >
> > This is rather ugly, but will be gone soon after we move
> > d_alloc_parallel() out of the directory lock as ->lookup() will *always*
> > called with a shared lock on the parent.
>
> Neil,
>
> Forgive me for being skeptical about the *always* part.

Scepticism should always be encouraged.

>
> How long ago did we add ->iterate_shared()?
>
> It's true that Linus eventually got rid of ->iterate(), but we did not
> get rid of the assumption that iterate_shared() might be upgraded
> to exclusive lock.
>
> The obvious reason is that *someone* needs to do this work for
> old filesystems, which are also hard to test and nobody wants to
> touch them.
>
> I have nothing against this patch, but I think it is more realistic
> to state that LOOKUP_SHARED is here to stay, so if you think it
> is too ugly, maybe there is something to be done about it.
> Personally, I do not see the ugliness though.
>
> Am I misjudging the situation of shared lookup wrt old filesystems?

Yes.
All filesystems must support ->lookup with a shared lock because it is
already the case that with a simple lookup only a shared lock is provided.
A filesystem *could* examine the lookup_flags and deduce if the lock is
actually exclusive (e.g. if LOOKUP_CREATE is set) and misbehave
accordingly, but a vanishing small number of filesystems (NFS and ....
I can't think of any others) ever look at the lookup_flags and I'm
certain none do anything so ... creative.

So I'm certain that it is safe from the filesystem's perspective to always
call with only a shared lock. All that is needed is to change the VFS
to only ever do that. That means separating the lock for lookup from
the lock for create/remove/move in directory ops, and one way to view
the current patch set (the complete one, not just this initial set) is
that it does exactly that.

Thanks for the review,
NeilBrown