Re: [RFC PATCH v3 8/8] fuse: implementation of mkobj_handle+statx+open compound operation

[Luis Henriques <luis@igalia.com>]

Hi!

On Wed, Feb 25 2026, Horst Birthelmer wrote:

> Hi Luis,
>
> On Wed, Feb 25, 2026 at 11:24:39AM +0000, Luis Henriques wrote:
>> The implementation of this compound operation allows atomic_open() to use
>> file handle.  It also introduces a new MKOBJ_HANDLE operation that will
>> handle the file system object creation and will return the file handle.
>> 
>> The atomicity of the operation (create + open) needs to be handled in
>> user-space (e.g. the handling of the O_EXCL flag).
>> 
>> Signed-off-by: Luis Henriques <luis@igalia.com>
>> ---
>>  fs/fuse/dir.c             | 219 +++++++++++++++++++++++++++++++++++++-
>>  include/uapi/linux/fuse.h |   2 +
>>  2 files changed, 220 insertions(+), 1 deletion(-)
>> 
>> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
>> index 7fa8c405f1a3..b5beb1d62c3d 100644
>> --- a/fs/fuse/dir.c
>> +++ b/fs/fuse/dir.c
>> @@ -1173,6 +1173,220 @@ static int fuse_create_open(struct mnt_idmap *idmap, struct inode *dir,
>>  	return err;
>>  }
>>  
>> +static int fuse_mkobj_handle_init(struct fuse_mount *fm, struct fuse_args *args,
>> +				  struct mnt_idmap *idmap, struct inode *dir,
>> +				  struct dentry *entry, unsigned int flags,
>> +				  umode_t mode,
>> +				  struct fuse_create_in *inarg,
>> +				  struct fuse_entry2_out *outarg,
>> +				  struct fuse_file_handle **fh)
>> +{
>> +	struct fuse_inode *fi;
>> +	size_t fh_size = sizeof(struct fuse_file_handle) + MAX_HANDLE_SZ;
>> +	int err = 0;
>> +
>> +	*fh = kzalloc(fh_size, GFP_KERNEL);
>> +	if (!*fh)
>> +		return -ENOMEM;
>> +
>> +	memset(inarg, 0, sizeof(*inarg));
>> +	memset(outarg, 0, sizeof(*outarg));
>> +
>> +	inarg->flags = flags;
>> +	inarg->mode = mode;
>> +	inarg->umask = current_umask();
>> +
>> +	if (fm->fc->handle_killpriv_v2 && (flags & O_TRUNC) &&
>> +	    !(flags & O_EXCL) && !capable(CAP_FSETID))
>> +		inarg->open_flags |= FUSE_OPEN_KILL_SUIDGID;
>> +
>> +	args->opcode = FUSE_MKOBJ_HANDLE;
>> +	args->nodeid = get_node_id(dir);
>> +	args->in_numargs = 2;
>> +	args->in_args[0].size = sizeof(*inarg);
>> +	args->in_args[0].value = inarg;
>> +	args->in_args[1].size = entry->d_name.len + 1;
>> +	args->in_args[1].value = entry->d_name.name;
>> +
>> +	err = get_create_ext(idmap, args, dir, entry, mode);
>> +	if (err)
>> +		goto out_err;
>> +	fi = get_fuse_inode(dir);
>> +	if (fi && fi->fh) {
>> +		if (!args->is_ext) {
>> +			args->is_ext = true;
>> +			args->ext_idx = args->in_numargs++;
>> +		}
>> +		err = create_ext_handle(&args->in_args[args->ext_idx], fi);
>> +		if (err)
>> +			goto out_err;
>> +	}
>> +
>> +	args->out_numargs = 2;
>> +	args->out_args[0].size = sizeof(*outarg);
>> +	args->out_args[0].value = outarg;
>> +	args->out_args[1].size = fh_size;
>> +	args->out_args[1].value = *fh;
>> +
>> +out_err:
>> +	if (err) {
>> +		kfree(*fh);
>> +		free_ext_value(args);
>> +	}
>> +
>> +	return err;
>> +}
>> +
>> +static int fuse_mkobj_statx_open(struct mnt_idmap *idmap, struct inode *dir,
>> +				 struct dentry *entry, struct file *file,
>> +				 unsigned int flags, umode_t mode)
>> +{
>> +	struct fuse_compound_req *compound;
>> +	struct fuse_mount *fm = get_fuse_mount(dir);
>> +	struct fuse_inode *fi = NULL;
>> +	struct fuse_create_in mkobj_in;
>> +	struct fuse_entry2_out mkobj_out;
>> +	struct fuse_statx_in statx_in;
>> +	struct fuse_statx_out statx_out;
>> +	struct fuse_open_in open_in;
>> +	struct fuse_open_out *open_outp;
>> +	FUSE_ARGS(mkobj_args);
>> +	FUSE_ARGS(statx_args);
>> +	FUSE_ARGS(open_args);
>> +	struct fuse_forget_link *forget;
>> +	struct fuse_file *ff;
>> +	struct fuse_attr attr;
>> +	struct fuse_file_handle *fh = NULL;
>> +	struct inode *inode;
>> +	int epoch, ret = -EIO;
>> +	int i;
>> +
>> +	epoch = atomic_read(&fm->fc->epoch);
>> +
>> +	ret = -ENOMEM;
>> +	forget = fuse_alloc_forget();
>> +	if (!forget)
>> +		return -ENOMEM;
>> +	ff = fuse_file_alloc(fm, true);
>> +	if (!ff)
>> +		goto out_forget;
>> +
>> +	if (!fm->fc->dont_mask)
>> +		mode &= ~current_umask();
>> +
>> +	flags &= ~O_NOCTTY;
>> +
>> +	compound = fuse_compound_alloc(fm, FUSE_COMPOUND_ATOMIC);
>> +	if (!compound)
>> +		goto out_free_ff;
>> +
>
> Just to clarify for myself and maybe others.
> You want this to be processed atomic on the fuse server and never
> be separated by the upcoming 'decode and send separate' code in the
> kernel?
> Is that really necessarry? What would the consequences be, 
> if this is not really atomic?

No, you're right -- it's unlikely that this flag is required.  If I
remember correctly from the discussion, the flags and what they mean is
one of the things still not written in stone for the compound operations,
right?

Regarding this compound specifically, what I wanted was to ensure is that:

- The operations are serialised as they are interdependent(*), and
- If one operation fails, the others can be aborted.

(*) Actually, the last 2 ops (statx and open) could be parallelised.

>> +	fi = get_fuse_inode(dir);
>> +	if (!fi) {
>> +		ret = -EIO;
>> +		goto out_compound;
>> +	}
>> +	ret = fuse_mkobj_handle_init(fm, &mkobj_args, idmap, dir, entry, flags,
>> +				     mode, &mkobj_in, &mkobj_out, &fh);
>> +	if (ret)
>> +		goto out_compound;
>> +
>> +	ret = fuse_compound_add(compound, &mkobj_args);
>> +	if (ret)
>> +		goto out_mkobj_args;
>> +
>> +	fuse_statx_init(&statx_args, &statx_in, FUSE_ROOT_ID, NULL, &statx_out);
>> +	ret = fuse_compound_add(compound, &statx_args);
>> +	if (ret)
>> +		goto out_mkobj_args;
>> +
>> +	ff->fh = 0;
>> +	ff->open_flags = FOPEN_KEEP_CACHE;
>> +	memset(&open_in, 0, sizeof(open_in));
>> +
>> +	/* XXX flags handling */
>> +	open_in.flags = ff->open_flags & ~(O_CREAT | O_EXCL | O_NOCTTY);
>> +	if (!fm->fc->atomic_o_trunc)
>> +		open_in.flags &= ~O_TRUNC;
>> +	if (fm->fc->handle_killpriv_v2 &&
>> +	    (open_in.flags & O_TRUNC) && !capable(CAP_FSETID))
>> +		open_in.open_flags |= FUSE_OPEN_KILL_SUIDGID;
>> +
>> +	open_outp = &ff->args->open_outarg;
>> +	fuse_open_args_fill(&open_args, FUSE_ROOT_ID, FUSE_OPEN, &open_in,
>> +			    open_outp);
>> +
>> +	ret = fuse_compound_add(compound, &open_args);
>> +	if (ret)
>> +		goto out_mkobj_args;
>> +
>> +	ret = fuse_compound_send(compound);
>
> Your compound looks good so far ;-)

