public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Horst Birthelmer <horst@birthelmer.de>
To: Luis Henriques <luis@igalia.com>
Cc: Miklos Szeredi <miklos@szeredi.hu>,
	 Amir Goldstein <amir73il@gmail.com>,
	Bernd Schubert <bschubert@ddn.com>,
	 Bernd Schubert <bernd@bsbernd.com>,
	"Darrick J. Wong" <djwong@kernel.org>,
	 Horst Birthelmer <hbirthelmer@ddn.com>,
	Joanne Koong <joannelkoong@gmail.com>,
	 Kevin Chen <kchen@ddn.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	 Matt Harvey <mharvey@jumptrading.com>,
	kernel-dev@igalia.com
Subject: Re: [RFC PATCH v3 8/8] fuse: implementation of mkobj_handle+statx+open compound operation
Date: Wed, 25 Feb 2026 16:08:10 +0100	[thread overview]
Message-ID: <aZ8O2ohfGEgqE6TT@fedora.fritz.box> (raw)
In-Reply-To: <20260225112439.27276-9-luis@igalia.com>

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?

> +	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 ;-)

> +	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?

> +	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,
> 

  reply	other threads:[~2026-02-25 15:16 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-25 11:24 [RFC PATCH v3 0/8] fuse: LOOKUP_HANDLE operation Luis Henriques
2026-02-25 11:24 ` [RFC PATCH v3 1/8] fuse: simplify fuse_lookup_name() interface Luis Henriques
2026-02-27 15:46   ` Miklos Szeredi
2026-02-28 14:42     ` Luis Henriques
2026-02-25 11:24 ` [RFC PATCH v3 2/8] fuse: export extend_arg() and factor out fuse_ext_size() Luis Henriques
2026-02-25 11:24 ` [RFC PATCH v3 3/8] fuse: store index of the variable length argument Luis Henriques
2026-02-27 15:41   ` Miklos Szeredi
2026-02-28 14:50     ` Luis Henriques
2026-02-25 11:24 ` [RFC PATCH v3 4/8] fuse: drop unnecessary argument from fuse_lookup_init() Luis Henriques
2026-02-27 15:57   ` Miklos Szeredi
2026-02-25 11:24 ` [RFC PATCH v3 5/8] fuse: extract helper functions from fuse_do_statx() Luis Henriques
2026-02-25 11:24 ` [RFC PATCH v3 6/8] fuse: implementation of lookup_handle+statx compound operation Luis Henriques
2026-02-25 18:06   ` Amir Goldstein
2026-02-26  9:54     ` Luis Henriques
2026-02-26 10:08       ` Amir Goldstein
2026-02-26 10:29         ` Miklos Szeredi
2026-02-26 15:06           ` Luis Henriques
2026-02-26 15:44             ` Miklos Szeredi
2026-02-26 16:17               ` Luis Henriques
2026-02-26 10:33         ` Luis Henriques
2026-02-25 11:24 ` [RFC PATCH v3 7/8] fuse: export fuse_open_args_fill() helper function Luis Henriques
2026-02-25 11:24 ` [RFC PATCH v3 8/8] fuse: implementation of mkobj_handle+statx+open compound operation Luis Henriques
2026-02-25 15:08   ` Horst Birthelmer [this message]
2026-02-25 17:26     ` Luis Henriques
2026-02-25 15:14 ` [RFC PATCH v3 0/8] fuse: LOOKUP_HANDLE operation Horst Birthelmer
2026-02-25 17:06   ` Luis Henriques

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aZ8O2ohfGEgqE6TT@fedora.fritz.box \
    --to=horst@birthelmer.de \
    --cc=amir73il@gmail.com \
    --cc=bernd@bsbernd.com \
    --cc=bschubert@ddn.com \
    --cc=djwong@kernel.org \
    --cc=hbirthelmer@ddn.com \
    --cc=joannelkoong@gmail.com \
    --cc=kchen@ddn.com \
    --cc=kernel-dev@igalia.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luis@igalia.com \
    --cc=mharvey@jumptrading.com \
    --cc=miklos@szeredi.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox