Re: [PATCH 05/14] namei: prepare do_mkdirat for refactoring

[Dmitry Kadashev <dkadashev@gmail.com>]

On Fri, Jul 16, 2021 at 3:17 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Thu, Jul 15, 2021 at 05:35:51PM +0700, Dmitry Kadashev wrote:
> > This is just a preparation for the move of the main mkdirat logic to a
> > separate function to make the logic easier to follow.  This change
> > contains the flow changes so that the actual change to move the main
> > logic to a separate function does no change the flow at all.
> >
> > Just like the similar patches for rmdir and unlink a few commits before,
> > there two changes here:
> >
> > 1. Previously on filename_create() error the function used to exit
> > immediately, and now it will check the return code to see if ESTALE
> > retry is appropriate. The filename_create() does its own retries on
> > ESTALE (at least via filename_parentat() used inside), but this extra
> > check should be completely fine.
>
> This is the wrong way to go.  Really.  Look at it that way - LOOKUP_REVAL
> is the final stage of escalation; if we had to go there, there's no
> point being optimistic about the last dcache lookup, nevermind trying
> to retry the parent pathwalk if we fail with -ESTALE doing it.
>
> I'm not saying that it's something worth optimizing for; the problem
> is different - the logics makes no sense whatsoever that way.  It's
> a matter of reader's cycles wasted on "what the fuck are we trying
> to do here?", not the CPU cycles wasted on execution.
>
> While we are at it, it makes no sense for filename_parentat() and its
> ilk to go for RCU and normal if it's been given LOOKUP_REVAL - I mean,
> look at the sequence of calls in there.  And try to make sense of
> it.  Especially of the "OK, RCU attempt told us to sod off and try normal;
> here, let's call path_parentat() with LOOKUP_REVAL for flags and if it
> says -ESTALE, call it again with exact same arguments" part.
>
> Seriously, look at that from the point of view of somebody who tries
> to make sense of the entire thing

OK, let me try to venture down that "change the way ESTALE retries are
done completely" path. The problem here is I'm not familiar with the
code enough to be sure the conversion is 1-to-1 (i.e. that we can't get
ESTALE from somewhere unexpected), and that retries are open-coded in
quite a few places it seems. Anyway, I'll try and dig in and come back
with either an RFC patch or some questions. Thanks for the feedback, Al.

-- 
Dmitry Kadashev