* Re: [PATCH v2] fuse: allow synchronous FUSE_INIT
2025-08-27 10:59 [PATCH v2] fuse: allow synchronous FUSE_INIT Miklos Szeredi
@ 2025-08-27 19:02 ` Darrick J. Wong
2025-08-27 19:51 ` Miklos Szeredi
2025-08-27 22:56 ` Joanne Koong
2025-08-29 15:43 ` Darrick J. Wong
2 siblings, 1 reply; 11+ messages in thread
From: Darrick J. Wong @ 2025-08-27 19:02 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: linux-fsdevel, Joanne Koong, John Groves, Bernd Schubert
On Wed, Aug 27, 2025 at 12:59:55PM +0200, Miklos Szeredi wrote:
> FUSE_INIT has always been asynchronous with mount. That means that the
> server processed this request after the mount syscall returned.
>
> This means that FUSE_INIT can't supply the root inode's ID, hence it
> currently has a hardcoded value. There are other limitations such as not
> being able to perform getxattr during mount, which is needed by selinux.
>
> To remove these limitations allow server to process FUSE_INIT while
> initializing the in-core super block for the fuse filesystem. This can
> only be done if the server is prepared to handle this, so add
> FUSE_DEV_IOC_SYNC_INIT ioctl, which
>
> a) lets the server know whether this feature is supported, returning
> ENOTTY othewrwise.
>
> b) lets the kernel know to perform a synchronous initialization
>
> The implementation is slightly tricky, since fuse_dev/fuse_conn are set up
> only during super block creation. This is solved by setting the private
> data of the fuse device file to a special value ((struct fuse_dev *) 1) and
> waiting for this to be turned into a proper fuse_dev before commecing with
> operations on the device file.
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
> v2:
>
> - make fuse_send_init() perform sync/async sequence based on fc->sync_init
> (Joanne)
>
> fs/fuse/cuse.c | 3 +-
> fs/fuse/dev.c | 74 +++++++++++++++++++++++++++++----------
> fs/fuse/dev_uring.c | 4 +--
> fs/fuse/fuse_dev_i.h | 13 +++++--
> fs/fuse/fuse_i.h | 5 ++-
> fs/fuse/inode.c | 50 ++++++++++++++++++++------
> include/uapi/linux/fuse.h | 1 +
> 7 files changed, 115 insertions(+), 35 deletions(-)
>
> diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c
> index b39844d75a80..28c96961e85d 100644
> --- a/fs/fuse/cuse.c
> +++ b/fs/fuse/cuse.c
> @@ -52,6 +52,7 @@
> #include <linux/user_namespace.h>
>
> #include "fuse_i.h"
> +#include "fuse_dev_i.h"
>
> #define CUSE_CONNTBL_LEN 64
>
> @@ -547,7 +548,7 @@ static int cuse_channel_open(struct inode *inode, struct file *file)
> */
> static int cuse_channel_release(struct inode *inode, struct file *file)
> {
> - struct fuse_dev *fud = file->private_data;
> + struct fuse_dev *fud = __fuse_get_dev(file);
> struct cuse_conn *cc = fc_to_cc(fud->fc);
>
> /* remove from the conntbl, no more access from this point on */
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 8ac074414897..948f45c6e0ef 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -1530,14 +1530,34 @@ static int fuse_dev_open(struct inode *inode, struct file *file)
> return 0;
> }
>
> +struct fuse_dev *fuse_get_dev(struct file *file)
> +{
> + struct fuse_dev *fud = __fuse_get_dev(file);
> + int err;
> +
> + if (likely(fud))
> + return fud;
> +
> + err = wait_event_interruptible(fuse_dev_waitq,
> + READ_ONCE(file->private_data) != FUSE_DEV_SYNC_INIT);
> + if (err)
> + return ERR_PTR(err);
> +
> + fud = __fuse_get_dev(file);
> + if (!fud)
> + return ERR_PTR(-EPERM);
> +
> + return fud;
> +}
> +
> static ssize_t fuse_dev_read(struct kiocb *iocb, struct iov_iter *to)
> {
> struct fuse_copy_state cs;
> struct file *file = iocb->ki_filp;
> struct fuse_dev *fud = fuse_get_dev(file);
>
> - if (!fud)
> - return -EPERM;
> + if (IS_ERR(fud))
> + return PTR_ERR(fud);
>
> if (!user_backed_iter(to))
> return -EINVAL;
> @@ -1557,8 +1577,8 @@ static ssize_t fuse_dev_splice_read(struct file *in, loff_t *ppos,
> struct fuse_copy_state cs;
> struct fuse_dev *fud = fuse_get_dev(in);
>
> - if (!fud)
> - return -EPERM;
> + if (IS_ERR(fud))
> + return PTR_ERR(fud);
>
> bufs = kvmalloc_array(pipe->max_usage, sizeof(struct pipe_buffer),
> GFP_KERNEL);
> @@ -2233,8 +2253,8 @@ static ssize_t fuse_dev_write(struct kiocb *iocb, struct iov_iter *from)
> struct fuse_copy_state cs;
> struct fuse_dev *fud = fuse_get_dev(iocb->ki_filp);
>
> - if (!fud)
> - return -EPERM;
> + if (IS_ERR(fud))
> + return PTR_ERR(fud);
>
> if (!user_backed_iter(from))
> return -EINVAL;
> @@ -2258,8 +2278,8 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe,
> ssize_t ret;
>
> fud = fuse_get_dev(out);
> - if (!fud)
> - return -EPERM;
> + if (IS_ERR(fud))
> + return PTR_ERR(fud);
>
> pipe_lock(pipe);
>
> @@ -2343,7 +2363,7 @@ static __poll_t fuse_dev_poll(struct file *file, poll_table *wait)
> struct fuse_iqueue *fiq;
> struct fuse_dev *fud = fuse_get_dev(file);
>
> - if (!fud)
> + if (IS_ERR(fud))
> return EPOLLERR;
>
> fiq = &fud->fc->iq;
> @@ -2490,7 +2510,7 @@ void fuse_wait_aborted(struct fuse_conn *fc)
>
> int fuse_dev_release(struct inode *inode, struct file *file)
> {
> - struct fuse_dev *fud = fuse_get_dev(file);
> + struct fuse_dev *fud = __fuse_get_dev(file);
>
> if (fud) {
> struct fuse_conn *fc = fud->fc;
> @@ -2521,8 +2541,8 @@ static int fuse_dev_fasync(int fd, struct file *file, int on)
> {
> struct fuse_dev *fud = fuse_get_dev(file);
>
> - if (!fud)
> - return -EPERM;
> + if (IS_ERR(fud))
> + return PTR_ERR(fud);
>
> /* No locking - fasync_helper does its own locking */
> return fasync_helper(fd, file, on, &fud->fc->iq.fasync);
> @@ -2532,7 +2552,7 @@ static int fuse_device_clone(struct fuse_conn *fc, struct file *new)
> {
> struct fuse_dev *fud;
>
> - if (new->private_data)
> + if (__fuse_get_dev(new))
> return -EINVAL;
>
> fud = fuse_dev_alloc_install(fc);
> @@ -2563,7 +2583,7 @@ static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
> * uses the same ioctl handler.
> */
> if (fd_file(f)->f_op == file->f_op)
> - fud = fuse_get_dev(fd_file(f));
> + fud = __fuse_get_dev(fd_file(f));
>
> res = -EINVAL;
> if (fud) {
> @@ -2581,8 +2601,8 @@ static long fuse_dev_ioctl_backing_open(struct file *file,
> struct fuse_dev *fud = fuse_get_dev(file);
> struct fuse_backing_map map;
>
> - if (!fud)
> - return -EPERM;
> + if (IS_ERR(fud))
> + return PTR_ERR(fud);
>
> if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
> return -EOPNOTSUPP;
> @@ -2598,8 +2618,8 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp)
> struct fuse_dev *fud = fuse_get_dev(file);
> int backing_id;
>
> - if (!fud)
> - return -EPERM;
> + if (IS_ERR(fud))
> + return PTR_ERR(fud);
>
> if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
> return -EOPNOTSUPP;
> @@ -2610,6 +2630,19 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp)
> return fuse_backing_close(fud->fc, backing_id);
> }
>
> +static long fuse_dev_ioctl_sync_init(struct file *file)
As I noted in a reply to the v1 patch, I /think/ I'm going to want to
have a privileged mount helper open /dev/fuse and pass that fd to a
completely unprivileged helper. For privileged functionality (like
iomap) I think it would be useful for the mount helper to be able to
attach that iomap capability to the fusedev so that the fuse server can
ask if may use iomap before FUSE_INIT time.
IOWs, would you be willing to rename this to FUSE_DEV_IOC_SET_FEATURE
and take a u32 code? Then programs with an open /dev/fuse fd can
incrementally add pieces as required. The first one would be
FUSE_DEV_SYNC_INIT, the second one would be FUSE_DEV_ADD_IOMAP, etc.
(I don't mind adding another ioctl number for fuse/iomap, but this would
be an opportunity to add fewer ioctl numbers.)
> +{
> + int err = -EINVAL;
> +
> + mutex_lock(&fuse_mutex);
> + if (!__fuse_get_dev(file)) {
> + WRITE_ONCE(file->private_data, FUSE_DEV_SYNC_INIT);
> + err = 0;
> + }
> + mutex_unlock(&fuse_mutex);
> + return err;
> +}
> +
> static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
> unsigned long arg)
> {
> @@ -2625,6 +2658,9 @@ static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
> case FUSE_DEV_IOC_BACKING_CLOSE:
> return fuse_dev_ioctl_backing_close(file, argp);
>
> + case FUSE_DEV_IOC_SYNC_INIT:
> + return fuse_dev_ioctl_sync_init(file);
> +
> default:
> return -ENOTTY;
> }
> @@ -2633,7 +2669,7 @@ static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
> #ifdef CONFIG_PROC_FS
> static void fuse_dev_show_fdinfo(struct seq_file *seq, struct file *file)
> {
> - struct fuse_dev *fud = fuse_get_dev(file);
> + struct fuse_dev *fud = __fuse_get_dev(file);
> if (!fud)
> return;
>
> diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
> index 249b210becb1..bef38ed78249 100644
> --- a/fs/fuse/dev_uring.c
> +++ b/fs/fuse/dev_uring.c
> @@ -1139,9 +1139,9 @@ int fuse_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
> return -EINVAL;
>
> fud = fuse_get_dev(cmd->file);
> - if (!fud) {
> + if (IS_ERR(fud)) {
> pr_info_ratelimited("No fuse device found\n");
> - return -ENOTCONN;
> + return PTR_ERR(fud);
> }
> fc = fud->fc;
>
> diff --git a/fs/fuse/fuse_dev_i.h b/fs/fuse/fuse_dev_i.h
> index 5a9bd771a319..6e8373f97040 100644
> --- a/fs/fuse/fuse_dev_i.h
> +++ b/fs/fuse/fuse_dev_i.h
> @@ -12,6 +12,8 @@
> #define FUSE_INT_REQ_BIT (1ULL << 0)
> #define FUSE_REQ_ID_STEP (1ULL << 1)
>
> +extern struct wait_queue_head fuse_dev_waitq;
> +
> struct fuse_arg;
> struct fuse_args;
> struct fuse_pqueue;
> @@ -37,15 +39,22 @@ struct fuse_copy_state {
> } ring;
> };
>
> -static inline struct fuse_dev *fuse_get_dev(struct file *file)
> +#define FUSE_DEV_SYNC_INIT ((struct fuse_dev *) 1)
> +#define FUSE_DEV_PTR_MASK (~1UL)
> +
> +static inline struct fuse_dev *__fuse_get_dev(struct file *file)
> {
> /*
> * Lockless access is OK, because file->private data is set
> * once during mount and is valid until the file is released.
> */
> - return READ_ONCE(file->private_data);
> + struct fuse_dev *fud = READ_ONCE(file->private_data);
> +
> + return (typeof(fud)) ((unsigned long) fud & FUSE_DEV_PTR_MASK);
s/unsigned long/uintptr_t/ here ?
> }
>
> +struct fuse_dev *fuse_get_dev(struct file *file);
> +
> unsigned int fuse_req_hash(u64 unique);
> struct fuse_req *fuse_request_find(struct fuse_pqueue *fpq, u64 unique);
>
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 486fa550c951..233c6111f768 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -904,6 +904,9 @@ struct fuse_conn {
> /* Is link not implemented by fs? */
> unsigned int no_link:1;
>
> + /* Is synchronous FUSE_INIT allowed? */
> + unsigned int sync_init:1;
> +
> /* Use io_uring for communication */
> unsigned int io_uring;
>
> @@ -1318,7 +1321,7 @@ struct fuse_dev *fuse_dev_alloc_install(struct fuse_conn *fc);
> struct fuse_dev *fuse_dev_alloc(void);
> void fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc);
> void fuse_dev_free(struct fuse_dev *fud);
> -void fuse_send_init(struct fuse_mount *fm);
> +int fuse_send_init(struct fuse_mount *fm);
>
> /**
> * Fill in superblock and initialize fuse connection
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 9d26a5bc394d..7cf47d5bcc87 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -7,6 +7,7 @@
> */
>
> #include "fuse_i.h"
> +#include "fuse_dev_i.h"
> #include "dev_uring_i.h"
>
> #include <linux/dax.h>
> @@ -34,6 +35,7 @@ MODULE_LICENSE("GPL");
> static struct kmem_cache *fuse_inode_cachep;
> struct list_head fuse_conn_list;
> DEFINE_MUTEX(fuse_mutex);
> +DECLARE_WAIT_QUEUE_HEAD(fuse_dev_waitq);
>
> static int set_global_limit(const char *val, const struct kernel_param *kp);
>
> @@ -1466,7 +1468,7 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
> wake_up_all(&fc->blocked_waitq);
> }
>
> -void fuse_send_init(struct fuse_mount *fm)
> +static struct fuse_init_args *fuse_new_init(struct fuse_mount *fm)
> {
> struct fuse_init_args *ia;
> u64 flags;
> @@ -1525,10 +1527,29 @@ void fuse_send_init(struct fuse_mount *fm)
> ia->args.out_args[0].value = &ia->out;
> ia->args.force = true;
> ia->args.nocreds = true;
> - ia->args.end = process_init_reply;
>
> - if (fuse_simple_background(fm, &ia->args, GFP_KERNEL) != 0)
> - process_init_reply(fm, &ia->args, -ENOTCONN);
> + return ia;
> +}
> +
> +int fuse_send_init(struct fuse_mount *fm)
> +{
> + struct fuse_init_args *ia = fuse_new_init(fm);
> + int err;
> +
> + if (fm->fc->sync_init) {
> + err = fuse_simple_request(fm, &ia->args);
> + /* Ignore size of init reply */
> + if (err > 0)
> + err = 0;
> + } else {
> + ia->args.end = process_init_reply;
> + err = fuse_simple_background(fm, &ia->args, GFP_KERNEL);
> + if (!err)
> + return 0;
> + err = -ENOTCONN;
> + }
> + process_init_reply(fm, &ia->args, err);
> + return err;
> }
> EXPORT_SYMBOL_GPL(fuse_send_init);
>
> @@ -1867,8 +1888,12 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
>
> mutex_lock(&fuse_mutex);
> err = -EINVAL;
> - if (ctx->fudptr && *ctx->fudptr)
> - goto err_unlock;
> + if (ctx->fudptr && *ctx->fudptr) {
> + if (*ctx->fudptr == FUSE_DEV_SYNC_INIT) {
> + fc->sync_init = 1;
> + } else
> + goto err_unlock;
> + }
>
> err = fuse_ctl_add_conn(fc);
> if (err)
> @@ -1876,8 +1901,10 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
>
> list_add_tail(&fc->entry, &fuse_conn_list);
> sb->s_root = root_dentry;
> - if (ctx->fudptr)
> + if (ctx->fudptr) {
> *ctx->fudptr = fud;
> + wake_up_all(&fuse_dev_waitq);
> + }
> mutex_unlock(&fuse_mutex);
> return 0;
>
> @@ -1898,6 +1925,7 @@ EXPORT_SYMBOL_GPL(fuse_fill_super_common);
> static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
> {
> struct fuse_fs_context *ctx = fsc->fs_private;
> + struct fuse_mount *fm;
> int err;
>
> if (!ctx->file || !ctx->rootmode_present ||
> @@ -1918,8 +1946,10 @@ static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
> return err;
> /* file->private_data shall be visible on all CPUs after this */
> smp_mb();
> - fuse_send_init(get_fuse_mount_super(sb));
> - return 0;
> +
> + fm = get_fuse_mount_super(sb);
> +
> + return fuse_send_init(fm);
If process_init_reply hits the ok==false case and clears fc->conn_init,
should this return an error here to abort the mount?
--D
> }
>
> /*
> @@ -1980,7 +2010,7 @@ static int fuse_get_tree(struct fs_context *fsc)
> * Allow creating a fuse mount with an already initialized fuse
> * connection
> */
> - fud = READ_ONCE(ctx->file->private_data);
> + fud = __fuse_get_dev(ctx->file);
> if (ctx->file->f_op == &fuse_dev_operations && fud) {
> fsc->sget_key = fud->fc;
> sb = sget_fc(fsc, fuse_test_super, fuse_set_no_super);
> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> index 3942d1fda599..30bf0846547f 100644
> --- a/include/uapi/linux/fuse.h
> +++ b/include/uapi/linux/fuse.h
> @@ -1130,6 +1130,7 @@ struct fuse_backing_map {
> #define FUSE_DEV_IOC_BACKING_OPEN _IOW(FUSE_DEV_IOC_MAGIC, 1, \
> struct fuse_backing_map)
> #define FUSE_DEV_IOC_BACKING_CLOSE _IOW(FUSE_DEV_IOC_MAGIC, 2, uint32_t)
> +#define FUSE_DEV_IOC_SYNC_INIT _IO(FUSE_DEV_IOC_MAGIC, 3)
>
> struct fuse_lseek_in {
> uint64_t fh;
> --
> 2.49.0
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2] fuse: allow synchronous FUSE_INIT
2025-08-27 19:02 ` Darrick J. Wong
@ 2025-08-27 19:51 ` Miklos Szeredi
2025-08-27 20:26 ` Darrick J. Wong
0 siblings, 1 reply; 11+ messages in thread
From: Miklos Szeredi @ 2025-08-27 19:51 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Miklos Szeredi, linux-fsdevel, Joanne Koong, John Groves,
Bernd Schubert
On Wed, 27 Aug 2025 at 21:02, Darrick J. Wong <djwong@kernel.org> wrote:
> IOWs, would you be willing to rename this to FUSE_DEV_IOC_SET_FEATURE
> and take a u32 code? Then programs with an open /dev/fuse fd can
> incrementally add pieces as required. The first one would be
> FUSE_DEV_SYNC_INIT, the second one would be FUSE_DEV_ADD_IOMAP, etc.
Okay, so this is not a mask, and individual features would need to be
set with separate ioctl calls, right?
That would allow negotiating features.
> > + return (typeof(fud)) ((unsigned long) fud & FUSE_DEV_PTR_MASK);
>
> s/unsigned long/uintptr_t/ here ?
Okay.
> If process_init_reply hits the ok==false case and clears fc->conn_init,
> should this return an error here to abort the mount?
Yes, fixed.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] fuse: allow synchronous FUSE_INIT
2025-08-27 19:51 ` Miklos Szeredi
@ 2025-08-27 20:26 ` Darrick J. Wong
0 siblings, 0 replies; 11+ messages in thread
From: Darrick J. Wong @ 2025-08-27 20:26 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Miklos Szeredi, linux-fsdevel, Joanne Koong, John Groves,
Bernd Schubert
On Wed, Aug 27, 2025 at 09:51:04PM +0200, Miklos Szeredi wrote:
> On Wed, 27 Aug 2025 at 21:02, Darrick J. Wong <djwong@kernel.org> wrote:
>
> > IOWs, would you be willing to rename this to FUSE_DEV_IOC_SET_FEATURE
> > and take a u32 code? Then programs with an open /dev/fuse fd can
> > incrementally add pieces as required. The first one would be
> > FUSE_DEV_SYNC_INIT, the second one would be FUSE_DEV_ADD_IOMAP, etc.
>
> Okay, so this is not a mask, and individual features would need to be
> set with separate ioctl calls, right?
>
> That would allow negotiating features.
Correct, one feature per ioctl call.
> > > + return (typeof(fud)) ((unsigned long) fud & FUSE_DEV_PTR_MASK);
> >
> > s/unsigned long/uintptr_t/ here ?
>
> Okay.
>
> > If process_init_reply hits the ok==false case and clears fc->conn_init,
> > should this return an error here to abort the mount?
>
> Yes, fixed.
Cool, thanks!
FWIW when I added the second FUSE_DEV_ mask, I ended up rewiring the
code like this:
#define FUSE_DEV_SYNC_INIT (1UL << 0)
#define FUSE_DEV_INHERIT_IOMAP (1UL << 1)
#define FUSE_DEV_FLAGS_MASK (FUSE_DEV_SYNC_INIT | FUSE_DEV_INHERIT_IOMAP)
#define FUSE_DEV_PTR_MASK (~FUSE_DEV_FLAGS_MASK)
and adding helpers to take care of all the annoying casts:
static inline void __fuse_set_dev_flags(struct file *file, uintptr_t flag)
{
uintptr_t old_flags =
(uintptr_t)READ_ONCE(file->private_data) & FUSE_DEV_FLAGS_MASK;
WRITE_ONCE(file->private_data,
(struct fuse_dev *)(old_flags | flag));
}
and fuse_fill_super_common ends up looking like this:
if (ctx->fudptr) {
uintptr_t raw = (uintptr_t)(*ctx->fudptr);
uintptr_t flags = raw & FUSE_DEV_FLAGS_MASK;
if (raw & FUSE_DEV_PTR_MASK)
goto err_unlock;
if (flags & FUSE_DEV_SYNC_INIT)
fc->sync_init = 1;
if (flags & FUSE_DEV_INHERIT_IOMAP)
fc->may_iomap = 1;
}
--D
> Thanks,
> Miklos
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] fuse: allow synchronous FUSE_INIT
2025-08-27 10:59 [PATCH v2] fuse: allow synchronous FUSE_INIT Miklos Szeredi
2025-08-27 19:02 ` Darrick J. Wong
@ 2025-08-27 22:56 ` Joanne Koong
2025-08-28 0:36 ` Darrick J. Wong
2025-08-28 13:09 ` Miklos Szeredi
2025-08-29 15:43 ` Darrick J. Wong
2 siblings, 2 replies; 11+ messages in thread
From: Joanne Koong @ 2025-08-27 22:56 UTC (permalink / raw)
To: Miklos Szeredi
Cc: linux-fsdevel, Darrick J. Wong, John Groves, Bernd Schubert
On Wed, Aug 27, 2025 at 4:00 AM Miklos Szeredi <mszeredi@redhat.com> wrote:
>
> FUSE_INIT has always been asynchronous with mount. That means that the
> server processed this request after the mount syscall returned.
>
> This means that FUSE_INIT can't supply the root inode's ID, hence it
> currently has a hardcoded value. There are other limitations such as not
> being able to perform getxattr during mount, which is needed by selinux.
>
> To remove these limitations allow server to process FUSE_INIT while
> initializing the in-core super block for the fuse filesystem. This can
> only be done if the server is prepared to handle this, so add
> FUSE_DEV_IOC_SYNC_INIT ioctl, which
>
> a) lets the server know whether this feature is supported, returning
> ENOTTY othewrwise.
>
> b) lets the kernel know to perform a synchronous initialization
>
> The implementation is slightly tricky, since fuse_dev/fuse_conn are set up
> only during super block creation. This is solved by setting the private
> data of the fuse device file to a special value ((struct fuse_dev *) 1) and
> waiting for this to be turned into a proper fuse_dev before commecing with
> operations on the device file.
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
> v2:
>
> - make fuse_send_init() perform sync/async sequence based on fc->sync_init
> (Joanne)
>
> fs/fuse/cuse.c | 3 +-
> fs/fuse/dev.c | 74 +++++++++++++++++++++++++++++----------
> fs/fuse/dev_uring.c | 4 +--
> fs/fuse/fuse_dev_i.h | 13 +++++--
> fs/fuse/fuse_i.h | 5 ++-
> fs/fuse/inode.c | 50 ++++++++++++++++++++------
> include/uapi/linux/fuse.h | 1 +
> 7 files changed, 115 insertions(+), 35 deletions(-)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 8ac074414897..948f45c6e0ef 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -1530,14 +1530,34 @@ static int fuse_dev_open(struct inode *inode, struct file *file)
> return 0;
> }
>
> +struct fuse_dev *fuse_get_dev(struct file *file)
> +{
> + struct fuse_dev *fud = __fuse_get_dev(file);
> + int err;
> +
> + if (likely(fud))
> + return fud;
> +
> + err = wait_event_interruptible(fuse_dev_waitq,
> + READ_ONCE(file->private_data) != FUSE_DEV_SYNC_INIT);
I wonder if we should make the semantics the same for synchronous and
non-synchronous inits here, i.e. doing a wait for
"(READ_ONCE(file->private_data) != FUSE_DEV_SYNC_INIT) &&
READ_ONCE(file->private_data) != NULL", so that from the libfuse point
of view, the flow can be unified between the two, eg
i) send sync_init ioctl call if doing a synchronous init
ii) kick off thread to read requests
iii) do mount call
otherwise for async inits, the mount call needs to happen first.
> + if (err)
> + return ERR_PTR(err);
> +
> + fud = __fuse_get_dev(file);
> + if (!fud)
> + return ERR_PTR(-EPERM);
> +
> + return fud;
> +}
> +
> static ssize_t fuse_dev_read(struct kiocb *iocb, struct iov_iter *to)
> {
> struct fuse_copy_state cs;
> struct file *file = iocb->ki_filp;
> struct fuse_dev *fud = fuse_get_dev(file);
>
> - if (!fud)
> - return -EPERM;
> + if (IS_ERR(fud))
> + return PTR_ERR(fud);
>
> if (!user_backed_iter(to))
> return -EINVAL;
> @@ -1557,8 +1577,8 @@ static ssize_t fuse_dev_splice_read(struct file *in, loff_t *ppos,
> struct fuse_copy_state cs;
> struct fuse_dev *fud = fuse_get_dev(in);
>
> - if (!fud)
> - return -EPERM;
> + if (IS_ERR(fud))
> + return PTR_ERR(fud);
>
> bufs = kvmalloc_array(pipe->max_usage, sizeof(struct pipe_buffer),
> GFP_KERNEL);
> @@ -2233,8 +2253,8 @@ static ssize_t fuse_dev_write(struct kiocb *iocb, struct iov_iter *from)
> struct fuse_copy_state cs;
> struct fuse_dev *fud = fuse_get_dev(iocb->ki_filp);
Does this (and below in fuse_dev_splice_write()) need to be
fuse_get_dev()? afaict, fuse_dev_write() only starts getting used
after fud has already been initialized. i see why it's needed for
fuse_dev_read() since otherwise the server doesn't know when it can
start calling fuse_dev_read(), but for fuse_dev_write(), it seems like
that only gets used after fud is already initialized.
>
> - if (!fud)
> - return -EPERM;
> + if (IS_ERR(fud))
> + return PTR_ERR(fud);
>
> if (!user_backed_iter(from))
> return -EINVAL;
> @@ -2258,8 +2278,8 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe,
> ssize_t ret;
>
> fud = fuse_get_dev(out);
> - if (!fud)
> - return -EPERM;
> + if (IS_ERR(fud))
> + return PTR_ERR(fud);
>
> pipe_lock(pipe);
>
> @@ -2581,8 +2601,8 @@ static long fuse_dev_ioctl_backing_open(struct file *file,
> struct fuse_dev *fud = fuse_get_dev(file);
Should this be __fuse_get_dev()?
> struct fuse_backing_map map;
>
> - if (!fud)
> - return -EPERM;
> + if (IS_ERR(fud))
> + return PTR_ERR(fud);
>
> if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
> return -EOPNOTSUPP;
> @@ -2598,8 +2618,8 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp)
> struct fuse_dev *fud = fuse_get_dev(file);
Same question here.
> int backing_id;
>
> - if (!fud)
> - return -EPERM;
> + if (IS_ERR(fud))
> + return PTR_ERR(fud);
>
> if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
> return -EOPNOTSUPP;
> @@ -2610,6 +2630,19 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp)
> return fuse_backing_close(fud->fc, backing_id);
> }
>
> +static long fuse_dev_ioctl_sync_init(struct file *file)
> +{
> + int err = -EINVAL;
> +
> + mutex_lock(&fuse_mutex);
> + if (!__fuse_get_dev(file)) {
> + WRITE_ONCE(file->private_data, FUSE_DEV_SYNC_INIT);
Does this still need a WRITE_ONCE if it's accessed within the scope of
the mutex? My understanding (maybe wrong) is that a mutex implicitly
serves as also a memory barrier. If not, then we probably also need a
WRITE_ONCE() around the *ctx->fudptr assignment in
fuse_fill_super_common()?
Thanks,
Joanne
> + err = 0;
> + }
> + mutex_unlock(&fuse_mutex);
> + return err;
> +}
> +
> @@ -1876,8 +1901,10 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
>
> list_add_tail(&fc->entry, &fuse_conn_list);
> sb->s_root = root_dentry;
> - if (ctx->fudptr)
> + if (ctx->fudptr) {
> *ctx->fudptr = fud;
> + wake_up_all(&fuse_dev_waitq);
> + }
> mutex_unlock(&fuse_mutex);
> return 0;
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2] fuse: allow synchronous FUSE_INIT
2025-08-27 22:56 ` Joanne Koong
@ 2025-08-28 0:36 ` Darrick J. Wong
2025-08-28 13:09 ` Miklos Szeredi
1 sibling, 0 replies; 11+ messages in thread
From: Darrick J. Wong @ 2025-08-28 0:36 UTC (permalink / raw)
To: Joanne Koong; +Cc: Miklos Szeredi, linux-fsdevel, John Groves, Bernd Schubert
On Wed, Aug 27, 2025 at 03:56:49PM -0700, Joanne Koong wrote:
> On Wed, Aug 27, 2025 at 4:00 AM Miklos Szeredi <mszeredi@redhat.com> wrote:
> >
> > FUSE_INIT has always been asynchronous with mount. That means that the
> > server processed this request after the mount syscall returned.
> >
> > This means that FUSE_INIT can't supply the root inode's ID, hence it
> > currently has a hardcoded value. There are other limitations such as not
> > being able to perform getxattr during mount, which is needed by selinux.
> >
> > To remove these limitations allow server to process FUSE_INIT while
> > initializing the in-core super block for the fuse filesystem. This can
> > only be done if the server is prepared to handle this, so add
> > FUSE_DEV_IOC_SYNC_INIT ioctl, which
> >
> > a) lets the server know whether this feature is supported, returning
> > ENOTTY othewrwise.
> >
> > b) lets the kernel know to perform a synchronous initialization
> >
> > The implementation is slightly tricky, since fuse_dev/fuse_conn are set up
> > only during super block creation. This is solved by setting the private
> > data of the fuse device file to a special value ((struct fuse_dev *) 1) and
> > waiting for this to be turned into a proper fuse_dev before commecing with
> > operations on the device file.
> >
> > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> > ---
> > v2:
> >
> > - make fuse_send_init() perform sync/async sequence based on fc->sync_init
> > (Joanne)
> >
> > fs/fuse/cuse.c | 3 +-
> > fs/fuse/dev.c | 74 +++++++++++++++++++++++++++++----------
> > fs/fuse/dev_uring.c | 4 +--
> > fs/fuse/fuse_dev_i.h | 13 +++++--
> > fs/fuse/fuse_i.h | 5 ++-
> > fs/fuse/inode.c | 50 ++++++++++++++++++++------
> > include/uapi/linux/fuse.h | 1 +
> > 7 files changed, 115 insertions(+), 35 deletions(-)
> >
> > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> > index 8ac074414897..948f45c6e0ef 100644
> > --- a/fs/fuse/dev.c
> > +++ b/fs/fuse/dev.c
> > @@ -1530,14 +1530,34 @@ static int fuse_dev_open(struct inode *inode, struct file *file)
> > return 0;
> > }
> >
> > +struct fuse_dev *fuse_get_dev(struct file *file)
> > +{
> > + struct fuse_dev *fud = __fuse_get_dev(file);
> > + int err;
> > +
> > + if (likely(fud))
> > + return fud;
> > +
> > + err = wait_event_interruptible(fuse_dev_waitq,
> > + READ_ONCE(file->private_data) != FUSE_DEV_SYNC_INIT);
>
> I wonder if we should make the semantics the same for synchronous and
> non-synchronous inits here, i.e. doing a wait for
> "(READ_ONCE(file->private_data) != FUSE_DEV_SYNC_INIT) &&
> READ_ONCE(file->private_data) != NULL", so that from the libfuse point
> of view, the flow can be unified between the two, eg
> i) send sync_init ioctl call if doing a synchronous init
> ii) kick off thread to read requests
> iii) do mount call
> otherwise for async inits, the mount call needs to happen first.
I don't think you can compare it against NULL directly, because
FUSE_DEV_SYNC_INIT != NULL evaluates to true.
How about
err = wait_event_interruptible(fuse_dev_waitq,
__fuse_get_dev(file) != NULL);
?
> > + if (err)
> > + return ERR_PTR(err);
> > +
> > + fud = __fuse_get_dev(file);
> > + if (!fud)
> > + return ERR_PTR(-EPERM);
> > +
> > + return fud;
> > +}
> > +
> > static ssize_t fuse_dev_read(struct kiocb *iocb, struct iov_iter *to)
> > {
> > struct fuse_copy_state cs;
> > struct file *file = iocb->ki_filp;
> > struct fuse_dev *fud = fuse_get_dev(file);
> >
> > - if (!fud)
> > - return -EPERM;
> > + if (IS_ERR(fud))
> > + return PTR_ERR(fud);
> >
> > if (!user_backed_iter(to))
> > return -EINVAL;
> > @@ -1557,8 +1577,8 @@ static ssize_t fuse_dev_splice_read(struct file *in, loff_t *ppos,
> > struct fuse_copy_state cs;
> > struct fuse_dev *fud = fuse_get_dev(in);
> >
> > - if (!fud)
> > - return -EPERM;
> > + if (IS_ERR(fud))
> > + return PTR_ERR(fud);
> >
> > bufs = kvmalloc_array(pipe->max_usage, sizeof(struct pipe_buffer),
> > GFP_KERNEL);
> > @@ -2233,8 +2253,8 @@ static ssize_t fuse_dev_write(struct kiocb *iocb, struct iov_iter *from)
> > struct fuse_copy_state cs;
> > struct fuse_dev *fud = fuse_get_dev(iocb->ki_filp);
>
> Does this (and below in fuse_dev_splice_write()) need to be
> fuse_get_dev()? afaict, fuse_dev_write() only starts getting used
> after fud has already been initialized. i see why it's needed for
> fuse_dev_read() since otherwise the server doesn't know when it can
> start calling fuse_dev_read(), but for fuse_dev_write(), it seems like
> that only gets used after fud is already initialized.
I think most of these functions could just do:
struct fuse_dev *fud = __fuse_get_dev(iocb->ki_filp);
if (!fud)
return -EPERM;
Just like the old days, but it's one churn (if test) vs. another
(callsite) type of churn. Either way, we want to error out of all of
these functions if you haven't actually mounted the fuse server to
create the fud, right?
> >
> > - if (!fud)
> > - return -EPERM;
> > + if (IS_ERR(fud))
> > + return PTR_ERR(fud);
> >
> > if (!user_backed_iter(from))
> > return -EINVAL;
> > @@ -2258,8 +2278,8 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe,
> > ssize_t ret;
> >
> > fud = fuse_get_dev(out);
> > - if (!fud)
> > - return -EPERM;
> > + if (IS_ERR(fud))
> > + return PTR_ERR(fud);
> >
> > pipe_lock(pipe);
> >
> > @@ -2581,8 +2601,8 @@ static long fuse_dev_ioctl_backing_open(struct file *file,
> > struct fuse_dev *fud = fuse_get_dev(file);
>
> Should this be __fuse_get_dev()?
>
> > struct fuse_backing_map map;
> >
> > - if (!fud)
> > - return -EPERM;
> > + if (IS_ERR(fud))
> > + return PTR_ERR(fud);
> >
> > if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
> > return -EOPNOTSUPP;
> > @@ -2598,8 +2618,8 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp)
> > struct fuse_dev *fud = fuse_get_dev(file);
>
> Same question here.
>
> > int backing_id;
> >
> > - if (!fud)
> > - return -EPERM;
> > + if (IS_ERR(fud))
> > + return PTR_ERR(fud);
> >
> > if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
> > return -EOPNOTSUPP;
> > @@ -2610,6 +2630,19 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp)
> > return fuse_backing_close(fud->fc, backing_id);
> > }
> >
> > +static long fuse_dev_ioctl_sync_init(struct file *file)
> > +{
> > + int err = -EINVAL;
> > +
> > + mutex_lock(&fuse_mutex);
> > + if (!__fuse_get_dev(file)) {
> > + WRITE_ONCE(file->private_data, FUSE_DEV_SYNC_INIT);
>
> Does this still need a WRITE_ONCE if it's accessed within the scope of
> the mutex? My understanding (maybe wrong) is that a mutex implicitly
> serves as also a memory barrier. If not, then we probably also need a
> WRITE_ONCE() around the *ctx->fudptr assignment in
> fuse_fill_super_common()?
I agree with this, the (re)ordering before the mutex unlock doesn't
matter because the unlock is a write barrier. But I don't think it
hurts to have redundant WRITE_ONCE.
--D
>
> Thanks,
> Joanne
>
> > + err = 0;
> > + }
> > + mutex_unlock(&fuse_mutex);
> > + return err;
> > +}
> > +
> > @@ -1876,8 +1901,10 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
> >
> > list_add_tail(&fc->entry, &fuse_conn_list);
> > sb->s_root = root_dentry;
> > - if (ctx->fudptr)
> > + if (ctx->fudptr) {
> > *ctx->fudptr = fud;
> > + wake_up_all(&fuse_dev_waitq);
> > + }
> > mutex_unlock(&fuse_mutex);
> > return 0;
> >
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2] fuse: allow synchronous FUSE_INIT
2025-08-27 22:56 ` Joanne Koong
2025-08-28 0:36 ` Darrick J. Wong
@ 2025-08-28 13:09 ` Miklos Szeredi
2025-08-28 16:06 ` Joanne Koong
1 sibling, 1 reply; 11+ messages in thread
From: Miklos Szeredi @ 2025-08-28 13:09 UTC (permalink / raw)
To: Joanne Koong; +Cc: linux-fsdevel, Darrick J. Wong, John Groves, Bernd Schubert
On Thu, Aug 28, 2025 at 12:57 AM Joanne Koong <joannelkoong@gmail.com> wrote:
> I wonder if we should make the semantics the same for synchronous and
> non-synchronous inits here, i.e. doing a wait for
> "(READ_ONCE(file->private_data) != FUSE_DEV_SYNC_INIT) &&
> READ_ONCE(file->private_data) != NULL", so that from the libfuse point
> of view, the flow can be unified between the two, eg
> i) send sync_init ioctl call if doing a synchronous init
> ii) kick off thread to read requests
> iii) do mount call
> otherwise for async inits, the mount call needs to happen first.
Do you suggest that libfuse should ignore the return value of the
sync_init ioctl?
That doesn't work, because old kernels will return an error on read
from an uninitialized fuse dev. Also if kernel now blocks before
mount, that might break some odd server that expects an error.
> > @@ -2233,8 +2253,8 @@ static ssize_t fuse_dev_write(struct kiocb *iocb, struct iov_iter *from)
> > struct fuse_copy_state cs;
> > struct fuse_dev *fud = fuse_get_dev(iocb->ki_filp);
>
> Does this (and below in fuse_dev_splice_write()) need to be
> fuse_get_dev()? afaict, fuse_dev_write() only starts getting used
> after fud has already been initialized. i see why it's needed for
> fuse_dev_read() since otherwise the server doesn't know when it can
> start calling fuse_dev_read(), but for fuse_dev_write(), it seems like
> that only gets used after fud is already initialized.
Yeah.
> > @@ -2610,6 +2630,19 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp)
> > return fuse_backing_close(fud->fc, backing_id);
> > }
> >
> > +static long fuse_dev_ioctl_sync_init(struct file *file)
> > +{
> > + int err = -EINVAL;
> > +
> > + mutex_lock(&fuse_mutex);
> > + if (!__fuse_get_dev(file)) {
> > + WRITE_ONCE(file->private_data, FUSE_DEV_SYNC_INIT);
>
> Does this still need a WRITE_ONCE if it's accessed within the scope of
> the mutex? My understanding (maybe wrong) is that a mutex implicitly
> serves as also a memory barrier.
Mutex does act as a barrier, but __fuse_get_dev() does an unlocked
read, so this WRITE_ONCE is to balance that. I'm not a expert in this
area though.
> If not, then we probably also need a
> WRITE_ONCE() around the *ctx->fudptr assignment in
> fuse_fill_super_common()?
Possibly.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2] fuse: allow synchronous FUSE_INIT
2025-08-28 13:09 ` Miklos Szeredi
@ 2025-08-28 16:06 ` Joanne Koong
0 siblings, 0 replies; 11+ messages in thread
From: Joanne Koong @ 2025-08-28 16:06 UTC (permalink / raw)
To: Miklos Szeredi
Cc: linux-fsdevel, Darrick J. Wong, John Groves, Bernd Schubert
On Thu, Aug 28, 2025 at 6:10 AM Miklos Szeredi <mszeredi@redhat.com> wrote:
>
> On Thu, Aug 28, 2025 at 12:57 AM Joanne Koong <joannelkoong@gmail.com> wrote:
>
> > I wonder if we should make the semantics the same for synchronous and
> > non-synchronous inits here, i.e. doing a wait for
> > "(READ_ONCE(file->private_data) != FUSE_DEV_SYNC_INIT) &&
> > READ_ONCE(file->private_data) != NULL", so that from the libfuse point
> > of view, the flow can be unified between the two, eg
> > i) send sync_init ioctl call if doing a synchronous init
> > ii) kick off thread to read requests
> > iii) do mount call
> > otherwise for async inits, the mount call needs to happen first.
>
> Do you suggest that libfuse should ignore the return value of the
> sync_init ioctl?
>
> That doesn't work, because old kernels will return an error on read
> from an uninitialized fuse dev. Also if kernel now blocks before
> mount, that might break some odd server that expects an error.
Oh right that's a good point, I forgot they might be running an older kernel.
I guess libfuse will have to have two code paths for this regardless
then, which seems totally fine.
Thanks,
Joanne
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] fuse: allow synchronous FUSE_INIT
2025-08-27 10:59 [PATCH v2] fuse: allow synchronous FUSE_INIT Miklos Szeredi
2025-08-27 19:02 ` Darrick J. Wong
2025-08-27 22:56 ` Joanne Koong
@ 2025-08-29 15:43 ` Darrick J. Wong
2025-08-29 16:11 ` Bernd Schubert
2025-08-29 16:15 ` Miklos Szeredi
2 siblings, 2 replies; 11+ messages in thread
From: Darrick J. Wong @ 2025-08-29 15:43 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: linux-fsdevel, Joanne Koong, John Groves, Bernd Schubert
On Wed, Aug 27, 2025 at 12:59:55PM +0200, Miklos Szeredi wrote:
> FUSE_INIT has always been asynchronous with mount. That means that the
> server processed this request after the mount syscall returned.
>
> This means that FUSE_INIT can't supply the root inode's ID, hence it
> currently has a hardcoded value. There are other limitations such as not
> being able to perform getxattr during mount, which is needed by selinux.
>
> To remove these limitations allow server to process FUSE_INIT while
> initializing the in-core super block for the fuse filesystem. This can
> only be done if the server is prepared to handle this, so add
> FUSE_DEV_IOC_SYNC_INIT ioctl, which
>
> a) lets the server know whether this feature is supported, returning
> ENOTTY othewrwise.
>
> b) lets the kernel know to perform a synchronous initialization
>
> The implementation is slightly tricky, since fuse_dev/fuse_conn are set up
> only during super block creation. This is solved by setting the private
> data of the fuse device file to a special value ((struct fuse_dev *) 1) and
> waiting for this to be turned into a proper fuse_dev before commecing with
> operations on the device file.
By the way, how is libfuse supposed to use SYNC_INIT? I gather libfuse
will have to start up the background fuse workers threads to listen for
events /before/ the actual mount() call?
--D
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
> v2:
>
> - make fuse_send_init() perform sync/async sequence based on fc->sync_init
> (Joanne)
>
> fs/fuse/cuse.c | 3 +-
> fs/fuse/dev.c | 74 +++++++++++++++++++++++++++++----------
> fs/fuse/dev_uring.c | 4 +--
> fs/fuse/fuse_dev_i.h | 13 +++++--
> fs/fuse/fuse_i.h | 5 ++-
> fs/fuse/inode.c | 50 ++++++++++++++++++++------
> include/uapi/linux/fuse.h | 1 +
> 7 files changed, 115 insertions(+), 35 deletions(-)
>
> diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c
> index b39844d75a80..28c96961e85d 100644
> --- a/fs/fuse/cuse.c
> +++ b/fs/fuse/cuse.c
> @@ -52,6 +52,7 @@
> #include <linux/user_namespace.h>
>
> #include "fuse_i.h"
> +#include "fuse_dev_i.h"
>
> #define CUSE_CONNTBL_LEN 64
>
> @@ -547,7 +548,7 @@ static int cuse_channel_open(struct inode *inode, struct file *file)
> */
> static int cuse_channel_release(struct inode *inode, struct file *file)
> {
> - struct fuse_dev *fud = file->private_data;
> + struct fuse_dev *fud = __fuse_get_dev(file);
> struct cuse_conn *cc = fc_to_cc(fud->fc);
>
> /* remove from the conntbl, no more access from this point on */
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 8ac074414897..948f45c6e0ef 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -1530,14 +1530,34 @@ static int fuse_dev_open(struct inode *inode, struct file *file)
> return 0;
> }
>
> +struct fuse_dev *fuse_get_dev(struct file *file)
> +{
> + struct fuse_dev *fud = __fuse_get_dev(file);
> + int err;
> +
> + if (likely(fud))
> + return fud;
> +
> + err = wait_event_interruptible(fuse_dev_waitq,
> + READ_ONCE(file->private_data) != FUSE_DEV_SYNC_INIT);
> + if (err)
> + return ERR_PTR(err);
> +
> + fud = __fuse_get_dev(file);
> + if (!fud)
> + return ERR_PTR(-EPERM);
> +
> + return fud;
> +}
> +
> static ssize_t fuse_dev_read(struct kiocb *iocb, struct iov_iter *to)
> {
> struct fuse_copy_state cs;
> struct file *file = iocb->ki_filp;
> struct fuse_dev *fud = fuse_get_dev(file);
>
> - if (!fud)
> - return -EPERM;
> + if (IS_ERR(fud))
> + return PTR_ERR(fud);
>
> if (!user_backed_iter(to))
> return -EINVAL;
> @@ -1557,8 +1577,8 @@ static ssize_t fuse_dev_splice_read(struct file *in, loff_t *ppos,
> struct fuse_copy_state cs;
> struct fuse_dev *fud = fuse_get_dev(in);
>
> - if (!fud)
> - return -EPERM;
> + if (IS_ERR(fud))
> + return PTR_ERR(fud);
>
> bufs = kvmalloc_array(pipe->max_usage, sizeof(struct pipe_buffer),
> GFP_KERNEL);
> @@ -2233,8 +2253,8 @@ static ssize_t fuse_dev_write(struct kiocb *iocb, struct iov_iter *from)
> struct fuse_copy_state cs;
> struct fuse_dev *fud = fuse_get_dev(iocb->ki_filp);
>
> - if (!fud)
> - return -EPERM;
> + if (IS_ERR(fud))
> + return PTR_ERR(fud);
>
> if (!user_backed_iter(from))
> return -EINVAL;
> @@ -2258,8 +2278,8 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe,
> ssize_t ret;
>
> fud = fuse_get_dev(out);
> - if (!fud)
> - return -EPERM;
> + if (IS_ERR(fud))
> + return PTR_ERR(fud);
>
> pipe_lock(pipe);
>
> @@ -2343,7 +2363,7 @@ static __poll_t fuse_dev_poll(struct file *file, poll_table *wait)
> struct fuse_iqueue *fiq;
> struct fuse_dev *fud = fuse_get_dev(file);
>
> - if (!fud)
> + if (IS_ERR(fud))
> return EPOLLERR;
>
> fiq = &fud->fc->iq;
> @@ -2490,7 +2510,7 @@ void fuse_wait_aborted(struct fuse_conn *fc)
>
> int fuse_dev_release(struct inode *inode, struct file *file)
> {
> - struct fuse_dev *fud = fuse_get_dev(file);
> + struct fuse_dev *fud = __fuse_get_dev(file);
>
> if (fud) {
> struct fuse_conn *fc = fud->fc;
> @@ -2521,8 +2541,8 @@ static int fuse_dev_fasync(int fd, struct file *file, int on)
> {
> struct fuse_dev *fud = fuse_get_dev(file);
>
> - if (!fud)
> - return -EPERM;
> + if (IS_ERR(fud))
> + return PTR_ERR(fud);
>
> /* No locking - fasync_helper does its own locking */
> return fasync_helper(fd, file, on, &fud->fc->iq.fasync);
> @@ -2532,7 +2552,7 @@ static int fuse_device_clone(struct fuse_conn *fc, struct file *new)
> {
> struct fuse_dev *fud;
>
> - if (new->private_data)
> + if (__fuse_get_dev(new))
> return -EINVAL;
>
> fud = fuse_dev_alloc_install(fc);
> @@ -2563,7 +2583,7 @@ static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
> * uses the same ioctl handler.
> */
> if (fd_file(f)->f_op == file->f_op)
> - fud = fuse_get_dev(fd_file(f));
> + fud = __fuse_get_dev(fd_file(f));
>
> res = -EINVAL;
> if (fud) {
> @@ -2581,8 +2601,8 @@ static long fuse_dev_ioctl_backing_open(struct file *file,
> struct fuse_dev *fud = fuse_get_dev(file);
> struct fuse_backing_map map;
>
> - if (!fud)
> - return -EPERM;
> + if (IS_ERR(fud))
> + return PTR_ERR(fud);
>
> if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
> return -EOPNOTSUPP;
> @@ -2598,8 +2618,8 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp)
> struct fuse_dev *fud = fuse_get_dev(file);
> int backing_id;
>
> - if (!fud)
> - return -EPERM;
> + if (IS_ERR(fud))
> + return PTR_ERR(fud);
>
> if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
> return -EOPNOTSUPP;
> @@ -2610,6 +2630,19 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp)
> return fuse_backing_close(fud->fc, backing_id);
> }
>
> +static long fuse_dev_ioctl_sync_init(struct file *file)
> +{
> + int err = -EINVAL;
> +
> + mutex_lock(&fuse_mutex);
> + if (!__fuse_get_dev(file)) {
> + WRITE_ONCE(file->private_data, FUSE_DEV_SYNC_INIT);
> + err = 0;
> + }
> + mutex_unlock(&fuse_mutex);
> + return err;
> +}
> +
> static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
> unsigned long arg)
> {
> @@ -2625,6 +2658,9 @@ static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
> case FUSE_DEV_IOC_BACKING_CLOSE:
> return fuse_dev_ioctl_backing_close(file, argp);
>
> + case FUSE_DEV_IOC_SYNC_INIT:
> + return fuse_dev_ioctl_sync_init(file);
> +
> default:
> return -ENOTTY;
> }
> @@ -2633,7 +2669,7 @@ static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
> #ifdef CONFIG_PROC_FS
> static void fuse_dev_show_fdinfo(struct seq_file *seq, struct file *file)
> {
> - struct fuse_dev *fud = fuse_get_dev(file);
> + struct fuse_dev *fud = __fuse_get_dev(file);
> if (!fud)
> return;
>
> diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
> index 249b210becb1..bef38ed78249 100644
> --- a/fs/fuse/dev_uring.c
> +++ b/fs/fuse/dev_uring.c
> @@ -1139,9 +1139,9 @@ int fuse_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags)
> return -EINVAL;
>
> fud = fuse_get_dev(cmd->file);
> - if (!fud) {
> + if (IS_ERR(fud)) {
> pr_info_ratelimited("No fuse device found\n");
> - return -ENOTCONN;
> + return PTR_ERR(fud);
> }
> fc = fud->fc;
>
> diff --git a/fs/fuse/fuse_dev_i.h b/fs/fuse/fuse_dev_i.h
> index 5a9bd771a319..6e8373f97040 100644
> --- a/fs/fuse/fuse_dev_i.h
> +++ b/fs/fuse/fuse_dev_i.h
> @@ -12,6 +12,8 @@
> #define FUSE_INT_REQ_BIT (1ULL << 0)
> #define FUSE_REQ_ID_STEP (1ULL << 1)
>
> +extern struct wait_queue_head fuse_dev_waitq;
> +
> struct fuse_arg;
> struct fuse_args;
> struct fuse_pqueue;
> @@ -37,15 +39,22 @@ struct fuse_copy_state {
> } ring;
> };
>
> -static inline struct fuse_dev *fuse_get_dev(struct file *file)
> +#define FUSE_DEV_SYNC_INIT ((struct fuse_dev *) 1)
> +#define FUSE_DEV_PTR_MASK (~1UL)
> +
> +static inline struct fuse_dev *__fuse_get_dev(struct file *file)
> {
> /*
> * Lockless access is OK, because file->private data is set
> * once during mount and is valid until the file is released.
> */
> - return READ_ONCE(file->private_data);
> + struct fuse_dev *fud = READ_ONCE(file->private_data);
> +
> + return (typeof(fud)) ((unsigned long) fud & FUSE_DEV_PTR_MASK);
> }
>
> +struct fuse_dev *fuse_get_dev(struct file *file);
> +
> unsigned int fuse_req_hash(u64 unique);
> struct fuse_req *fuse_request_find(struct fuse_pqueue *fpq, u64 unique);
>
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 486fa550c951..233c6111f768 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -904,6 +904,9 @@ struct fuse_conn {
> /* Is link not implemented by fs? */
> unsigned int no_link:1;
>
> + /* Is synchronous FUSE_INIT allowed? */
> + unsigned int sync_init:1;
> +
> /* Use io_uring for communication */
> unsigned int io_uring;
>
> @@ -1318,7 +1321,7 @@ struct fuse_dev *fuse_dev_alloc_install(struct fuse_conn *fc);
> struct fuse_dev *fuse_dev_alloc(void);
> void fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc);
> void fuse_dev_free(struct fuse_dev *fud);
> -void fuse_send_init(struct fuse_mount *fm);
> +int fuse_send_init(struct fuse_mount *fm);
>
> /**
> * Fill in superblock and initialize fuse connection
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 9d26a5bc394d..7cf47d5bcc87 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -7,6 +7,7 @@
> */
>
> #include "fuse_i.h"
> +#include "fuse_dev_i.h"
> #include "dev_uring_i.h"
>
> #include <linux/dax.h>
> @@ -34,6 +35,7 @@ MODULE_LICENSE("GPL");
> static struct kmem_cache *fuse_inode_cachep;
> struct list_head fuse_conn_list;
> DEFINE_MUTEX(fuse_mutex);
> +DECLARE_WAIT_QUEUE_HEAD(fuse_dev_waitq);
>
> static int set_global_limit(const char *val, const struct kernel_param *kp);
>
> @@ -1466,7 +1468,7 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
> wake_up_all(&fc->blocked_waitq);
> }
>
> -void fuse_send_init(struct fuse_mount *fm)
> +static struct fuse_init_args *fuse_new_init(struct fuse_mount *fm)
> {
> struct fuse_init_args *ia;
> u64 flags;
> @@ -1525,10 +1527,29 @@ void fuse_send_init(struct fuse_mount *fm)
> ia->args.out_args[0].value = &ia->out;
> ia->args.force = true;
> ia->args.nocreds = true;
> - ia->args.end = process_init_reply;
>
> - if (fuse_simple_background(fm, &ia->args, GFP_KERNEL) != 0)
> - process_init_reply(fm, &ia->args, -ENOTCONN);
> + return ia;
> +}
> +
> +int fuse_send_init(struct fuse_mount *fm)
> +{
> + struct fuse_init_args *ia = fuse_new_init(fm);
> + int err;
> +
> + if (fm->fc->sync_init) {
> + err = fuse_simple_request(fm, &ia->args);
> + /* Ignore size of init reply */
> + if (err > 0)
> + err = 0;
> + } else {
> + ia->args.end = process_init_reply;
> + err = fuse_simple_background(fm, &ia->args, GFP_KERNEL);
> + if (!err)
> + return 0;
> + err = -ENOTCONN;
> + }
> + process_init_reply(fm, &ia->args, err);
> + return err;
> }
> EXPORT_SYMBOL_GPL(fuse_send_init);
>
> @@ -1867,8 +1888,12 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
>
> mutex_lock(&fuse_mutex);
> err = -EINVAL;
> - if (ctx->fudptr && *ctx->fudptr)
> - goto err_unlock;
> + if (ctx->fudptr && *ctx->fudptr) {
> + if (*ctx->fudptr == FUSE_DEV_SYNC_INIT) {
> + fc->sync_init = 1;
> + } else
> + goto err_unlock;
> + }
>
> err = fuse_ctl_add_conn(fc);
> if (err)
> @@ -1876,8 +1901,10 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
>
> list_add_tail(&fc->entry, &fuse_conn_list);
> sb->s_root = root_dentry;
> - if (ctx->fudptr)
> + if (ctx->fudptr) {
> *ctx->fudptr = fud;
> + wake_up_all(&fuse_dev_waitq);
> + }
> mutex_unlock(&fuse_mutex);
> return 0;
>
> @@ -1898,6 +1925,7 @@ EXPORT_SYMBOL_GPL(fuse_fill_super_common);
> static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
> {
> struct fuse_fs_context *ctx = fsc->fs_private;
> + struct fuse_mount *fm;
> int err;
>
> if (!ctx->file || !ctx->rootmode_present ||
> @@ -1918,8 +1946,10 @@ static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
> return err;
> /* file->private_data shall be visible on all CPUs after this */
> smp_mb();
> - fuse_send_init(get_fuse_mount_super(sb));
> - return 0;
> +
> + fm = get_fuse_mount_super(sb);
> +
> + return fuse_send_init(fm);
> }
>
> /*
> @@ -1980,7 +2010,7 @@ static int fuse_get_tree(struct fs_context *fsc)
> * Allow creating a fuse mount with an already initialized fuse
> * connection
> */
> - fud = READ_ONCE(ctx->file->private_data);
> + fud = __fuse_get_dev(ctx->file);
> if (ctx->file->f_op == &fuse_dev_operations && fud) {
> fsc->sget_key = fud->fc;
> sb = sget_fc(fsc, fuse_test_super, fuse_set_no_super);
> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> index 3942d1fda599..30bf0846547f 100644
> --- a/include/uapi/linux/fuse.h
> +++ b/include/uapi/linux/fuse.h
> @@ -1130,6 +1130,7 @@ struct fuse_backing_map {
> #define FUSE_DEV_IOC_BACKING_OPEN _IOW(FUSE_DEV_IOC_MAGIC, 1, \
> struct fuse_backing_map)
> #define FUSE_DEV_IOC_BACKING_CLOSE _IOW(FUSE_DEV_IOC_MAGIC, 2, uint32_t)
> +#define FUSE_DEV_IOC_SYNC_INIT _IO(FUSE_DEV_IOC_MAGIC, 3)
>
> struct fuse_lseek_in {
> uint64_t fh;
> --
> 2.49.0
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2] fuse: allow synchronous FUSE_INIT
2025-08-29 15:43 ` Darrick J. Wong
@ 2025-08-29 16:11 ` Bernd Schubert
2025-08-29 16:15 ` Miklos Szeredi
1 sibling, 0 replies; 11+ messages in thread
From: Bernd Schubert @ 2025-08-29 16:11 UTC (permalink / raw)
To: Darrick J. Wong, Miklos Szeredi; +Cc: linux-fsdevel, Joanne Koong, John Groves
On 8/29/25 17:43, Darrick J. Wong wrote:
> On Wed, Aug 27, 2025 at 12:59:55PM +0200, Miklos Szeredi wrote:
>> FUSE_INIT has always been asynchronous with mount. That means that the
>> server processed this request after the mount syscall returned.
>>
>> This means that FUSE_INIT can't supply the root inode's ID, hence it
>> currently has a hardcoded value. There are other limitations such as not
>> being able to perform getxattr during mount, which is needed by selinux.
>>
>> To remove these limitations allow server to process FUSE_INIT while
>> initializing the in-core super block for the fuse filesystem. This can
>> only be done if the server is prepared to handle this, so add
>> FUSE_DEV_IOC_SYNC_INIT ioctl, which
>>
>> a) lets the server know whether this feature is supported, returning
>> ENOTTY othewrwise.
>>
>> b) lets the kernel know to perform a synchronous initialization
>>
>> The implementation is slightly tricky, since fuse_dev/fuse_conn are set up
>> only during super block creation. This is solved by setting the private
>> data of the fuse device file to a special value ((struct fuse_dev *) 1) and
>> waiting for this to be turned into a proper fuse_dev before commecing with
>> operations on the device file.
>
> By the way, how is libfuse supposed to use SYNC_INIT? I gather libfuse
> will have to start up the background fuse workers threads to listen for
> events /before/ the actual mount() call?
libfuse actually starts worker threads dynamically. Not something that I
like too much, but I also don't want to change it too much. The io-uring
threads are started all at once, when FUSE_INIT is received.
Regarding the /dev/fuse worker threads, there are also existing races with
actual thread buffer size - I'm going to change it to only allow one
thread until FUSE_INIT is processed.
Thanks,
Bernd
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] fuse: allow synchronous FUSE_INIT
2025-08-29 15:43 ` Darrick J. Wong
2025-08-29 16:11 ` Bernd Schubert
@ 2025-08-29 16:15 ` Miklos Szeredi
1 sibling, 0 replies; 11+ messages in thread
From: Miklos Szeredi @ 2025-08-29 16:15 UTC (permalink / raw)
To: Darrick J. Wong
Cc: Miklos Szeredi, linux-fsdevel, Joanne Koong, John Groves,
Bernd Schubert
On Fri, 29 Aug 2025 at 17:45, Darrick J. Wong <djwong@kernel.org> wrote:
> By the way, how is libfuse supposed to use SYNC_INIT? I gather libfuse
> will have to start up the background fuse workers threads to listen for
> events /before/ the actual mount() call?
Not necessarily before, but obviously in parallel with mount. Just
needs two threads, one fires off the mount and the other the read. No
other synchronizations necessary.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 11+ messages in thread