public inbox for linux-api@vger.kernel.org
 help / color / mirror / Atom feed
From: "Drew DeVault" <sir@cmpwn.com>
To: "Al Viro" <viro@zeniv.linux.org.uk>
Cc: <linux-fsdevel@vger.kernel.org>, <linux-api@vger.kernel.org>,
	"Aleksa Sarai" <cyphar@cyphar.com>
Subject: Re: [RFC PATCH] fs: introduce mkdirat2 syscall for atomic mkdir
Date: Sun, 28 Feb 2021 08:56:04 -0500	[thread overview]
Message-ID: <C9L7RW0S7YU0.16I8160PKEP0K@taiga> (raw)
In-Reply-To: <YDsGzhBzLzSp6nPj@zeniv-ca.linux.org.uk>

On Sat Feb 27, 2021 at 9:58 PM EST, Al Viro wrote:
> open() *always* returns descriptor or an error, for one thing.
> And quite a few of open() flags are completely wrong for mkdir,
> starting with symlink following and truncation.

So does mkdirat2. Are you referring to the do_mkdirat2 function? I
merged mkdir/mkdirat/mkdirat2 into one function with a flag to enable
the mkdirat2 behavior, to avoid copying and pasting much of the
functionality. However, the syscalls themselves don't overload their
return value as you expect. mkdir & mkdirat both still return 0 or an
error, and mkdirat2 always returns an fd or an error. If you prefer, I
can leave their implementations separate so that this is more clear.

I supposed the flags might be wrong - should I just introduce a new set
of flags, with the specific ones which are useful (which I think is just
O_CLOEXEC)?

> What's more, your implementation is both racy and deadlock-prone -
> it repeats the entire pathwalk with no warranty that it'll
> arrive to the object you've created *AND* if you have
> something like /foo/bar/baz/../../splat and dentry of bar
> gets evicted on memory pressure, that pathwalk will end up
> trying to look bar up. In the already locked /foo, aka
> /foo/bar/baz/../..

This is down to unfamiliarity with this code, I think. I'll try to give
it a closer look.

> TBH, I don't understand what are you trying to achieve -
> what will that mkdir+open combination buy you, especially
> since that atomicity goes straight out of window if you try
> to use that on e.g. NFS. How is the userland supposed to make
> use of that thing?

I'm trying to close what appears to be an oversight in the API. See the
previous threads:

https://lore.kernel.org/linux-fsdevel/C9KKYZ4T5O53.338Y48UIQ9W3H@taiga/T/#t
https://lore.kernel.org/linux-fsdevel/20200316142057.xo24zea3k5zwswra@yavin/

Userland uses it the same way they use mkdir+open, but in one call, so
that they can use the directory they make as soon as it's created. The
atomicity goal, if possible, would also add a reference to the new
directory via the open fd, so they can use it even if it's removed by
another process. It makes such applications less error-prone, albiet in
a minor edge case.

I'm not sure what's involved with the NFS case, but I can look into it.

  reply	other threads:[~2021-02-28 13:57 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-28  0:25 [RFC PATCH] fs: introduce mkdirat2 syscall for atomic mkdir Drew DeVault
2021-02-28  2:13 ` Al Viro
2021-02-28  2:21   ` Drew DeVault
2021-02-28  2:58     ` Al Viro
2021-02-28 13:56       ` Drew DeVault [this message]
2021-03-01 19:02       ` J. Bruce Fields
2021-03-08 13:50         ` Stefan Metzmacher
2021-02-28  2:24 ` Matthew Wilcox
2021-02-28  2:26   ` Drew DeVault
2021-02-28  4:03     ` Matthew Wilcox
2021-02-28 13:57       ` Drew DeVault
2021-03-01 19:09         ` J. Bruce Fields
2021-03-01 19:35           ` Matthew Wilcox
2021-03-01 20:10             ` J. Bruce Fields
2021-03-02  8:24             ` Miklos Szeredi
2021-03-02  7:13         ` Amir Goldstein
2021-03-03  2:39           ` Aleksa Sarai

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=C9L7RW0S7YU0.16I8160PKEP0K@taiga \
    --to=sir@cmpwn.com \
    --cc=cyphar@cyphar.com \
    --cc=linux-api@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