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;
+ size_t fh_size = sizeof(*fh) + MAX_HANDLE_SZ;
+ 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) {
+ 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);
+}
+
+static void fuse_statx_init(struct fuse_args *args, struct fuse_statx_in *inarg,
+ u64 nodeid, struct fuse_file *ff,
+ struct fuse_statx_out *outarg);
+static int fuse_statx_update(struct mnt_idmap *idmap,
+ struct fuse_statx_out *outarg, struct inode *inode,
+ u64 attr_version, struct kstat *stat);
+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)
+{
+ struct fuse_compound_req *compound;
+ struct fuse_inode *fi = NULL;
+ struct fuse_statx_in statx_in;
+ FUSE_ARGS(lookup_args);
+ FUSE_ARGS(statx_args);
+ int ret;
+
+ if (parent_inode)
+ fi = get_fuse_inode(parent_inode);
+
+ compound = fuse_compound_alloc(fm, 0);
+ if (!compound)
+ return -ENOMEM;
+
+ ret = fuse_lookup_handle_init(&lookup_args, parent_nodeid, fi,
+ name, lookup_out);
+ if (ret)
+ goto out_compound;
+
+ ret = fuse_compound_add(compound, &lookup_args);
+ if (ret)
+ goto out_args;
+
+ /*
+ * XXX nodeid is the parent of the inode we want! At this point
+ * we still don't have the nodeid. Using FUSE_ROOT_ID for now.
+ */
+ fuse_statx_init(&statx_args, &statx_in, FUSE_ROOT_ID, NULL, statx_out);
+ ret = fuse_compound_add(compound, &statx_args);
+ if (ret)
+ goto out_args;
+
+ ret = fuse_compound_send(compound);
+ if (ret)
+ goto out_args;
+
+ ret = fuse_compound_get_error(compound, 0);
+ if (ret || !lookup_out->nodeid)
+ goto out_args;
+ if (lookup_out->nodeid == FUSE_ROOT_ID &&
+ lookup_out->generation != 0) {
+ pr_warn_once("root generation should be zero\n");
+ lookup_out->generation = 0;
+ }
+ if ((lookup_args.out_args[1].size > 0) &&
+ (lookup_args.out_args[1].value)) {
+ struct fuse_file_handle *h = lookup_args.out_args[1].value;
+
+ *fh = kzalloc(sizeof(*fh) + h->size, GFP_KERNEL);
+ if (!*fh) {
+ ret = -ENOMEM;
+ goto out_args;
+ }
+ (*fh)->size = h->size;
+ memcpy((*fh)->handle, h->handle, (*fh)->size);
+ }
+
+ ret = fuse_compound_get_error(compound, 1);
+ if (ret) {
+ kfree(*fh);
+ *fh = NULL;
+ }
+
+out_args:
+ fuse_req_free_argvar_ext(&lookup_args);
+out_compound:
+ kfree(compound);
+
+ return ret;
+}
+
+static int fuse_lookup_handle_name(struct super_block *sb, u64 nodeid,
+ struct inode *dir, const struct qstr *name,
+ u64 *time, struct inode **inode)
+{
+ struct fuse_mount *fm = get_fuse_mount_super(sb);
+ struct fuse_file_handle *fh = NULL;
+ struct fuse_entry2_out lookup_out;
+ struct fuse_statx_out statx_out;
+ struct fuse_attr attr;
+ struct fuse_forget_link *forget;
+ u64 attr_version, evict_ctr;
+ int ret;
+
+ forget = fuse_alloc_forget();
+ if (!forget)
+ return -ENOMEM;
+
+ attr_version = fuse_get_attr_version(fm->fc);
+ evict_ctr = fuse_get_evict_ctr(fm->fc);
+
+ ret = do_lookup_handle_statx(fm, nodeid, dir, name, &lookup_out,
+ &statx_out, &fh);
+ if (ret)
+ goto out_forget;
+
+ fuse_statx_to_attr(&statx_out.stat, &attr);
+ WARN_ON(fuse_invalid_attr(&attr));
+
+ *inode = fuse_iget(sb, lookup_out.nodeid, lookup_out.generation,
+ &attr, ATTR_TIMEOUT(&statx_out),
+ attr_version, evict_ctr, fh);
+ ret = -ENOMEM;
+ if (!*inode) {
+ fuse_queue_forget(fm->fc, forget, lookup_out.nodeid, 1);
+ goto out_forget;
+ }
+ if (time)
+ *time = fuse_time_to_jiffies(lookup_out.entry_valid,
+ lookup_out.entry_valid_nsec);
+
+ /* XXX idmap? */
+ ret = fuse_statx_update(&nop_mnt_idmap, &statx_out, *inode,
+ attr_version, NULL);
+
+out_forget:
+ kfree(forget);
+
+ return ret;
+}
+
+int fuse_lookup_name(struct super_block *sb, u64 nodeid, struct inode *dir,
+ const struct qstr *name, u64 *time, struct inode **inode)
{
struct fuse_mount *fm = get_fuse_mount_super(sb);
FUSE_ARGS(args);
@@ -561,6 +812,9 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
if (name->len > fm->fc->name_max)
goto out;
+ if (fm->fc->lookup_handle)
+ return fuse_lookup_handle_name(sb, nodeid, dir, name, time,
+ inode);
forget = fuse_alloc_forget();
err = -ENOMEM;
@@ -586,7 +840,7 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
*inode = fuse_iget(sb, outarg.nodeid, outarg.generation,
&outarg.attr, ATTR_TIMEOUT(&outarg),
- attr_version, evict_ctr);
+ attr_version, evict_ctr, NULL);
err = -ENOMEM;
if (!*inode) {
fuse_queue_forget(fm->fc, forget, outarg.nodeid, 1);
@@ -621,7 +875,7 @@ static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
epoch = atomic_read(&fc->epoch);
locked = fuse_lock_inode(dir);
- err = fuse_lookup_name(dir->i_sb, get_node_id(dir), &entry->d_name,
+ err = fuse_lookup_name(dir->i_sb, get_node_id(dir), dir, &entry->d_name,
&time, &inode);
fuse_unlock_inode(dir, locked);
if (err == -ENOENT)
@@ -882,7 +1136,7 @@ static int fuse_create_open(struct mnt_idmap *idmap, struct inode *dir,
ff->nodeid = outentry.nodeid;
ff->open_flags = outopenp->open_flags;
inode = fuse_iget(dir->i_sb, outentry.nodeid, outentry.generation,
- &outentry.attr, ATTR_TIMEOUT(&outentry), 0, 0);
+ &outentry.attr, ATTR_TIMEOUT(&outentry), 0, 0, NULL);
if (!inode) {
flags &= ~(O_CREAT | O_EXCL | O_TRUNC);
fuse_sync_release(NULL, ff, flags);
@@ -1009,7 +1263,7 @@ static struct dentry *create_new_entry(struct mnt_idmap *idmap, struct fuse_moun
goto out_put_forget_req;
inode = fuse_iget(dir->i_sb, outarg.nodeid, outarg.generation,
- &outarg.attr, ATTR_TIMEOUT(&outarg), 0, 0);
+ &outarg.attr, ATTR_TIMEOUT(&outarg), 0, 0, NULL);
if (!inode) {
fuse_queue_forget(fm->fc, forget, outarg.nodeid, 1);
return ERR_PTR(-ENOMEM);
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 04f09e2ccfd0..173032887fc2 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -223,6 +223,8 @@ struct fuse_inode {
* so preserve the blocksize specified by the server.
*/
u8 cached_i_blkbits;
+
+ struct fuse_file_handle *fh;
};
/** FUSE inode state bits */
@@ -923,6 +925,9 @@ struct fuse_conn {
/* Is synchronous FUSE_INIT allowed? */
unsigned int sync_init:1;
+ /** Is LOOKUP_HANDLE implemented by the fs? */
+ unsigned int lookup_handle:1;
+
/* Use io_uring for communication */
unsigned int io_uring;
@@ -1072,6 +1077,18 @@ static inline int invalid_nodeid(u64 nodeid)
return !nodeid || nodeid == FUSE_ROOT_ID;
}
+static inline bool fuse_file_handle_is_equal(struct fuse_conn *fc,
+ struct fuse_file_handle *fh1,
+ struct fuse_file_handle *fh2)
+{
+ if (!fc->lookup_handle ||
+ ((fh1 == fh2) && !fh1) || /* both NULL */
+ (fh1 && fh2 && (fh1->size == fh2->size) &&
+ (!memcmp(fh1->handle, fh2->handle, fh1->size))))
+ return true;
+ return false;
+}
+
static inline u64 fuse_get_attr_version(struct fuse_conn *fc)
{
return atomic64_read(&fc->attr_version);
@@ -1148,10 +1165,10 @@ extern const struct dentry_operations fuse_dentry_operations;
struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
int generation, struct fuse_attr *attr,
u64 attr_valid, u64 attr_version,
- u64 evict_ctr);
+ u64 evict_ctr, struct fuse_file_handle *fh);
-int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name,
- u64 *time, struct inode **inode);
+int fuse_lookup_name(struct super_block *sb, u64 nodeid, struct inode *dir,
+ const struct qstr *name, u64 *time, struct inode **inode);
/**
* Send FORGET command
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 006436a3ad06..9f2c0c9e877c 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -120,6 +120,8 @@ static struct inode *fuse_alloc_inode(struct super_block *sb)
if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
fuse_inode_backing_set(fi, NULL);
+ fi->fh = NULL;
+
return &fi->inode;
out_free_forget:
@@ -141,6 +143,8 @@ static void fuse_free_inode(struct inode *inode)
if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
fuse_backing_put(fuse_inode_backing(fi));
+ kfree(fi->fh);
+
kmem_cache_free(fuse_inode_cachep, fi);
}
@@ -465,7 +469,7 @@ static int fuse_inode_set(struct inode *inode, void *_nodeidp)
struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
int generation, struct fuse_attr *attr,
u64 attr_valid, u64 attr_version,
- u64 evict_ctr)
+ u64 evict_ctr, struct fuse_file_handle *fh)
{
struct inode *inode;
struct fuse_inode *fi;
@@ -505,6 +509,26 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
if (!inode)
return NULL;
+ fi = get_fuse_inode(inode);
+ if (fc->lookup_handle && !fi->fh) {
+ if (!fh && (nodeid != FUSE_ROOT_ID)) {
+ pr_err("NULL file handle for nodeid %llu\n", nodeid);
+ WARN_ON_ONCE(1);
+ } else {
+ size_t sz = sizeof(struct fuse_file_handle);
+
+ if (fh)
+ sz += fh->size;
+
+ fi->fh = kzalloc(sz, GFP_KERNEL);
+ if (!fi->fh) {
+ iput(inode);
+ return NULL; // ENOMEM
+ }
+ if (fh)
+ memcpy(fi->fh, fh, sz);
+ }
+ }
if ((inode_state_read_once(inode) & I_NEW)) {
inode->i_flags |= S_NOATIME;
if (!fc->writeback_cache || !S_ISREG(attr->mode))
@@ -512,7 +536,8 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
inode->i_generation = generation;
fuse_init_inode(inode, attr, fc);
unlock_new_inode(inode);
- } else if (fuse_stale_inode(inode, generation, attr)) {
+ } else if (fuse_stale_inode(inode, generation, attr) ||
+ (!fuse_file_handle_is_equal(fc, fi->fh, fh))) {
/* nodeid was reused, any I/O on the old inode should fail */
fuse_make_bad(inode);
if (inode != d_inode(sb->s_root)) {
@@ -521,7 +546,6 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
goto retry;
}
}
- fi = get_fuse_inode(inode);
spin_lock(&fi->lock);
fi->nlookup++;
spin_unlock(&fi->lock);
@@ -1068,7 +1092,7 @@ static struct inode *fuse_get_root_inode(struct super_block *sb, unsigned int mo
attr.mode = mode;
attr.ino = FUSE_ROOT_ID;
attr.nlink = 1;
- return fuse_iget(sb, FUSE_ROOT_ID, 0, &attr, 0, 0, 0);
+ return fuse_iget(sb, FUSE_ROOT_ID, 0, &attr, 0, 0, 0, NULL);
}
struct fuse_inode_handle {
@@ -1094,7 +1118,8 @@ static struct dentry *fuse_get_dentry(struct super_block *sb,
if (!fc->export_support)
goto out_err;
- err = fuse_lookup_name(sb, handle->nodeid, &name, NULL, &inode);
+ err = fuse_lookup_name(sb, handle->nodeid, NULL, &name, NULL,
+ &inode);
if (err && err != -ENOENT)
goto out_err;
if (err || !inode) {
@@ -1115,9 +1140,9 @@ static struct dentry *fuse_get_dentry(struct super_block *sb,
return entry;
- out_iput:
+out_iput:
iput(inode);
- out_err:
+out_err:
return ERR_PTR(err);
}
@@ -1194,7 +1219,7 @@ static struct dentry *fuse_get_parent(struct dentry *child)
return ERR_PTR(-ESTALE);
err = fuse_lookup_name(child_inode->i_sb, get_node_id(child_inode),
- &dotdot_name, NULL, &inode);
+ child_inode, &dotdot_name, NULL, &inode);
if (err) {
if (err == -ENOENT)
return ERR_PTR(-ESTALE);
@@ -1459,6 +1484,9 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
if (flags & FUSE_REQUEST_TIMEOUT)
timeout = arg->request_timeout;
+
+ if (flags & FUSE_HAS_LOOKUP_HANDLE)
+ fc->lookup_handle = 1;
} else {
ra_pages = fc->max_read / PAGE_SIZE;
fc->no_lock = 1;
@@ -1509,7 +1537,7 @@ static struct fuse_init_args *fuse_new_init(struct fuse_mount *fm)
FUSE_SECURITY_CTX | FUSE_CREATE_SUPP_GROUP |
FUSE_HAS_EXPIRE_ONLY | FUSE_DIRECT_IO_ALLOW_MMAP |
FUSE_NO_EXPORT_SUPPORT | FUSE_HAS_RESEND | FUSE_ALLOW_IDMAP |
- FUSE_REQUEST_TIMEOUT;
+ FUSE_REQUEST_TIMEOUT | FUSE_LOOKUP_HANDLE;
#ifdef CONFIG_FUSE_DAX
if (fm->fc->dax)
flags |= FUSE_MAP_ALIGNMENT;
@@ -1745,7 +1773,7 @@ static int fuse_fill_super_submount(struct super_block *sb,
fuse_fill_attr_from_inode(&root_attr, parent_fi);
root = fuse_iget(sb, parent_fi->nodeid, 0, &root_attr, 0, 0,
- fuse_get_evict_ctr(fm->fc));
+ fuse_get_evict_ctr(fm->fc), NULL);
/*
* This inode is just a duplicate, so it is not looked up and
* its nlookup should not be incremented. fuse_iget() does
diff --git a/fs/fuse/readdir.c b/fs/fuse/readdir.c
index c2aae2eef086..2b59a196bcbf 100644
--- a/fs/fuse/readdir.c
+++ b/fs/fuse/readdir.c
@@ -235,7 +235,7 @@ static int fuse_direntplus_link(struct file *file,
} else {
inode = fuse_iget(dir->i_sb, o->nodeid, o->generation,
&o->attr, ATTR_TIMEOUT(o),
- attr_version, evict_ctr);
+ attr_version, evict_ctr, NULL);
if (!inode)
inode = ERR_PTR(-ENOMEM);
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 113583c4efb6..89e6176abe25 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -240,6 +240,9 @@
* - add FUSE_COPY_FILE_RANGE_64
* - add struct fuse_copy_file_range_out
* - add FUSE_NOTIFY_PRUNE
+ *
+ * 7.46
+ * - add FUSE_LOOKUP_HANDLE
*/
#ifndef _LINUX_FUSE_H
@@ -275,7 +278,7 @@
#define FUSE_KERNEL_VERSION 7
/** Minor version number of this interface */
-#define FUSE_KERNEL_MINOR_VERSION 45
+#define FUSE_KERNEL_MINOR_VERSION 46
/** The node ID of the root inode */
#define FUSE_ROOT_ID 1
@@ -495,6 +498,7 @@ struct fuse_file_lock {
#define FUSE_ALLOW_IDMAP (1ULL << 40)
#define FUSE_OVER_IO_URING (1ULL << 41)
#define FUSE_REQUEST_TIMEOUT (1ULL << 42)
+#define FUSE_HAS_LOOKUP_HANDLE (1ULL << 43)
/**
* CUSE INIT request/reply flags
@@ -609,6 +613,7 @@ enum fuse_ext_type {
/* Types 0..31 are reserved for fuse_secctx_header */
FUSE_MAX_NR_SECCTX = 31,
FUSE_EXT_GROUPS = 32,
+ FUSE_EXT_HANDLE = 33,
};
enum fuse_opcode {
@@ -671,6 +676,8 @@ enum fuse_opcode {
*/
FUSE_COMPOUND = 54,
+ FUSE_LOOKUP_HANDLE = 55,
+
/* CUSE specific operations */
CUSE_INIT = 4096,
@@ -707,6 +714,20 @@ struct fuse_entry_out {
struct fuse_attr attr;
};
+struct fuse_entry2_out {
+ uint64_t nodeid;
+ uint64_t generation;
+ uint64_t entry_valid;
+ uint32_t entry_valid_nsec;
+ uint32_t flags;
+ uint64_t spare;
+};
+
+struct fuse_file_handle {
+ uint16_t size;
+ uint8_t handle[];
+};
+
struct fuse_forget_in {
uint64_t nlookup;
};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.
Hi Amir,
On Wed, Feb 25 2026, Amir Goldstein wrote:
> 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?
Good point, I'll make sure next revision will follow a different
convention.
>> + 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.
You're right, I should have pointed that out in the cover letter at least.
In the previous version that maximum size was indeed provided by the
server. But from the discussion here [0] I understood that this
negotiation should be dropped. Here's what Miklos suggested:
> How about allocating variable length arguments on demand? That would
> allow getting rid of max_handle_size negotiation.
>
> args->out_var_alloc = true;
> args->out_args[1].size = MAX_HANDLE_SZ;
> args->out_args[1].value = NULL; /* Will be allocated to the actual size of the handle */
Obviously that's not what the code is currently doing. The plan is to
eventually set the .value to NULL and do the allocation elsewhere,
according to the actual size returned.
Because I didn't yet thought how/where the allocation could be done
instead, this code is currently simplifying things, and that's why I
picked this MAX_HANDLE_SZ.
Sorry, I should have pointed that out at in a comment as well.
[0] https://lore.kernel.org/all/CAJfpegszP+2XA=vADK4r09KU30BQd-r9sNu2Dog88yLG8iV7WQ@mail.gmail.com
>> + 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
Ack!
>> + 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.
If I understand you correctly, you're suggesting that the out_arg that
will return the handle should be handled on the stack as well and then it
would be copied to an allocated fi->handle. Sure, that can be done.
On the other hand, as I mentioned above, the outarg allocation is just a
simplification. So maybe the actual allocation of the handle may be done
elsewhere with the _actual_ fh size, and then simply used in fh->handle.
Please let me know if I got your comment right.
(And thanks for the comments, by the way!)
Cheers,
--
Luís
On Thu, Feb 26, 2026 at 10:54 AM Luis Henriques <luis@igalia.com> wrote:
>
> Hi Amir,
>
> On Wed, Feb 25 2026, Amir Goldstein wrote:
>
> > 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?
>
> Good point, I'll make sure next revision will follow a different
> convention.
>
> >> + 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.
>
> You're right, I should have pointed that out in the cover letter at least.
> In the previous version that maximum size was indeed provided by the
> server. But from the discussion here [0] I understood that this
> negotiation should be dropped. Here's what Miklos suggested:
>
> > How about allocating variable length arguments on demand? That would
> > allow getting rid of max_handle_size negotiation.
> >
> > args->out_var_alloc = true;
> > args->out_args[1].size = MAX_HANDLE_SZ;
> > args->out_args[1].value = NULL; /* Will be allocated to the actual size of the handle */
>
> Obviously that's not what the code is currently doing. The plan is to
> eventually set the .value to NULL and do the allocation elsewhere,
> according to the actual size returned.
>
> Because I didn't yet thought how/where the allocation could be done
> instead, this code is currently simplifying things, and that's why I
> picked this MAX_HANDLE_SZ.
>
> Sorry, I should have pointed that out at in a comment as well.
>
> [0] https://lore.kernel.org/all/CAJfpegszP+2XA=vADK4r09KU30BQd-r9sNu2Dog88yLG8iV7WQ@mail.gmail.com
>
> >> + 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
>
> Ack!
>
> >> + 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.
>
> If I understand you correctly, you're suggesting that the out_arg that
> will return the handle should be handled on the stack as well and then it
> would be copied to an allocated fi->handle. Sure, that can be done.
>
> On the other hand, as I mentioned above, the outarg allocation is just a
> simplification. So maybe the actual allocation of the handle may be done
> elsewhere with the _actual_ fh size, and then simply used in fh->handle.
>
> Please let me know if I got your comment right.
> (And thanks for the comments, by the way!)
file handle on stack only makes sense for small pre allocated size.
If the server has full control over handle size, then that is not relevant.
At some point we will need to address the fact that the most common
case is for very small file handles.
In struct fanotify_fid_event, we used a small inline buffer to optimize this
case. This could also be done for fuse_inode::handle, but we can worry about
that later.
Thanks,
Amir.
On Thu, Feb 26 2026, Amir Goldstein wrote:
> On Thu, Feb 26, 2026 at 10:54 AM Luis Henriques <luis@igalia.com> wrote:
>>
>> Hi Amir,
>>
>> On Wed, Feb 25 2026, Amir Goldstein wrote:
>>
>> > 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?
>>
>> Good point, I'll make sure next revision will follow a different
>> convention.
>>
>> >> + 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.
>>
>> You're right, I should have pointed that out in the cover letter at least.
>> In the previous version that maximum size was indeed provided by the
>> server. But from the discussion here [0] I understood that this
>> negotiation should be dropped. Here's what Miklos suggested:
>>
>> > How about allocating variable length arguments on demand? That would
>> > allow getting rid of max_handle_size negotiation.
>> >
>> > args->out_var_alloc = true;
>> > args->out_args[1].size = MAX_HANDLE_SZ;
>> > args->out_args[1].value = NULL; /* Will be allocated to the actual size of the handle */
>>
>> Obviously that's not what the code is currently doing. The plan is to
>> eventually set the .value to NULL and do the allocation elsewhere,
>> according to the actual size returned.
>>
>> Because I didn't yet thought how/where the allocation could be done
>> instead, this code is currently simplifying things, and that's why I
>> picked this MAX_HANDLE_SZ.
>>
>> Sorry, I should have pointed that out at in a comment as well.
>>
>> [0] https://lore.kernel.org/all/CAJfpegszP+2XA=vADK4r09KU30BQd-r9sNu2Dog88yLG8iV7WQ@mail.gmail.com
>>
>> >> + 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
>>
>> Ack!
>>
>> >> + 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.
>>
>> If I understand you correctly, you're suggesting that the out_arg that
>> will return the handle should be handled on the stack as well and then it
>> would be copied to an allocated fi->handle. Sure, that can be done.
>>
>> On the other hand, as I mentioned above, the outarg allocation is just a
>> simplification. So maybe the actual allocation of the handle may be done
>> elsewhere with the _actual_ fh size, and then simply used in fh->handle.
>>
>> Please let me know if I got your comment right.
>> (And thanks for the comments, by the way!)
>
> file handle on stack only makes sense for small pre allocated size.
> If the server has full control over handle size, then that is not relevant.
>
> At some point we will need to address the fact that the most common
> case is for very small file handles.
>
> In struct fanotify_fid_event, we used a small inline buffer to optimize this
> case. This could also be done for fuse_inode::handle, but we can worry about
> that later.
Thanks, I had took a look into it before -- I think you had pointed it to
me!. But I agree that this is something that can be handled once I have
most of the other things sorted out.
Cheers,
--
Luís
On Thu, 26 Feb 2026 at 11:08, Amir Goldstein <amir73il@gmail.com> wrote:
> file handle on stack only makes sense for small pre allocated size.
> If the server has full control over handle size, then that is not relevant.
I thought the point was that the file handle is available in
fi->handle and doesn't need to be allocated/copied. Instead
extensions could be done with an argument vector, like this:
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -326,6 +326,12 @@ struct fuse_folio_desc {
unsigned int offset;
};
+struct fuse_ext_arg {
+ u32 type;
+ u32 size;
+ const void *value;
+};
+
struct fuse_args {
uint64_t nodeid;
uint32_t opcode;
@@ -346,6 +352,7 @@ struct fuse_args {
bool is_pinned:1;
bool invalidate_vmap:1;
struct fuse_in_arg in_args[4];
+ struct fuse_ext_arg ext_args[2];
struct fuse_arg out_args[2];
void (*end)(struct fuse_mount *fm, struct fuse_args *args, int error);
/* Used for kvec iter backed by vmalloc address */
Thanks,
Miklos
On Thu, Feb 26 2026, Miklos Szeredi wrote:
> On Thu, 26 Feb 2026 at 11:08, Amir Goldstein <amir73il@gmail.com> wrote:
>
>> file handle on stack only makes sense for small pre allocated size.
>> If the server has full control over handle size, then that is not relevant.
>
> I thought the point was that the file handle is available in
> fi->handle and doesn't need to be allocated/copied. Instead
> extensions could be done with an argument vector, like this:
Right now the code is using extensions in the lookup_handle operation
inargs, and only if a file handle is available for the parent inode.
Are you saying that outargs should also use extensions for getting the
file handle in a lookup_handle?
Cheers,
--
Luís
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -326,6 +326,12 @@ struct fuse_folio_desc {
> unsigned int offset;
> };
>
> +struct fuse_ext_arg {
> + u32 type;
> + u32 size;
> + const void *value;
> +};
> +
> struct fuse_args {
> uint64_t nodeid;
> uint32_t opcode;
> @@ -346,6 +352,7 @@ struct fuse_args {
> bool is_pinned:1;
> bool invalidate_vmap:1;
> struct fuse_in_arg in_args[4];
> + struct fuse_ext_arg ext_args[2];
> struct fuse_arg out_args[2];
> void (*end)(struct fuse_mount *fm, struct fuse_args *args, int error);
> /* Used for kvec iter backed by vmalloc address */
>
> Thanks,
> Miklos
On Thu, 26 Feb 2026 at 16:07, Luis Henriques <luis@igalia.com> wrote: > Are you saying that outargs should also use extensions for getting the > file handle in a lookup_handle? No. I'm saying that extend_arg() thing is messy and a using vectored args for extensions (same as we do for normal input arguments) might be better. Thanks, Miklos
On Thu, Feb 26 2026, Miklos Szeredi wrote: > On Thu, 26 Feb 2026 at 16:07, Luis Henriques <luis@igalia.com> wrote: > >> Are you saying that outargs should also use extensions for getting the >> file handle in a lookup_handle? > > No. > > I'm saying that extend_arg() thing is messy and a using vectored args > for extensions (same as we do for normal input arguments) might be > better. It is indeed a messy interface. I'll have a look into it and see how to modify it to accommodate your suggestion. And thanks for your patience ;-) Cheers, -- Luís
© 2016 - 2026 Red Hat, Inc.