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

[Bernd Schubert <bernd@bsbernd.com>]

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.

> +		else if (!fuse_daemonize_is_used())
> +			; /* empty */

Unless I misread it, it would still go into the return 0?

> +		else if (se->debug)
>  			fuse_log(FUSE_LOG_DEBUG,
>  					"fuse: sync init not enabled\n");

And now every time a log message here.

>  		return 0;

Why can't we change it to

if (!fuse_daemonize_is_used() && !want_sync_init)
       return 0;


If someone uses then new mount API, that someone has also tested sync
init. 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.

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.


Thanks,
Bernd