lkml.org 
[lkml]   [2023]   [May]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Subject[PATCH v2 0/6] io_uring: add getdents support, take 2
    Date
    This is an attempt to revive discussion after bringing it up as a reply
    to the last attempt last week.

    Since the problem with the previous attempt at adding getdents to
    io_uring was that offset was problematic, we can just not add an offset
    parameter: using different offsets is rarely useful in practice (maybe
    for some cacheless userspace NFS server?) and more isn't worth the cost
    of allowing arbitrary offsets: just allow rewind as a flag instead.
    [happy to drop even that flag for what I care about, but that might be
    useful enough on its own as io_uring rightfully has no seek API]

    The new API does nothing that cannot be achieved with plain syscalls so
    it shouldn't be introducing any new problem, the only downside is that
    having the state in the file struct isn't very uring-ish and if a
    better solution is found later that will probably require duplicating
    some logic in a new flag... But that seems like it would likely be a
    distant future, and this version should be usable right away.

    To repeat the rationale for having getdents available from uring as it
    has been a while, while I believe there can be real performance
    improvements as suggested in [1], the real reason is to allow coherency
    in code: applications using io_uring usually center their event loop
    around the ring, and having the occasional synchronous getdents call in
    there is cumbersome and hard to do properly when you consider e.g. a
    slow or acting up network fs...
    [1] https://lore.kernel.org/linux-fsdevel/20210218122640.GA334506@wantstofly.org/
    (unfortunately the source is no longer available...)

    liburing implementation:
    https://github.com/martinetd/liburing/commits/getdents
    (will submit properly after initial discussions here)

    Previous discussion:
    https://lore.kernel.org/all/20211221164004.119663-1-shr@fb.com/T/#m517583f23502f32b040c819d930359313b3db00c

    Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
    ---
    Changes in v2:
    - implement NOWAIT as suggested by dchinner; I'm unable to provide
    reliable benchmarks but it does indeed look positive (and makes sense)
    to avoid spinning out to another thread when not required.
    FWIW though, the serializing readdirs per inode argument given in v1
    thread isn't valid: serialization is only done in io_prep_async_work
    for regular files (REQ_F_ISREG, set from file mode through FFS_ISREG),
    so dir operations aren't serialized in our case.
    If I was pedantic I'd say we might want that hashing for filesystems
    that don't implement interate_shared, but that info comes too late and
    these filesystems should become less frequent so leaving as is.
    - implement NOWAIT for kernfs and libfs to test with /sys
    (the tentative patch for xfs didn't seem to work, didn't take the time
    to debug)
    - try to implement some EOF flag in CQE as suggested by Clay Harris,
    this is less clearly cut and left as RFC.
    The liburing test/getdents.t implementation has grown options to test
    this flag and also try async explicitly in the latest commit:
    https://github.com/martinetd/liburing/commits/getdents
    - vfs_getdents split: add missing internal.h include
    - Link to v1: https://lore.kernel.org/r/20230422-uring-getdents-v1-0-14c1db36e98c@codewreck.org

    ---
    Dominique Martinet (6):
    fs: split off vfs_getdents function of getdents64 syscall
    vfs_getdents/struct dir_context: add flags field
    io_uring: add support for getdents
    kernfs: implement readdir FMODE_NOWAIT
    libfs: set FMODE_NOWAIT on dir open
    RFC: io_uring getdents: test returning an EOF flag in CQE

    fs/internal.h | 8 ++++++
    fs/kernfs/dir.c | 21 ++++++++++++++-
    fs/libfs.c | 10 ++++---
    fs/readdir.c | 38 +++++++++++++++++++++------
    include/linux/fs.h | 10 +++++++
    include/uapi/linux/io_uring.h | 9 +++++++
    io_uring/fs.c | 61 +++++++++++++++++++++++++++++++++++++++++++
    io_uring/fs.h | 3 +++
    io_uring/opdef.c | 8 ++++++
    9 files changed, 156 insertions(+), 12 deletions(-)
    ---
    base-commit: 58390c8ce1bddb6c623f62e7ed36383e7fa5c02f
    change-id: 20230422-uring-getdents-2aab84d240aa

    Best regards,
    --
    Dominique Martinet | Asmadeus

    \
     
     \ /
      Last update: 2023-05-10 12:53    [W:2.779 / U:0.096 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site