| Message ID | 20210712123649.1102392-5-dkadashev@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | namei: clean up retry logic in various do_* functions | expand |
On Mon, Jul 12, 2021 at 5:37 AM Dmitry Kadashev <dkadashev@gmail.com> wrote: > > Moving the main logic to a helper function makes the whole thing much > easier to follow. This patch works, but the conversion is not one-to-one. > -static int do_mknodat(int dfd, struct filename *name, umode_t mode, > - unsigned int dev) > +static int mknodat_helper(int dfd, struct filename *name, umode_t mode, > + unsigned int dev, unsigned int lookup_flags) > { > struct user_namespace *mnt_userns; > struct dentry *dentry; > struct path path; > int error; > - unsigned int lookup_flags = 0; > > error = may_mknod(mode); > if (error) > - goto out1; > -retry: Note how "may_mknod()" was _outside_ the retry before, and now it's inside: > +static int do_mknodat(int dfd, struct filename *name, umode_t mode, > + unsigned int dev) > +{ > + int error; > + > + error = mknodat_helper(dfd, name, mode, dev, 0); > + if (retry_estale(error, 0)) > + error = mknodat_helper(dfd, name, mode, dev, LOOKUP_REVAL); > + > putname(name); > return error; which happens to not be fatal - doing the may_mknod() twice will not break anything - but it doesn't match what it used to do. A few options options: (a) a separate patch to move the "may_mknod()" to the two callers first (b) a separate patch to move the "may_mknod()" first into the repeat loop, with the comment being that it's ok. (c) keep the logic the same, with the code something like static int do_mknodat(int dfd, struct filename *name, umode_t mode, unsigned int dev) { int error; error = may_mknod(mode); if (!error) { error = mknodat_helper(dfd, name, mode, dev, 0); if (retry_estale(error, 0)) error = mknodat_helper(dfd, name, mode, dev, LOOKUP_REVAL); } putname(name); return error; } or (d) keep the patch as-is, but with an added commit message note about how it's not one-to-one and why it's ok. Hmm? So this patch could be fine, but it really wants to note how it changes the logic and why that's fine. Or, the patch should be modified. Linus
diff --git a/fs/namei.c b/fs/namei.c index b9762e2cf3b9..7bf7a9f38ce2 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -3745,29 +3745,27 @@ static int may_mknod(umode_t mode) } } -static int do_mknodat(int dfd, struct filename *name, umode_t mode, - unsigned int dev) +static int mknodat_helper(int dfd, struct filename *name, umode_t mode, + unsigned int dev, unsigned int lookup_flags) { struct user_namespace *mnt_userns; struct dentry *dentry; struct path path; int error; - unsigned int lookup_flags = 0; error = may_mknod(mode); if (error) - goto out1; -retry: + return error; dentry = __filename_create(dfd, name, &path, lookup_flags); error = PTR_ERR(dentry); if (IS_ERR(dentry)) - goto out1; + return error; if (!IS_POSIXACL(path.dentry->d_inode)) mode &= ~current_umask(); error = security_path_mknod(&path, dentry, mode, dev); if (error) - goto out2; + goto out; mnt_userns = mnt_user_ns(path.mnt); switch (mode & S_IFMT) { @@ -3786,13 +3784,20 @@ static int do_mknodat(int dfd, struct filename *name, umode_t mode, dentry, mode, 0); break; } -out2: +out: done_path_create(&path, dentry); - if (retry_estale(error, lookup_flags)) { - lookup_flags |= LOOKUP_REVAL; - goto retry; - } -out1: + return error; +} + +static int do_mknodat(int dfd, struct filename *name, umode_t mode, + unsigned int dev) +{ + int error; + + error = mknodat_helper(dfd, name, mode, dev, 0); + if (retry_estale(error, 0)) + error = mknodat_helper(dfd, name, mode, dev, LOOKUP_REVAL); + putname(name); return error; }
Moving the main logic to a helper function makes the whole thing much easier to follow. Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Al Viro <viro@zeniv.linux.org.uk> Cc: Christian Brauner <christian.brauner@ubuntu.com> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> Link: https://lore.kernel.org/io-uring/CAHk-=wiG+sN+2zSoAOggKCGue2kOJvw3rQySvQXsZstRQFTN+g@mail.gmail.com/ Signed-off-by: Dmitry Kadashev <dkadashev@gmail.com> --- fs/namei.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-)