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

Amir Goldstein
2026-02-25T18:06:53Z

On Wed, Feb 25, 2026 at 12:25 PM Luis Henriques <luis@igalia.com> wrote:
>
> The implementation of lookup_handle+statx compound operation extends the
> lookup operation so that a file handle is be passed into the kernel. It
> also needs to include an extra inarg, so that the parent directory file
> handle can be sent to user-space. This extra inarg is added as an extension
> header to the request.
>
> By having a separate statx including in a compound operation allows the
> attr to be dropped from the lookup_handle request, simplifying the
> traditional FUSE lookup operation.
>
> Signed-off-by: Luis Henriques <luis@igalia.com>
> ---
> fs/fuse/dir.c | 294 +++++++++++++++++++++++++++++++++++---
> fs/fuse/fuse_i.h | 23 ++-
> fs/fuse/inode.c | 48 +++++--
> fs/fuse/readdir.c | 2 +-
> include/uapi/linux/fuse.h | 23 ++-
> 5 files changed, 355 insertions(+), 35 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 5c0f1364c392..7fa8c405f1a3 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -21,6 +21,7 @@
> #include <linux/security.h>
> #include <linux/types.h>
> #include <linux/kernel.h>
> +#include <linux/exportfs.h>
>
> static bool __read_mostly allow_sys_admin_access;
> module_param(allow_sys_admin_access, bool, 0644);
> @@ -372,6 +373,47 @@ static void fuse_lookup_init(struct fuse_args *args, u64 nodeid,
> args->out_args[0].value = outarg;
> }
>
> +static int do_lookup_handle_statx(struct fuse_mount *fm, u64 parent_nodeid,
> + struct inode *parent_inode,
> + const struct qstr *name,
> + struct fuse_entry2_out *lookup_out,
> + struct fuse_statx_out *statx_out,
> + struct fuse_file_handle **fh);
> +static void fuse_statx_to_attr(struct fuse_statx *sx, struct fuse_attr *attr);
> +static int do_reval_lookup(struct fuse_mount *fm, u64 parent_nodeid,
> + const struct qstr *name, u64 *nodeid,
> + u64 *generation, u64 *attr_valid,
> + struct fuse_attr *attr, struct fuse_file_handle **fh)
> +{
> + struct fuse_entry_out entry_out;
> + struct fuse_entry2_out lookup_out;
> + struct fuse_statx_out statx_out;
> + FUSE_ARGS(lookup_args);
> + int ret = 0;
> +
> + if (fm->fc->lookup_handle) {
> + ret = do_lookup_handle_statx(fm, parent_nodeid, NULL, name,
> + &lookup_out, &statx_out, fh);
> + if (!ret) {
> + *nodeid = lookup_out.nodeid;
> + *generation = lookup_out.generation;
> + *attr_valid = fuse_time_to_jiffies(lookup_out.entry_valid,
> + lookup_out.entry_valid_nsec);
> + fuse_statx_to_attr(&statx_out.stat, attr);
> + }
> + } else {
> + fuse_lookup_init(&lookup_args, parent_nodeid, name, &entry_out);
> + ret = fuse_simple_request(fm, &lookup_args);
> + if (!ret) {
> + *nodeid = entry_out.nodeid;
> + *generation = entry_out.generation;
> + *attr_valid = ATTR_TIMEOUT(&entry_out);
> + memcpy(attr, &entry_out.attr, sizeof(*attr));
> + }
> + }
> +
> + return ret;
> +}
> /*
> * Check whether the dentry is still valid
> *
> @@ -399,10 +441,11 @@ static int fuse_dentry_revalidate(struct inode *dir, const struct qstr *name,
> goto invalid;
> else if (time_before64(fuse_dentry_time(entry), get_jiffies_64()) ||
> (flags & (LOOKUP_EXCL | LOOKUP_REVAL | LOOKUP_RENAME_TARGET))) {
> - struct fuse_entry_out outarg;
> - FUSE_ARGS(args);
> struct fuse_forget_link *forget;
> + struct fuse_file_handle *fh = NULL;
> u64 attr_version;
> + u64 nodeid, generation, attr_valid;
> + struct fuse_attr attr;
>
> /* For negative dentries, always do a fresh lookup */
> if (!inode)
> @@ -421,35 +464,36 @@ static int fuse_dentry_revalidate(struct inode *dir, const struct qstr *name,
>
> attr_version = fuse_get_attr_version(fm->fc);
>
> - fuse_lookup_init(&args, get_node_id(dir), name, &outarg);
> - ret = fuse_simple_request(fm, &args);
> + ret = do_reval_lookup(fm, get_node_id(dir), name, &nodeid,
> + &generation, &attr_valid, &attr, &fh);
> /* Zero nodeid is same as -ENOENT */
> - if (!ret && !outarg.nodeid)
> + if (!ret && !nodeid)
> ret = -ENOENT;
> if (!ret) {
> fi = get_fuse_inode(inode);
> - if (outarg.nodeid != get_node_id(inode) ||
> - (bool) IS_AUTOMOUNT(inode) != (bool) (outarg.attr.flags & FUSE_ATTR_SUBMOUNT)) {
> - fuse_queue_forget(fm->fc, forget,
> - outarg.nodeid, 1);
> + if (!fuse_file_handle_is_equal(fm->fc, fi->fh, fh) ||
> + nodeid != get_node_id(inode) ||
> + (bool) IS_AUTOMOUNT(inode) != (bool) (attr.flags & FUSE_ATTR_SUBMOUNT)) {
> + fuse_queue_forget(fm->fc, forget, nodeid, 1);
> + kfree(fh);
> goto invalid;
> }
> spin_lock(&fi->lock);
> fi->nlookup++;
> spin_unlock(&fi->lock);
> }
> + kfree(fh);
> kfree(forget);
> if (ret == -ENOMEM || ret == -EINTR)
> goto out;
> - if (ret || fuse_invalid_attr(&outarg.attr) ||
> - fuse_stale_inode(inode, outarg.generation, &outarg.attr))
> + if (ret || fuse_invalid_attr(&attr) ||
> + fuse_stale_inode(inode, generation, &attr))
> goto invalid;
>
> forget_all_cached_acls(inode);
> - fuse_change_attributes(inode, &outarg.attr, NULL,
> - ATTR_TIMEOUT(&outarg),
> + fuse_change_attributes(inode, &attr, NULL, attr_valid,
> attr_version);
> - fuse_change_entry_timeout(entry, &outarg);
> + fuse_dentry_settime(entry, attr_valid);
> } else if (inode) {
> fi = get_fuse_inode(inode);
> if (flags & LOOKUP_RCU) {
> @@ -546,8 +590,215 @@ bool fuse_invalid_attr(struct fuse_attr *attr)
> return !fuse_valid_type(attr->mode) || !fuse_valid_size(attr->size);
> }
>
> -int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name,
> - u64 *time, struct inode **inode)
> +static int create_ext_handle(struct fuse_in_arg *ext, struct fuse_inode *fi)
> +{
> + struct fuse_ext_header *xh;
> + struct fuse_file_handle *fh;
> + u32 len;
> +
> + len = fuse_ext_size(sizeof(*fi->fh) + fi->fh->size);
> + xh = fuse_extend_arg(ext, len);
> + if (!xh)
> + return -ENOMEM;
> +
> + xh->size = len;
> + xh->type = FUSE_EXT_HANDLE;
> + fh = (struct fuse_file_handle *)&xh[1];
> + fh->size = fi->fh->size;
> + memcpy(fh->handle, fi->fh->handle, fh->size);
> +
> + return 0;
> +}
> +
> +static int fuse_lookup_handle_init(struct fuse_args *args, u64 nodeid,
> + struct fuse_inode *fi,
> + const struct qstr *name,
> + struct fuse_entry2_out *outarg)
> +{
> + struct fuse_file_handle *fh;

Considering that fuse has long used uint64_t fh as the convention
for a file id all over the code, it would be better to pick a different
convention for fuse file handle, perhaps ffh, or fhandle?

> + size_t fh_size = sizeof(*fh) + MAX_HANDLE_SZ;

I don't remember what we concluded last time, but
shouldn't the server request max_handle_sz at init?
This constant is quite arbitrary.

> + int ret = -ENOMEM;
> +
> + fh = kzalloc(fh_size, GFP_KERNEL);
> + if (!fh)
> + return ret;
> +
> + memset(outarg, 0, sizeof(struct fuse_entry2_out));
> + args->opcode = FUSE_LOOKUP_HANDLE;
> + args->nodeid = nodeid;
> + args->in_numargs = 3;
> + fuse_set_zero_arg0(args);
> + args->in_args[1].size = name->len;
> + args->in_args[1].value = name->name;
> + args->in_args[2].size = 1;
> + args->in_args[2].value = "";
> + if (fi && fi->fh) {

Same here fi->ffh? or fi->fhandle

> + args->is_ext = true;
> + args->ext_idx = args->in_numargs++;
> + args->in_args[args->ext_idx].size = 0;
> + ret = create_ext_handle(&args->in_args[args->ext_idx], fi);
> + if (ret) {
> + kfree(fh);
> + return ret;
> + }
> + }
> + args->out_numargs = 2;
> + args->out_argvar = true;
> + args->out_argvar_idx = 1;
> + args->out_args[0].size = sizeof(struct fuse_entry2_out);
> + args->out_args[0].value = outarg;
> +
> + /* XXX do allocation to the actual size of the handle */
> + args->out_args[1].size = fh_size;
> + args->out_args[1].value = fh;
> +
> + return 0;
> +}
> +
> +static void fuse_req_free_argvar_ext(struct fuse_args *args)
> +{
> + if (args->out_argvar)
> + kfree(args->out_args[args->out_argvar_idx].value);
> + if (args->is_ext)
> + kfree(args->in_args[args->ext_idx].value);
> +}
> +

Just wanted to point out that statx_out is > 256 bytes on stack
so allocating 127+4 and the added complexity of ext arg
seem awkward.

Unless we really want to support huge file handles (we don't?)
maybe the allocation can be restricted to fi->handle?
Not sure.

Thanks,
Amir.