Re: [PATCH 0/7] namei: clean up retry logic in various do_* functions

[Al Viro <viro@zeniv.linux.org.uk>]

On Mon, Jul 12, 2021 at 12:01:52PM -0700, Linus Torvalds wrote:
> On Mon, Jul 12, 2021 at 5:41 AM Dmitry Kadashev <dkadashev@gmail.com> wrote:
> >
> > Since this is on top of the stuff that is going to be in the Jens' tree
> > only until the 5.15 merge window, I'm assuming this series should go
> > there as well.
> 
> Yeah. Unless Al wants to pick this whole series up.
> 
> See my comments about the individual patches - some of them change
> code flow, others do. And I think changing code flow as part of
> cleanup is ok, but it at the very least needs to be mentioned (and it
> might be good to do the "move code that is idempotent inside the
> retry" as a separate patch from documentation purposes)

TBH, my main problem with this is that ESTALE retry logics had never
felt right.  We ask e.g. filename_create() to get us the parent.  We
tell it whether we want it to be maximally suspicious or not.  It
still does the same RCU-normal-LOOKUP_REVAL sequence, only for "trust
no one" variant it's RCU-LOOKUP_REVAL-LOOKUP_REVAL instead.  We are
*not* told how far in that sequence did it have to get.  What's more,
even if we had to get all way up to LOOKUP_REVAL, we ignore that
when we do dcache lookup for the last component - only the argument
of filename_create() is looked at.

It really smells like the calling conventions are wrong.  I agree that
all of that is, by definition, a very slow path - it's just that the
logics makes me go "WTF?" every time I see it... ;-/

Hell knows - perhaps the lookup_flags thing wants to be passed by
reference (all the way into path_parentat()) and have the "we had
to go for LOOKUP_REVAL" returned that way.  Not sure...

Al, still crawling out of the bloody ptrace/asm glue horrors at the moment...