On Thu, Aug 28, 2025 at 12:57 AM Joanne Koong <joannelkoong@xxxxxxxxx> 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
