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

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

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.