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

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

On Wed, May 06, 2026 at 12:29:22AM +0200, Bernd Schubert wrote:
> 
> 
> On 5/6/26 00:08, Darrick J. Wong wrote:
> > 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 
> 
> I need to work on libfuse log level setting, the current way doesn't
> distinguish between log levels. But given all the other things to do and
> given daemons I had been working with have their own logging system,
> this always had a very low prio for me.

<nod>  I'll leave it at this:

	/*
	 * Older fuse servers do not set want_sync_init or start the new
	 * daemonize code, so they get async init.
	 */
	if (!fuse_daemonize_is_used() || !se->want_sync_init) {
		if (se->debug)
			fuse_log(FUSE_LOG_DEBUG,
					"fuse: sync init not enabled\n");
		return 0;
	}

and then we can work on another commit that adds the magic env variable
that disables sync_init and/or fsmount.

> > 
> >>> +		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.
> 
> Ah right.
> 
> > 
> >>>  		return 0;
> >>
> >> Why can't we change it to
> >>
> >> if (!fuse_daemonize_is_used() && !want_sync_init)
> >>        return 0;
> > 
> > That works for me.
> 
> Let's do that with another variable and also env to disable sync init
> and new mount api.
> 
> > 
> >> If someone uses then new mount API, that someone has also tested sync
> >> init.
> > 
> > I'm confused by this statement.
> 
> I'm deeply sorry, I wanted to write "If someone uses the new daemonize
> API, that someone ...". Oh well, that wold only be true for kernels that
> have sync init.

Ah, I got it now. :)

> > 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[].
> 
> Thanks, I need to read through the docs again. I think we update
> mount_service.c after merging your series.

<nod>  I'll post the fix for mount_flags shortly, and the
mount_service.c port whenever I get to another round of patchset 2.

--D

> 
> Thanks,\Bernd
>