Hi Christian and Dominique,
On 7/13/23 00:02, Dominique Martinet wrote:
(replying as that was my code)
Christian Brauner wrote on Wed, Jul 12, 2023 at 01:31:57PM +0200:
On Tue, Jul 11, 2023 at 07:40:26PM +0800, Hao Xu wrote:
diff --git a/fs/readdir.c b/fs/readdir.c
index 9592259b7e7f..b80caf4c9321 100644
--- a/fs/readdir.c
+++ b/fs/readdir.c
@@ -358,12 +358,14 @@ static bool filldir64(struct dir_context *ctx, const char *name, int namlen,
* @file : pointer to file struct of directory
* @dirent : pointer to user directory structure
* @count : size of buffer
+ * @flags : additional dir_context flags
Why do you need that flag argument. The ->iterate{_shared}() i_op gets
passed the file so the filesystem can check
@file->f_mode & FMODE_NOWAIT, no?
As far as I understand it, it's not because the fd is capable of NOWAIT
that uring will call it in NOWAIT mode:
- if the first getdents call returned -EAGAIN it'll also fall back to
waiting in a separate thread (there's no "getdents poll" implementation,
so there's no other way of rescheduling a non-blocking call)
- it's also possible for the user to specify it wants IOSQE_ASYNC in the
sqe->flags (admitedly I'm not sure why would anyone do this, but that's
useful for benchmarks at least -- it skips the initial NOWAIT call
before falling back to threaded waiting call)
Even outsides of io_uring, a call to getdents64 should block, so even if
the filesystem supports non-blocking it should be explicitely required
by the caller.
Hi Christian,
My understanding of FMODE_NOWAIT is "this file support nowait IO". Just
like what Dominique
said, io_uring issue a request two rounds(let's simplify it here since
no apoll or task work involved),
and the first round is a nowait/nonblock try, the second one is an
offload-ed block try. So besides
a "ability" flag(FMODE_NOWAIT), we still need a "one-round" flag to
point out that "we do need to
do nowait IO this time".
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1719,8 +1719,16 @@ typedef bool (*filldir_t)(struct dir_context *, const char *, int, loff_t, u64,
struct dir_context {
filldir_t actor;
loff_t pos;
+ unsigned long flags;
};
+/*
+ * flags for dir_context flags
+ * DIR_CONTEXT_F_NOWAIT: Request non-blocking iterate
+ * (requires file->f_mode & FMODE_NOWAIT)
+ */
+#define DIR_CONTEXT_F_NOWAIT (1 << 0)
Even if this should be needed, I don't think this needs to use a full
flags field.
I also got a request to somehow pass back "are there more entries to
read after this call" to the caller in my v1, and I had done this as a
second flag -- in general my understanding was that it's better to add
flags than a specific boolean for extensibility but I have no opinon
here.
I've no strong opinion here, I kept it here as a flag variable to make it
more extendable in the future.
Thanks,
Hao
