Re: [PATCH v1.1 1/2] libfuse: don't use SYNC_INIT unless asked for

["Darrick J. Wong" <djwong@kernel.org>]

On Tue, May 05, 2026 at 10:02:18PM +0200, Bernd Schubert wrote:
> 
> 
> On 5/5/26 18:44, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > fuse2fs calls fuse_main, then starts threads from ->init.  It doesn't
> > call fuse_daemonize_early_start because I haven't ported it to use any
> > of the new APIs.  I don't have io_uring enabled for fuse on my dev box.
> > 
> > fuse_main calls fuse_session_mount calls fuse_session_mount_new_api
> > calls session_start_sync_init.  At the start of the function,
> > se->want_sync_init, se->uring.enable, and daemonize.active are all
> > false.  The first branch is not taken, so we call FUSE_DEV_IOC_SYNC_INIT
> > and enable sync_init even though the user didn't ask for that and didn't
> > prepare for it either.
> > 
> > FUSE_DEV_IOC_SYNC_INIT succeeds, so we send the synchronous FUSE_INIT
> > from mount, which calls fuse2fs' init() method.  That starts the
> > background threads and returns.  Upon return to the kernel, the mount()
> > now succeeds, and the next thing that fuse_main does is call
> > fuse_daemonize().  Since we didn't call fuse_daemonize_early_start, the
> > daemonize forks the process and the threads die with the parent.
> > 
> > If we didn't ask for SYNC_INIT, don't enable it.  This is needed to
> > maintain compatibility with older fuse servers that only support
> > asynchronous FUSE_INIT.
> > 
> > Fixes: 3e1101057aea57 ("fuse mount: Support synchronous FUSE_INIT (privileged daemon)")
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> > v1.1: improve commit message, refine logging logic
> > ---
> >  lib/fuse_lowlevel.c |   18 +++++++++++++++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)
> > 
> > diff --git a/lib/fuse_lowlevel.c b/lib/fuse_lowlevel.c
> > index 0e16845d2f14ff..83158d02bb8827 100644
> > --- a/lib/fuse_lowlevel.c
> > +++ b/lib/fuse_lowlevel.c
> > @@ -4475,9 +4475,21 @@ static int session_start_sync_init(struct fuse_session *se, int fd)
> >  {
> >  	int err, res;
> >  
> > -	if (!se->want_sync_init &&
> > -		(se->uring.enable && !fuse_daemonize_is_used())) {
> > -		if (se->debug)
> > +	if (!se->want_sync_init) {
> > +		/*
> > +		 * SYNC_INIT is required for io_uring to initialize without
> > +		 * deadlocking the kernel if the fuse server crashes.
> > +		 *
> > +		 * !fuse_daemonize_is_used implies the fuse server doesn't know
> > +		 * about any of the SYNC_INIT APIs, so we don't enable sync
> > +		 * init or generate log messages.
> > +		 */
> > +		if (se->uring.enable)
> > +			fuse_log(FUSE_LOG_DEBUG,
> > +					"fuse: io_uring broken with async init\n");
> 
> So want_sync_init is not enabled and it still logs this? Note that
> default libfuse does not have a way to set log levels.

I'm not 

> > +		else if (!fuse_daemonize_is_used())
> > +			; /* empty */
> 
> Unless I misread it, it would still go into the return 0?

Correct.

> > +		else if (se->debug)
> >  			fuse_log(FUSE_LOG_DEBUG,
> >  					"fuse: sync init not enabled\n");
> 
> And now every time a log message here.

Only if you enable debug mode.

> >  		return 0;
> 
> Why can't we change it to
> 
> if (!fuse_daemonize_is_used() && !want_sync_init)
>        return 0;

That works for me.

> If someone uses then new mount API, that someone has also tested sync
> init.

I'm confused by this statement.

In this early bailout case (fuse server did not call
fuse_daemonize_early_start or fuse_session_want_sync_init) we return 0
without actually setting SYNC_INIT, correct?

Next, fuse_session_mount_new_api calls fuse_kern_fsmount_mo and libfuse
proceeds to mount with the new mount API.  The kernel uses the old async
init code, because nobody told it to do a synchronous init.

That sequence works fine, and I can prove it by strace'ing fuse2fs
startup.  The new mount API is used, but FUSE_INIT isn't seen by fuse2fs
until after the fuse_daemonize() call and the event loop gets set up.

In other words, fuse2fs uses the new mount API and does not use sync
init.  I think the correct statement is the inverse:

    If someone uses sync_init, that someone has also tested the new
    mount API.

My understanding of where we are right now is:

1. If a fuse server starts up the new daemonize code or sets
   want_sync_init, it will get SYNC_INIT.

   (I'm not sure what it means if they set want_sync_init and try to
   use the old daemonize code)

2. If it does neither of those things, it will not get SYNC_INIT.

In both cases we try the new fsmount().  If the fsmount API fails, then
we fall back to the classic mount().

Is that correct?

> At best we could change want_sync_init to an integer so that a
> daemon could disable sync init.
> For testing I'm probably also going to add an env variable that allows
> to disable sync init.

<shrug> That sounds ok to me.

> Btw, during the meetings I had asked AI to write tests for sync init,
> looks like "ro" doesn't work yet. AI had created patch to fix that, but
> I don't apply either the test nor the fix yet, need to carefully read
> through it, but need to do something else before going to bed.

Oh, yeah, mount_flags[] needs to have:

{"rw",           MS_RDONLY,        0,  1,    MOUNT_ATTR_RDONLY},
{"ro",           MS_RDONLY,        1,  1,    MOUNT_ATTR_RDONLY},

Sorry I missed that in the review.  I also missed that mount_service.c
should be ported to use mount_flags[].

--D