[v9,00/11] io_uring: add mkdir and [sym]linkat support

Message

Dmitry Kadashev July 8, 2021, 6:34 a.m. UTC

This started out as an attempt to add mkdirat support to io_uring which
is heavily based on renameat() / unlinkat() support.

During the review process more operations were added (linkat, symlinkat,
mknodat) mainly to keep things uniform internally (in namei.c), and
with things changed in namei.c adding support for these operations to
io_uring is trivial, so that was done too (except for mknodat). See
https://lore.kernel.org/io-uring/20210514145259.wtl4xcsp52woi6ab@wittgenstein/

The first patch makes putname() ignore IS_ERR_OR_NULL names and converts
a couple of places where the check was already implemented in the
callers. No functional changes.

The second one splits filename_lookup() that used to consume the passed
struct filename * on error but not on the success (returning it) into
two: filename_lookup(), that always consumes the name and
__filename_lookup() that never does. This is a preparation change to
enable the subsequent changes to filename_create and filename_lookup. No
functional changes.

The third patch is preparation with no functional changes, makes
do_mkdirat accept struct filename pointer rather than the user string.

4-7 just convert other similar do_* functions in namei.c to accept
struct filename, for uniformity with do_mkdirat, do_renameat and
do_unlinkat. No functional changes there.

8 changes do_* helpers in namei.c to return ints rather than some of
them returning ints and some longs.

9-11 leverages the previous changes to add mkdirat, symlinkat and linkat
support to io_uring correspondingly

Based on for-5.14/io_uring.

v9:
- reorder commits to keep io_uring ones nicely grouped at the end
- change 'fs:' to 'namei:' in related commit subjects, since this is
  what seems to be usually used in such cases

v8:
- update filename_parentat() calling conventions to be uniform with the
  ones followed by (changed in subsequent patches) filename_create()
  and filename_lookup()

v7:
- rebase
- make putname() ignore IS_ERR_OR_NULL names, remove conditional calls
  to it from the callers

v6:

- rebase
- add safety checks for IOPOLL mode
- add safety checks for unused sqe parts
- drop mknodat support from io_uring as requested by Jens
- add Christian's Acked-by

v5:
- rebase
- add symlinkat, linkat and mknodat support to io_uring

v4:
- update do_mknodat, do_symlinkat and do_linkat to accept struct
  filename for uniformity with do_mkdirat, do_renameat and do_unlinkat;

v3:
- rebase;

v2:
- do not mess with struct filename's refcount in do_mkdirat, instead add
  and use __filename_create() that does not drop the name on success;

Dmitry Kadashev (11):
  namei: ignore ERR/NULL names in putname()
  namei: change filename_parentat() calling conventions
  namei: make do_mkdirat() take struct filename
  namei: make do_mknodat() take struct filename
  namei: make do_symlinkat() take struct filename
  namei: add getname_uflags()
  namei: make do_linkat() take struct filename
  namei: update do_*() helpers to return ints
  io_uring: add support for IORING_OP_MKDIRAT
  io_uring: add support for IORING_OP_SYMLINKAT
  io_uring: add support for IORING_OP_LINKAT

 fs/exec.c                     |   8 +-
 fs/internal.h                 |   8 +-
 fs/io_uring.c                 | 196 ++++++++++++++++++++++++++++
 fs/namei.c                    | 239 +++++++++++++++++++---------------
 include/linux/fs.h            |   1 +
 include/uapi/linux/io_uring.h |   4 +
 6 files changed, 346 insertions(+), 110 deletions(-)

Comments

Linus Torvalds July 8, 2021, 6:34 p.m. UTC | #1

On Wed, Jul 7, 2021 at 11:35 PM Dmitry Kadashev <[e-mail]> wrote:
>
> v9:
> - reorder commits to keep io_uring ones nicely grouped at the end
> - change 'fs:' to 'namei:' in related commit subjects, since this is
>   what seems to be usually used in such cases

Ok, ack from me on this series, and as far as I'm concerned it can go
through the io_uring branch.

Al, please holler if you have any concerns.

I do see a few cleanups - the ones I've already mentioned to try to
remove some of the goto spaghetti, and I think we end up with just two
users of filename_create(), and we might just make those convert to
the new world order, and get rid of the __filename_create() vs
filename_creat() distinction.

But those cleanups might as well be left for later, so I don't think
that needs to hold the series up.

Al - one last chance to speak up..

           Linus

Jens Axboe July 8, 2021, 7:25 p.m. UTC | #2

On 7/8/21 12:34 PM, Linus Torvalds wrote:
> On Wed, Jul 7, 2021 at 11:35 PM Dmitry Kadashev <[e-mail]> wrote:
>>
>> v9:
>> - reorder commits to keep io_uring ones nicely grouped at the end
>> - change 'fs:' to 'namei:' in related commit subjects, since this is
>>   what seems to be usually used in such cases
> 
> Ok, ack from me on this series, and as far as I'm concerned it can go
> through the io_uring branch.

