On Fri, Jul 16, 2021 at 2:49 AM Al Viro [off-list ref] wrote:

On Thu, Jul 15, 2021 at 05:35:47PM +0700, Dmitry Kadashev wrote:

quoted

This is just a preparation for the move of the main rmdir 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.

Two changes here:

1. Previously on filename_parentat() error the function used to exit
immediately, and now it will check the return code to see if ESTALE
retry is appropriate. The filename_parentat() does its own retries on
ESTALE, but this extra check should be completely fine.

2. The retry_estale() check is wrapped in unlikely(). Some other places
already have that and overall it seems to make sense.

That's not the way to do it.

static inline bool
retry_estale(const long error, const unsigned int flags)
{
        return unlikely(error == -ESTALE && !(flags & LOOKUP_REVAL));
}

And strip the redundant unlikely in the callers.  Having that markup
in callers makes sense only when different callers have different
odds of positive result, which is very much not the case here.

Yeah, I thought about this, but wasn't sure about interplay of
inline+[un]likely(). But I see that it's used quite a bit throughout the
kernel code so I suppose it's fine. I'll use that next time, thanks.


--
Dmitry Kadashev