io-uring.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Dmitry Kadashev <dkadashev@gmail.com>
Cc: Jens Axboe <axboe@kernel.dk>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner <christian.brauner@ubuntu.com>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	io-uring <io-uring@vger.kernel.org>
Subject: Re: [PATCH 5/7] namei: clean up do_symlinkat retry logic
Date: Mon, 12 Jul 2021 11:54:31 -0700	[thread overview]
Message-ID: <CAHk-=whH4msnFkj=iYZ9NDmZEAiZKM+vii803M8gnEwEsF1-Yg@mail.gmail.com> (raw)
In-Reply-To: <20210712123649.1102392-6-dkadashev@gmail.com>

On Mon, Jul 12, 2021 at 5:37 AM Dmitry Kadashev <dkadashev@gmail.com> 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

  reply	other threads:[~2021-07-12 18:54 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-12 12:36 [PATCH 0/7] namei: clean up retry logic in various do_* functions Dmitry Kadashev
2021-07-12 12:36 ` [PATCH 1/7] namei: clean up do_rmdir retry logic Dmitry Kadashev
2021-07-13 14:53   ` Christian Brauner
2021-07-13 16:57     ` Linus Torvalds
2021-07-15 10:38       ` Dmitry Kadashev
2021-07-12 12:36 ` [PATCH 2/7] namei: clean up do_unlinkat " Dmitry Kadashev
2021-07-12 12:36 ` [PATCH 3/7] namei: clean up do_mkdirat " Dmitry Kadashev
2021-07-12 12:36 ` [PATCH 4/7] namei: clean up do_mknodat " Dmitry Kadashev
2021-07-12 18:46   ` Linus Torvalds
2021-07-12 12:36 ` [PATCH 5/7] namei: clean up do_symlinkat " Dmitry Kadashev
2021-07-12 18:54   ` Linus Torvalds [this message]
2021-07-12 12:36 ` [PATCH 6/7] namei: clean up do_linkat " Dmitry Kadashev
2021-07-12 18:57   ` Linus Torvalds
2021-07-12 12:36 ` [PATCH 7/7] namei: clean up do_renameat " Dmitry Kadashev
2021-07-12 12:41 ` [PATCH 0/7] namei: clean up retry logic in various do_* functions Dmitry Kadashev
2021-07-12 19:01   ` Linus Torvalds
2021-07-12 20:25     ` Al Viro
2021-07-13 12:28       ` Dmitry Kadashev
2021-07-13 10:22     ` Dmitry Kadashev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAHk-=whH4msnFkj=iYZ9NDmZEAiZKM+vii803M8gnEwEsF1-Yg@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=christian.brauner@ubuntu.com \
    --cc=dkadashev@gmail.com \
    --cc=io-uring@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).