I'll queue it up in a separate branch. I'm assuming we're talking 5.15
at this point.

> Al, please holler if you have any concerns.

Indeed.

Dmitry Kadashev Aug. 13, 2021, 9:32 a.m. UTC | #3

On Fri, Jul 9, 2021 at 2:25 AM Jens Axboe <[e-mail]> wrote:
>
> On 7/8/21 12:34 PM, Linus Torvalds wrote:
> > On Wed, Jul 7, 2021 at 11:35 PM Dmitry Kadashev <[e-mail]> wrote:
> >>
> >> v9:
> >> - reorder commits to keep io_uring ones nicely grouped at the end
> >> - change 'fs:' to 'namei:' in related commit subjects, since this is
> >>   what seems to be usually used in such cases
> >
> > Ok, ack from me on this series, and as far as I'm concerned it can go
> > through the io_uring branch.
>
> I'll queue it up in a separate branch. I'm assuming we're talking 5.15
> at this point.

Is this going to be merged into 5.15? I'm still working on the follow-up
patch (well, right at this moment I'm actually on vacation, but will be
working on it when I'm back), but hopefully it does not have to be
merged in the same merge window / version? Especially given the fact
that Al prefers it to be a bigger refactoring of the ESTALE retries
rather than just moving bits and pieces to helper functions to simplify
the flow, see here:

https://lore.kernel.org/io-uring/[e-mail]/

Jens Axboe Aug. 13, 2021, 2:12 p.m. UTC | #4

On 8/13/21 3:32 AM, Dmitry Kadashev wrote:
> On Fri, Jul 9, 2021 at 2:25 AM Jens Axboe <[e-mail]> wrote:
>>
>> On 7/8/21 12:34 PM, Linus Torvalds wrote:
>>> On Wed, Jul 7, 2021 at 11:35 PM Dmitry Kadashev <[e-mail]> wrote:
>>>>
>>>> v9:
>>>> - reorder commits to keep io_uring ones nicely grouped at the end
>>>> - change 'fs:' to 'namei:' in related commit subjects, since this is
>>>>   what seems to be usually used in such cases
>>>
>>> Ok, ack from me on this series, and as far as I'm concerned it can go
>>> through the io_uring branch.
>>
>> I'll queue it up in a separate branch. I'm assuming we're talking 5.15
>> at this point.
> 
> Is this going to be merged into 5.15? I'm still working on the follow-up
> patch (well, right at this moment I'm actually on vacation, but will be
> working on it when I'm back), but hopefully it does not have to be
> merged in the same merge window / version? Especially given the fact
> that Al prefers it to be a bigger refactoring of the ESTALE retries
> rather than just moving bits and pieces to helper functions to simplify
> the flow, see here:
> 
> https://lore.kernel.org/io-uring/[e-mail]/

I added this to the for-5.15/io_uring-vfs branch:

https://git.kernel.dk/cgit/linux-block/log/?h=for-5.15/io_uring-vfs

had one namei.c conflict, set_nameidata() taking one more parameter, and
just a trivial conflict in each io_uring patch at the end. Can you double
check them?

Dmitry Kadashev Aug. 16, 2021, 10:24 a.m. UTC | #5

On Fri, Aug 13, 2021 at 9:12 PM Jens Axboe <[e-mail]> wrote:
>
> On 8/13/21 3:32 AM, Dmitry Kadashev wrote:
> > On Fri, Jul 9, 2021 at 2:25 AM Jens Axboe <[e-mail]> wrote:
> >>
> >> On 7/8/21 12:34 PM, Linus Torvalds wrote:
> >>> On Wed, Jul 7, 2021 at 11:35 PM Dmitry Kadashev <[e-mail]> wrote:
> >>>>
> >>>> v9:
> >>>> - reorder commits to keep io_uring ones nicely grouped at the end
> >>>> - change 'fs:' to 'namei:' in related commit subjects, since this is
> >>>>   what seems to be usually used in such cases
> >>>
> >>> Ok, ack from me on this series, and as far as I'm concerned it can go
> >>> through the io_uring branch.
> >>
> >> I'll queue it up in a separate branch. I'm assuming we're talking 5.15
> >> at this point.
> >
> > Is this going to be merged into 5.15? I'm still working on the follow-up
> > patch (well, right at this moment I'm actually on vacation, but will be
> > working on it when I'm back), but hopefully it does not have to be
> > merged in the same merge window / version? Especially given the fact
> > that Al prefers it to be a bigger refactoring of the ESTALE retries
> > rather than just moving bits and pieces to helper functions to simplify
> > the flow, see here:
> >
> > https://lore.kernel.org/io-uring/[e-mail]/
>
> I added this to the for-5.15/io_uring-vfs branch:
>
> https://git.kernel.dk/cgit/linux-block/log/?h=for-5.15/io_uring-vfs
>
> had one namei.c conflict, set_nameidata() taking one more parameter, and
> just a trivial conflict in each io_uring patch at the end. Can you double
> check them?

Looks good to me, thanks!