Yey!

>> +	if (ret)
>> +		goto out_mkobj_args;
>> +
>> +	for (i = 0; i < 3; i++) {
>> +		int err;
>> +
>> +		err = fuse_compound_get_error(compound, i);
>> +		if (err && !ret)
>> +			ret = err;
>> +	}
>
> this is probably why you opted for that 'give me any occurred error'
> functionality?

Right, since there are interdependencies between the operations I only
care about the first failure.  Probably a pr_warning() could be used here
to log each of them.

Anyway, it's not clear that this pattern will be common enough in order to
think about an helper for it.  I guess that if we are bundling a bunch of
operations that can be parallelised, then error handling needs to be done
differently.

Cheers,
-- 
Luís

>> +	if (ret)
>> +		goto out_mkobj_args;
>> +
>> +	fuse_statx_to_attr(&statx_out.stat, &attr);
>> +	WARN_ON(fuse_invalid_attr(&attr));
>> +	ret = -EIO;
>> +	if (!S_ISREG(attr.mode) || invalid_nodeid(mkobj_out.nodeid) ||
>> +	    fuse_invalid_attr(&attr))
>> +		goto out_mkobj_args;
>> +
>> +	ff->fh = open_outp->fh;
>> +	ff->nodeid = mkobj_out.nodeid;
>> +	ff->open_flags = open_outp->open_flags;
>> +	inode = fuse_iget(dir->i_sb, mkobj_out.nodeid, mkobj_out.generation,
>> +			  &attr, ATTR_TIMEOUT(&statx_out), 0, 0, fh);
>> +	if (!inode) {
>> +		flags &= ~(O_CREAT | O_EXCL | O_TRUNC);
>> +		fuse_sync_release(NULL, ff, flags);
>> +		fuse_queue_forget(fm->fc, forget, mkobj_out.nodeid, 1);
>> +		ret = -ENOMEM;
>> +		goto out_mkobj_args;
>> +	}
>> +	d_instantiate(entry, inode);
>> +
>> +	entry->d_time = epoch;
>> +	fuse_dentry_settime(entry,
>> +		fuse_time_to_jiffies(mkobj_out.entry_valid,
>> +				     mkobj_out.entry_valid_nsec));
>> +	fuse_dir_changed(dir);
>> +	ret = generic_file_open(inode, file);
>> +	if (!ret) {
>> +		file->private_data = ff;
>> +		ret = finish_open(file, entry, fuse_finish_open);
>> +	}
>> +	if (ret) {
>> +		fuse_sync_release(get_fuse_inode(inode), ff, flags);
>> +	} else {
>> +		if (fm->fc->atomic_o_trunc && (flags & O_TRUNC))
>> +			truncate_pagecache(inode, 0);
>> +		else if (!(ff->open_flags & FOPEN_KEEP_CACHE))
>> +			invalidate_inode_pages2(inode->i_mapping);
>> +	}
>> +
>> +out_mkobj_args:
>> +	fuse_req_free_argvar_ext(&mkobj_args);
>> +out_compound:
>> +	kfree(compound);
>> +out_free_ff:
>> +	if (ret)
>> +		fuse_file_free(ff);
>> +out_forget:
>> +	kfree(forget);
>> +	kfree(fh);
>> +
>> +	return ret;
>> +}
>> +
>>  static int fuse_mknod(struct mnt_idmap *, struct inode *, struct dentry *,
>>  		      umode_t, dev_t);
>>  static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
>> @@ -1201,7 +1415,10 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
>>  	if (fc->no_create)
>>  		goto mknod;
>>  
>> -	err = fuse_create_open(idmap, dir, entry, file, flags, mode, FUSE_CREATE);
>> +	if (fc->lookup_handle)
>> +		err = fuse_mkobj_statx_open(idmap, dir, entry, file, flags, mode);
>> +	else
>> +		err = fuse_create_open(idmap, dir, entry, file, flags, mode, FUSE_CREATE);
>>  	if (err == -ENOSYS) {
>>  		fc->no_create = 1;
>>  		goto mknod;
>> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
>> index 89e6176abe25..f49eb1b8f2f3 100644
>> --- a/include/uapi/linux/fuse.h
>> +++ b/include/uapi/linux/fuse.h
>> @@ -243,6 +243,7 @@
>>   *
>>   *  7.46
>>   *  - add FUSE_LOOKUP_HANDLE
>> + *  - add FUSE_MKOBJ_HANDLE
>>   */
>>  
>>  #ifndef _LINUX_FUSE_H
>> @@ -677,6 +678,7 @@ enum fuse_opcode {
>>  	FUSE_COMPOUND		= 54,
>>  
>>  	FUSE_LOOKUP_HANDLE	= 55,
>> +	FUSE_MKOBJ_HANDLE	= 56,
>>  
>>  	/* CUSE specific operations */
>>  	CUSE_INIT		= 4096,
>>