[PATCH] fuse: Fix the condition to enable over-io-uring
Bernd Schubert <[email protected]>
| Newsgroups | org.kernel.feeds.b4-sent,dev.linux.lists.fuse-devel |
|---|---|
| Message-ID | <[email protected]> |
I had accidentally used "&&" instead of "||" and totally broken
the intention of the condition. This has several implications
1) fuse-servers can bypass the module option "enable_uring"
by setting FUSE_OVER_IO_URING in the FUSE_INIT reply, although
fuse-client/kernel wouldn't have announced availability of
that flag. I.e.
2) fuse-servers do not need to set FUSE_OVER_IO_URING in FUSE_INIT
reply and could just send FUSE_IO_URING_CMD_REGISTER at
any time while IO is already going on. That has further
implications:
2.1) The barrier in fuse_block_alloc() waiting for fuse_uring_ready()
is then bypassed - fuse requests are not blocked until queues
are initialized - results in dynamic switching from /dev/fuse
to io-uring.
2.2) The dynamic switching from /dev/fuse to io-uring exposes
at least one easily exposable lock order issue between
fch->bg_lock and queue->lock. There might more issues, as
dynamic run time switching was never tested.
The change itself is simple, but brings behavior change,
FUSE_OVER_IO_URING has to be set in the FUSE_INIT_REPLY by fuse servers
to accept any IORING_OP_URING_CMD. Libfuse does that and the only
non-libfuse implementation I found (fractal-fuse) also does that.
Qemu patches for fuse-io-uring are not merged yet, as far as I know.
Also moved up is the check for fch->initialized as a fuse-server
implementation might try to setup io-uring before FUSE_INIT is
processed and might have gotten -EOPNOTSUPP instead of -EAGAIN.
Also fixed is a stale comment that explain the handling of the
FUSE_OVER_IO_URING flag in early RFC versions.
If there should be a report from any library or application we
probably need to revert this commit, although slightly modified then
to prevent bypassing of the module option.
Signed-off-by: Bernd Schubert <[email protected]>
---
fs/fuse/dev_uring.c | 16 ++++++++--------
fs/fuse/inode.c | 4 ----
2 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
index 77c8cec43d9c..eba322f79df1 100644
--- a/fs/fuse/dev_uring.c
+++ b/fs/fuse/dev_uring.c
@@ -1237,8 +1237,15 @@ int fuse_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
}
fch = fud->chan;
+ /*
+ * fuse_uring_register() needs the ring to be initialized,
+ * we need to know the max payload size
+ */
+ if (!fch->initialized)
+ return -EAGAIN;
+
/* Once a connection has io-uring enabled on it, it can't be disabled */
- if (!enable_uring && !fch->io_uring) {
+ if (!enable_uring || !fch->io_uring) {
pr_info_ratelimited("fuse-io-uring is disabled\n");
return -EOPNOTSUPP;
}
@@ -1248,13 +1255,6 @@ int fuse_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
if (!fch->connected)
return -ENOTCONN;
- /*
- * fuse_uring_register() needs the ring to be initialized,
- * we need to know the max payload size
- */
- if (!fch->initialized)
- return -EAGAIN;
-
switch (cmd_op) {
case FUSE_IO_URING_CMD_REGISTER:
err = fuse_uring_register(cmd, issue_flags, fch);
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index d975073c6029..f29bd7432fa3 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1474,10 +1474,6 @@ static struct fuse_init_args *fuse_new_init(struct fuse_mount *fm)
if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
flags |= FUSE_PASSTHROUGH;
- /*
- * This is just an information flag for fuse server. No need to check
- * the reply - server is either sending IORING_OP_URING_CMD or not.
- */
if (fuse_uring_enabled())
flags |= FUSE_OVER_IO_URING;
---
base-commit: 818bebeb63dd6bf5f4e07e145f6cdbace520a34c
change-id: 20260821-fuse-fix-enable-condition-df87aa4c25c8
Best regards,
--
Bernd Schubert <[email protected]>