public inbox for [email protected]
 help / color / mirror / Atom feed
From: Dominique Martinet <[email protected]>
To: Dave Chinner <[email protected]>
Cc: Alexander Viro <[email protected]>,
	Christian Brauner <[email protected]>,
	Jens Axboe <[email protected]>,
	Pavel Begunkov <[email protected]>,
	Stefan Roesch <[email protected]>,
	[email protected], [email protected],
	[email protected]
Subject: Re: [PATCH RFC 2/2] io_uring: add support for getdents
Date: Mon, 24 Apr 2023 08:43:00 +0900	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

Dave Chinner wrote on Mon, Apr 24, 2023 at 08:40:45AM +1000:
> This doesn't actually introduce non-blocking getdents operations, so
> what's the point? If it just shuffles the getdents call off to a
> background thread, why bother with io_uring in the first place?

As said in the cover letter my main motivation really is simplifying the
userspace application:
 - style-wise, mixing in plain old getdents(2) or readdir(3) in the
middle of an io_uring handling loop just feels wrong; but this may just
be my OCD talking.
 - in my understanding io_uring has its own thread pool, so even if the
actual getdents is blocking other IOs can progress (assuming there is
less blocked getdents than threads), without having to build one's own
extra thread pool next to the uring handling.
Looking at io_uring/fs.c the other "metadata related" calls there also
use the synchronous APIs (renameat, unlinkat, mkdirat, symlinkat and
linkat all do), so I didn't think of that as a problem in itself.


> Filesystems like XFS can easily do non-blocking getdents calls - we
> just need the NOWAIT plumbing (like we added to the IO path with
> IOCB_NOWAIT) to tell the filesystem not to block on locks or IO.
> Indeed, filesystems often have async readahead built into their
> getdents paths (XFS does), so it seems to me that we really want
> non-blocking getdents to allow filesystems to take full advantage of
> doing work without blocking and then shuffling the remainder off to
> a background thread when it actually needs to wait for IO....

I believe that can be done without any change of this API, so that'll be
a very welcome addition when it is ready; I don't think the adding the
uring op should wait on this if we can agree a simple wrapper API is
good enough (or come up with a better one if someone has a Good Idea)

(looking at io_uring/rw.c for comparison, io_getdents() will "just" need
to be adjusted to issue an async req if IO_URING_F_NONBLOCK is set, and
the poll/retry logic sorted out)

Thanks,
-- 
Dominique Martinet | Asmadeus

  reply	other threads:[~2023-04-23 23:43 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-22  8:40 [PATCH RFC 0/2] io_uring: add getdents support, take 2 Dominique Martinet
2023-04-22  8:40 ` [PATCH RFC 1/2] fs: split off vfs_getdents function of getdents64 syscall Dominique Martinet
2023-04-22 10:34   ` Dominique Martinet
2023-04-22  8:40 ` [PATCH RFC 2/2] io_uring: add support for getdents Dominique Martinet
2023-04-23 22:40   ` Dave Chinner
2023-04-23 23:43     ` Dominique Martinet [this message]
2023-04-24  7:29       ` Clay Harris
2023-04-24  8:41         ` Dominique Martinet
2023-04-24  9:20           ` Clay Harris
2023-04-24 10:55             ` Dominique Martinet
2023-04-28  5:06       ` Dave Chinner
2023-04-28  6:14         ` Dominique Martinet
2023-04-28 11:27           ` Dominique Martinet
2023-04-30 23:15             ` Dave Chinner
2023-04-29  8:07           ` Dominique Martinet
2023-04-30 23:32             ` Dave Chinner
2023-05-01  0:49               ` Dominique Martinet
2023-05-01  7:16                 ` Dave Chinner

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 \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /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