On Mon, Jul 12, 2021 at 5:37 AM Dmitry Kadashev [off-list ref] wrote:

+
+int do_symlinkat(struct filename *from, int newdfd, struct filename *to)
+{
+       int error;
+
+       if (IS_ERR(from)) {
+               error = PTR_ERR(from);
+               goto out;
        }
-out_putnames:
+
+       error = symlinkat_helper(from, newdfd, to, 0);
+       if (retry_estale(error, 0))
+               error = symlinkat_helper(from, newdfd, to, LOOKUP_REVAL);
+
+out:
        putname(to);
        putname(from);
        return error;

So here you moved that part that was outside the retry loop into the
caller. Except it's very ugly and keeps the goto mess.

So I'd suggest either keep it as a nested if - avoiding the goto - or
like in the previous patch, do that "we can do this test twice" with a
big commit message note about why it's ok.

Because it _is_ ok to repeat the test inside the retry_estale, and
'from' won't have changed (and won't have been -ESTALE in the first
place).

Looking at the pattern of this and the previous one, I think just
repeating the test is what generates the cleanest end result.

               Linus