linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/2] RFC: Extend fuse-passthrough to directories
@ 2025-06-17 22:14 Paul Lawrence
  2025-06-17 22:14 ` [PATCH v1 1/2] fuse: Add backing file option to lookup Paul Lawrence
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Paul Lawrence @ 2025-06-17 22:14 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-kernel, linux-fsdevel, Paul Lawrence

This is the first part of a much larger patch set that would allow a directory
to be marked ‘passthrough’. At a high level, the fuse daemon can return an
optional extra argument to FUSE_LOOKUP that contains an fd. Extra fields are
added to the fuse_dentry, fuse_inode and fuse_file structs to have a backing
path, inode and file respectively. When fuse is performing an operation, it will
check for the existence of a backing object and if it exists forward the
operation to the backing object.

These two patches add the core infrastructure, handling of the extra argument
response to lookup, and forwarding open, flush and close to the backing file.
This is sufficient to validate the concept.

The questions I have:

* Is this something that interests the fuse maintainers and community?
* Is this approach the correct one?
* (if we agree yes to 1 & 2) Detailed analysis of the patches for errors.

A few observations:

I have matched backing objects to their fuse objects. Currently fuse passthrough
puts a backing file into the fuse inode. I’m not quite sure why this was done -
it seems to have been a very late change in the passthrough patch sets which
happened without comment. It does not really make sense for full directory
passthrough since unopened inodes still need to have backing inodes. The one
advantage I can see is that it reduces the number of opens/closes of the backing
file. However, this may also be a disadvantage - it moves closes, in particular,
to an arbitrary point when the inode is flushed from cache.

Backing operations need to happen in the context of the daemon, not the caller.
(I am a firm believer of this principle.) This is not yet implemented, and is
not (currently, and unfortunately) the way Android uses passthough. It is not
hard to do, and if these patches are otherwise acceptable, will be added.

There was a long discussion about the security issues of using an fd returned
from the fuse daemon in the context of fuse passthrough, and the end solution
was to use an ioctl to set the backing file. I have used the previously-rejected
approach of passing the fd in a struct in the fuse_daemon response. My defense
of this approach is

* The fd is simply used to pull out the path and inode
* All operations are revalidated
* Thus there is no risk even if a privileged process with a protected fd is
tricked into passing that fd back in this structure.

I’m sure we will discuss this at length if this patch set is otherwise deemed
valuable, and I am certainly not wedded to this approach.

I have written tests to validate this approach using tools/testing/selftests. I
don’t want this patch set to get derailed by a discussion of the way I wrote the
tests, so I have not included them. I am very open to any and every suggestion
as to how (and where) tests should be written for these patches.

Paul Lawrence (2):
  fuse: Add backing file option to lookup
  fuse: open/close backing file

 fs/fuse/Kconfig           |  13 +++++
 fs/fuse/Makefile          |   1 +
 fs/fuse/backing.c         |  97 ++++++++++++++++++++++++++++++++
 fs/fuse/dev.c             |  14 +++++
 fs/fuse/dir.c             | 108 +++++++++++++++++++++++++-----------
 fs/fuse/file.c            |  34 ++++++++----
 fs/fuse/fuse_i.h          |  61 +++++++++++++++++++-
 fs/fuse/inode.c           | 114 ++++++++++++++++++++++++++++++++++----
 include/uapi/linux/fuse.h |   4 ++
 9 files changed, 392 insertions(+), 54 deletions(-)
 create mode 100644 fs/fuse/backing.c

-- 
2.49.0.1112.g889b7c5bd8-goog


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v1 1/2] fuse: Add backing file option to lookup
  2025-06-17 22:14 [PATCH v1 0/2] RFC: Extend fuse-passthrough to directories Paul Lawrence
@ 2025-06-17 22:14 ` Paul Lawrence
  2025-06-18 13:46   ` kernel test robot
  2025-06-17 22:14 ` [PATCH v1 2/2] fuse: open/close backing file Paul Lawrence
  2025-06-18 11:01 ` [PATCH v1 0/2] RFC: Extend fuse-passthrough to directories Amir Goldstein
  2 siblings, 1 reply; 19+ messages in thread
From: Paul Lawrence @ 2025-06-17 22:14 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-kernel, linux-fsdevel, Paul Lawrence

This commit starts the process of adding directory passthrough to fuse.

* Add the Kconfig option CONFIG_FUSE_PASSTHROUGH_DIR
* Add struct fuse_entry_backing_out, an optional second out arg to
  FUSE_LOOKUP. This will contain the fd to the backing file or
  directory
* Synchronously store the backing file in the args
* Add backing inode to fuse inode and backing path to fuse dentry
* If FUSE_LOOKUP returns a backing fd, store path and inode

Signed-off-by: Paul Lawrence <paullawrence@google.com>
---
 fs/fuse/Kconfig           |  13 +++++
 fs/fuse/Makefile          |   1 +
 fs/fuse/backing.c         |  29 ++++++++++
 fs/fuse/dev.c             |  14 +++++
 fs/fuse/dir.c             | 114 +++++++++++++++++++++++++++++---------
 fs/fuse/fuse_i.h          |  22 +++++++-
 fs/fuse/inode.c           | 114 ++++++++++++++++++++++++++++++++++----
 include/uapi/linux/fuse.h |   4 ++
 8 files changed, 273 insertions(+), 38 deletions(-)
 create mode 100644 fs/fuse/backing.c

diff --git a/fs/fuse/Kconfig b/fs/fuse/Kconfig
index ca215a3cba3e..b8a05cb427df 100644
--- a/fs/fuse/Kconfig
+++ b/fs/fuse/Kconfig
@@ -75,3 +75,16 @@ config FUSE_IO_URING
 
 	  If you want to allow fuse server/client communication through io-uring,
 	  answer Y
+
+config FUSE_PASSTHROUGH_DIR
+	bool "FUSE directory passthrough operations support"
+	depends on FUSE_FS
+	select FS_STACK
+	help
+	  This allows bypassing FUSE server on a specified directory by mapping
+	  all FUSE operations to be performed directly on a backing directory.
+
+	  The bypass will automatically extend to all sub directories and files
+	  in that directory.
+
+	  If you want to allow directory passthrough operations, answer Y.
diff --git a/fs/fuse/Makefile b/fs/fuse/Makefile
index 3f0f312a31c1..79ca3a68b993 100644
--- a/fs/fuse/Makefile
+++ b/fs/fuse/Makefile
@@ -16,5 +16,6 @@ fuse-$(CONFIG_FUSE_DAX) += dax.o
 fuse-$(CONFIG_FUSE_PASSTHROUGH) += passthrough.o
 fuse-$(CONFIG_SYSCTL) += sysctl.o
 fuse-$(CONFIG_FUSE_IO_URING) += dev_uring.o
+fuse-$(CONFIG_FUSE_PASSTHROUGH_DIR) += backing.o
 
 virtiofs-y := virtio_fs.o
diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
new file mode 100644
index 000000000000..1dcc617bf660
--- /dev/null
+++ b/fs/fuse/backing.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * FUSE-BPF: Filesystem in Userspace with BPF
+ * Copyright (c) 2021 Google LLC
+ */
+
+#include "fuse_i.h"
+
+int fuse_handle_backing(struct fuse_entry_backing *feb,
+	struct inode **backing_inode, struct path *backing_path)
+{
+	struct file *backing_file = feb->backing_file;
+
+	if (!backing_file)
+		return -EINVAL;
+	if (IS_ERR(backing_file))
+		return PTR_ERR(backing_file);
+
+	if (backing_inode)
+		iput(*backing_inode);
+	*backing_inode = backing_file->f_inode;
+	ihold(*backing_inode);
+
+	path_put(backing_path);
+	*backing_path = backing_file->f_path;
+	path_get(backing_path);
+
+	return 0;
+}
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 6dcbaa218b7a..db1fbd1fdb85 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -2181,6 +2181,20 @@ static ssize_t fuse_dev_do_write(struct fuse_dev *fud,
 		err = fuse_copy_out_args(cs, req->args, nbytes);
 	fuse_copy_finish(cs);
 
+	if (!err && req->in.h.opcode == FUSE_LOOKUP &&
+			req->args->out_args[1].size ==
+			sizeof(struct fuse_entry_backing_out)) {
+		struct fuse_entry_backing_out *febo =
+			(struct fuse_entry_backing_out *)
+				req->args->out_args[1].value;
+		struct fuse_entry_backing *feb =
+			container_of(febo, struct fuse_entry_backing, out);
+
+		feb->backing_file = fget(febo->backing_fd);
+		if (!feb->backing_file)
+			err = -EBADFD;
+	}
+
 	spin_lock(&fpq->lock);
 	clear_bit(FR_LOCKED, &req->flags);
 	if (!fpq->connected)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 83ac192e7fdd..909463fae94d 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -34,7 +34,7 @@ static void fuse_advise_use_readdirplus(struct inode *dir)
 	set_bit(FUSE_I_ADVISE_RDPLUS, &fi->state);
 }
 
-#if BITS_PER_LONG >= 64
+#if BITS_PER_LONG >= 64 && !defined(CONFIG_FUSE_PASSTHROUGH_DIR)
 static inline void __fuse_dentry_settime(struct dentry *entry, u64 time)
 {
 	entry->d_fsdata = (void *) time;
@@ -47,7 +47,12 @@ static inline u64 fuse_dentry_time(const struct dentry *entry)
 
 #else
 union fuse_dentry {
-	u64 time;
+	struct {
+		u64 time;
+#ifdef CONFIG_FUSE_PASSTHROUGH_DIR
+		struct path backing_path;
+#endif
+	};
 	struct rcu_head rcu;
 };
 
@@ -60,6 +65,11 @@ static inline u64 fuse_dentry_time(const struct dentry *entry)
 {
 	return ((union fuse_dentry *) entry->d_fsdata)->time;
 }
+
+static inline union fuse_dentry *get_fuse_dentry(const struct dentry *entry)
+{
+	return entry->d_fsdata;
+}
 #endif
 
 static void fuse_dentry_settime(struct dentry *dentry, u64 time)
@@ -170,7 +180,8 @@ static void fuse_invalidate_entry(struct dentry *entry)
 
 static void fuse_lookup_init(struct fuse_conn *fc, struct fuse_args *args,
 			     u64 nodeid, const struct qstr *name,
-			     struct fuse_entry_out *outarg)
+			     struct fuse_entry_out *outarg,
+			     struct fuse_entry_backing_out *febo)
 {
 	memset(outarg, 0, sizeof(struct fuse_entry_out));
 	args->opcode = FUSE_LOOKUP;
@@ -181,9 +192,12 @@ static void fuse_lookup_init(struct fuse_conn *fc, struct fuse_args *args,
 	args->in_args[1].value = name->name;
 	args->in_args[2].size = 1;
 	args->in_args[2].value = "";
-	args->out_numargs = 1;
+	args->out_argvar = true;
+	args->out_numargs = 2;
 	args->out_args[0].size = sizeof(struct fuse_entry_out);
 	args->out_args[0].value = outarg;
+	args->out_args[1].size = sizeof(struct fuse_entry_backing_out);
+	args->out_args[1].value = febo;
 }
 
 /*
@@ -209,6 +223,7 @@ static int fuse_dentry_revalidate(struct inode *dir, const struct qstr *name,
 	else if (time_before64(fuse_dentry_time(entry), get_jiffies_64()) ||
 		 (flags & (LOOKUP_EXCL | LOOKUP_REVAL | LOOKUP_RENAME_TARGET))) {
 		struct fuse_entry_out outarg;
+		struct fuse_entry_backing feb;
 		FUSE_ARGS(args);
 		struct fuse_forget_link *forget;
 		u64 attr_version;
@@ -231,7 +246,7 @@ static int fuse_dentry_revalidate(struct inode *dir, const struct qstr *name,
 		attr_version = fuse_get_attr_version(fm->fc);
 
 		fuse_lookup_init(fm->fc, &args, get_node_id(dir),
-				 name, &outarg);
+				 name, &outarg, &feb.out);
 		ret = fuse_simple_request(fm, &args);
 		/* Zero nodeid is same as -ENOENT */
 		if (!ret && !outarg.nodeid)
@@ -278,7 +293,7 @@ static int fuse_dentry_revalidate(struct inode *dir, const struct qstr *name,
 	goto out;
 }
 
-#if BITS_PER_LONG < 64
+#if BITS_PER_LONG < 64 || defined(CONFIG_FUSE_PASSTHROUGH_DIR)
 static int fuse_dentry_init(struct dentry *dentry)
 {
 	dentry->d_fsdata = kzalloc(sizeof(union fuse_dentry),
@@ -290,6 +305,11 @@ static void fuse_dentry_release(struct dentry *dentry)
 {
 	union fuse_dentry *fd = dentry->d_fsdata;
 
+#ifdef CONFIG_FUSE_PASSTHROUGH_DIR
+	if (fd && fd->backing_path.dentry)
+		path_put(&fd->backing_path);
+#endif
+
 	kfree_rcu(fd, rcu);
 }
 #endif
@@ -329,7 +349,7 @@ static struct vfsmount *fuse_dentry_automount(struct path *path)
 const struct dentry_operations fuse_dentry_operations = {
 	.d_revalidate	= fuse_dentry_revalidate,
 	.d_delete	= fuse_dentry_delete,
-#if BITS_PER_LONG < 64
+#if BITS_PER_LONG < 64 || defined(CONFIG_FUSE_PASSTHROUGH_DIR)
 	.d_init		= fuse_dentry_init,
 	.d_release	= fuse_dentry_release,
 #endif
@@ -360,10 +380,12 @@ bool fuse_invalid_attr(struct fuse_attr *attr)
 }
 
 int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name,
-		     struct fuse_entry_out *outarg, struct inode **inode)
+		     struct fuse_entry_out *outarg, struct dentry *entry,
+		     struct inode **inode)
 {
 	struct fuse_mount *fm = get_fuse_mount_super(sb);
 	FUSE_ARGS(args);
+	struct fuse_entry_backing backing_arg = {0};
 	struct fuse_forget_link *forget;
 	u64 attr_version, evict_ctr;
 	int err;
@@ -382,23 +404,61 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
 	attr_version = fuse_get_attr_version(fm->fc);
 	evict_ctr = fuse_get_evict_ctr(fm->fc);
 
-	fuse_lookup_init(fm->fc, &args, nodeid, name, outarg);
+	fuse_lookup_init(fm->fc, &args, nodeid, name, outarg, &backing_arg.out);
 	err = fuse_simple_request(fm, &args);
-	/* Zero nodeid is same as -ENOENT, but with valid timeout */
-	if (err || !outarg->nodeid)
-		goto out_put_forget;
 
-	err = -EIO;
-	if (fuse_invalid_attr(&outarg->attr))
-		goto out_put_forget;
-	if (outarg->nodeid == FUSE_ROOT_ID && outarg->generation != 0) {
-		pr_warn_once("root generation should be zero\n");
-		outarg->generation = 0;
+#ifdef CONFIG_FUSE_PASSTHROUGH_DIR
+	if (err == sizeof(backing_arg.out)) {
+		struct file *backing_file;
+		struct inode *backing_inode;
+
+		err = -ENOENT;
+		if (!entry)
+			goto out_put_forget;
+
+		err = -EINVAL;
+		backing_file = backing_arg.backing_file;
+		if (!backing_file)
+			goto out_put_forget;
+
+		if (IS_ERR(backing_file)) {
+			err = PTR_ERR(backing_file);
+			goto out_put_forget;
+		}
+
+		backing_inode = backing_file->f_inode;
+		*inode = fuse_iget_backing(sb, backing_inode);
+		if (!*inode)
+			goto out_put_forget;
+
+		err = fuse_handle_backing(&backing_arg,
+				&get_fuse_inode(*inode)->backing_inode,
+				&get_fuse_dentry(entry)->backing_path);
+		if (err) {
+			iput(*inode);
+			*inode = NULL;
+			goto out_put_forget;
+		}
+	} else
+#endif
+	{
+		/* Zero nodeid is same as -ENOENT, but with valid timeout */
+		if (err || !outarg->nodeid)
+			goto out_put_forget;
+
+		err = -EIO;
+		if (fuse_invalid_attr(&outarg->attr))
+			goto out_put_forget;
+		if (outarg->nodeid == FUSE_ROOT_ID && outarg->generation != 0) {
+			pr_warn_once("root generation should be zero\n");
+			outarg->generation = 0;
+		}
+
+		*inode = fuse_iget(sb, outarg->nodeid, outarg->generation,
+				&outarg->attr, ATTR_TIMEOUT(outarg),
+				attr_version, evict_ctr);
 	}
 
-	*inode = fuse_iget(sb, outarg->nodeid, outarg->generation,
-			   &outarg->attr, ATTR_TIMEOUT(outarg),
-			   attr_version, evict_ctr);
 	err = -ENOMEM;
 	if (!*inode) {
 		fuse_queue_forget(fm->fc, forget, outarg->nodeid, 1);
@@ -406,9 +466,11 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
 	}
 	err = 0;
 
- out_put_forget:
+out_put_forget:
 	kfree(forget);
- out:
+out:
+	if (backing_arg.backing_file)
+		fput(backing_arg.backing_file);
 	return err;
 }
 
@@ -427,7 +489,7 @@ static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
 
 	locked = fuse_lock_inode(dir);
 	err = fuse_lookup_name(dir->i_sb, get_node_id(dir), &entry->d_name,
-			       &outarg, &inode);
+			       &outarg, entry, &inode);
 	fuse_unlock_inode(dir, locked);
 	if (err == -ENOENT) {
 		outarg_valid = false;
@@ -455,9 +517,9 @@ static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
 		fuse_advise_use_readdirplus(dir);
 	return newent;
 
- out_iput:
+out_iput:
 	iput(inode);
- out_err:
+out_err:
 	return ERR_PTR(err);
 }
 
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index d56d4fd956db..1dc04bc6ac49 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -106,6 +106,11 @@ struct fuse_backing {
 	struct rcu_head rcu;
 };
 
+struct fuse_entry_backing {
+	struct fuse_entry_backing_out out;
+	struct file *backing_file;
+};
+
 /** FUSE inode */
 struct fuse_inode {
 	/** Inode data */
@@ -213,6 +218,14 @@ struct fuse_inode {
 	/** Reference to backing file in passthrough mode */
 	struct fuse_backing *fb;
 #endif
+
+#ifdef CONFIG_FUSE_PASSTHROUGH_DIR
+	/**
+	 * Backing inode, if this inode is from a backing file system.
+	 * If this is set, nodeid is 0.
+	 */
+	struct inode *backing_inode;
+#endif
 };
 
 /** FUSE inode state bits */
@@ -1114,13 +1127,17 @@ extern const struct dentry_operations fuse_root_dentry_operations;
 /**
  * Get a filled in inode
  */
+struct inode *fuse_iget_backing(struct super_block *sb,
+			struct inode *backing_inode);
+
 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);
 
 int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name,
-		     struct fuse_entry_out *outarg, struct inode **inode);
+		     struct fuse_entry_out *outarg, struct dentry *entry,
+		     struct inode **inode);
 
 /**
  * Send FORGET command
@@ -1577,4 +1594,7 @@ extern void fuse_sysctl_unregister(void);
 #define fuse_sysctl_unregister()	do { } while (0)
 #endif /* CONFIG_SYSCTL */
 
+/* backing.c */
+int fuse_handle_backing(struct fuse_entry_backing *feb,
+	struct inode **backing_inode, struct path *backing_path);
 #endif /* _FS_FUSE_I_H */
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index fd48e8d37f2e..404d8cbe5f25 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -35,6 +35,8 @@ struct list_head fuse_conn_list;
 DEFINE_MUTEX(fuse_mutex);
 
 static int set_global_limit(const char *val, const struct kernel_param *kp);
+static void fuse_fill_attr_from_inode(struct fuse_attr *attr,
+	const struct fuse_inode *fi);
 
 unsigned int fuse_max_pages_limit = 256;
 /* default is no timeout */
@@ -195,6 +197,16 @@ static void fuse_evict_inode(struct inode *inode)
 	}
 }
 
+#ifdef CONFIG_FUSE_PASSTHROUGH_DIR
+static void fuse_destroy_inode(struct inode *inode)
+{
+	struct fuse_inode *fi = get_fuse_inode(inode);
+
+	if (fi->backing_inode)
+		iput(fi->backing_inode);
+}
+#endif
+
 static int fuse_reconfigure(struct fs_context *fsc)
 {
 	struct super_block *sb = fsc->root->d_sb;
@@ -443,22 +455,93 @@ static void fuse_init_inode(struct inode *inode, struct fuse_attr *attr,
 		inode->i_acl = inode->i_default_acl = ACL_DONT_CACHE;
 }
 
+struct fuse_inode_identifier {
+	u64 nodeid;
+	struct inode *backing_inode;
+};
+
 static int fuse_inode_eq(struct inode *inode, void *_nodeidp)
 {
-	u64 nodeid = *(u64 *) _nodeidp;
-	if (get_node_id(inode) == nodeid)
-		return 1;
-	else
-		return 0;
+	struct fuse_inode_identifier *fii =
+		(struct fuse_inode_identifier *) _nodeidp;
+	struct fuse_inode *fi = get_fuse_inode(inode);
+
+	return fii->nodeid == fi->nodeid;
+}
+
+static int fuse_inode_backing_eq(struct inode *inode, void *_nodeidp)
+{
+	struct fuse_inode_identifier *fii =
+		(struct fuse_inode_identifier *) _nodeidp;
+	struct fuse_inode *fi = get_fuse_inode(inode);
+
+	return fii->nodeid == fi->nodeid
+		&& fii->backing_inode == fi->backing_inode;
 }
 
 static int fuse_inode_set(struct inode *inode, void *_nodeidp)
 {
-	u64 nodeid = *(u64 *) _nodeidp;
-	get_fuse_inode(inode)->nodeid = nodeid;
+	struct fuse_inode_identifier *fii =
+		(struct fuse_inode_identifier *) _nodeidp;
+	struct fuse_inode *fi = get_fuse_inode(inode);
+
+	fi->nodeid = fii->nodeid;
+
+	return 0;
+}
+
+static int fuse_inode_backing_dir_set(struct inode *inode, void *_nodeidp)
+{
+	struct fuse_inode_identifier *fii =
+		(struct fuse_inode_identifier *) _nodeidp;
+	struct fuse_inode *fi = get_fuse_inode(inode);
+
+	fi->nodeid = fii->nodeid;
+#ifdef CONFIG_FUSE_PASSTHROUGH_DIR
+	fi->backing_inode = fii->backing_inode;
+	if (fi->backing_inode)
+		ihold(fi->backing_inode);
+#endif
+
 	return 0;
 }
 
+struct inode *fuse_iget_backing(struct super_block *sb,
+				struct inode *backing_inode)
+{
+	struct inode *inode;
+	struct fuse_inode *fi;
+	struct fuse_conn *fc = get_fuse_conn_super(sb);
+	struct fuse_inode_identifier fii = {
+		.nodeid = 0,
+		.backing_inode = backing_inode,
+	};
+	struct fuse_attr attr;
+	unsigned long hash = (unsigned long) backing_inode;
+
+	fuse_fill_attr_from_inode(&attr, get_fuse_inode(backing_inode));
+	inode = iget5_locked(sb, hash, fuse_inode_backing_eq,
+			     fuse_inode_backing_dir_set, &fii);
+	if (!inode)
+		return NULL;
+
+	if ((inode->i_state & I_NEW)) {
+		inode->i_flags |= S_NOATIME;
+		if (!fc->writeback_cache)
+			inode->i_flags |= S_NOCMTIME;
+		fuse_init_common(inode);
+		unlock_new_inode(inode);
+	}
+
+	fi = get_fuse_inode(inode);
+	fuse_init_inode(inode, &attr, fc);
+	spin_lock(&fi->lock);
+	fi->nlookup++;
+	spin_unlock(&fi->lock);
+
+	return inode;
+}
+
 struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
 			int generation, struct fuse_attr *attr,
 			u64 attr_valid, u64 attr_version,
@@ -467,6 +550,9 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
 	struct inode *inode;
 	struct fuse_inode *fi;
 	struct fuse_conn *fc = get_fuse_conn_super(sb);
+	struct fuse_inode_identifier fii = {
+		.nodeid = nodeid,
+	};
 
 	/*
 	 * Auto mount points get their node id from the submount root, which is
@@ -498,7 +584,7 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
 	}
 
 retry:
-	inode = iget5_locked(sb, nodeid, fuse_inode_eq, fuse_inode_set, &nodeid);
+	inode = iget5_locked(sb, nodeid, fuse_inode_eq, fuse_inode_set, &fii);
 	if (!inode)
 		return NULL;
 
@@ -533,13 +619,16 @@ struct inode *fuse_ilookup(struct fuse_conn *fc, u64 nodeid,
 {
 	struct fuse_mount *fm_iter;
 	struct inode *inode;
+	struct fuse_inode_identifier fii = {
+		.nodeid = nodeid,
+	};
 
 	WARN_ON(!rwsem_is_locked(&fc->killsb));
 	list_for_each_entry(fm_iter, &fc->mounts, fc_entry) {
 		if (!fm_iter->sb)
 			continue;
 
-		inode = ilookup5(fm_iter->sb, nodeid, fuse_inode_eq, &nodeid);
+		inode = ilookup5(fm_iter->sb, nodeid, fuse_inode_eq, &fii);
 		if (inode) {
 			if (fm)
 				*fm = fm_iter;
@@ -1072,7 +1161,7 @@ static struct dentry *fuse_get_dentry(struct super_block *sb,
 			goto out_err;
 
 		err = fuse_lookup_name(sb, handle->nodeid, &name, &outarg,
-				       &inode);
+				       NULL, &inode);
 		if (err && err != -ENOENT)
 			goto out_err;
 		if (err || !inode) {
@@ -1173,7 +1262,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, &outarg, &inode);
+			       &dotdot_name, &outarg, NULL, &inode);
 	if (err) {
 		if (err == -ENOENT)
 			return ERR_PTR(-ESTALE);
@@ -1201,6 +1290,9 @@ static const struct export_operations fuse_export_operations = {
 
 static const struct super_operations fuse_super_operations = {
 	.alloc_inode    = fuse_alloc_inode,
+#ifdef CONFIG_FUSE_PASSTHROUGH_DIR
+	.destroy_inode  = fuse_destroy_inode,
+#endif
 	.free_inode     = fuse_free_inode,
 	.evict_inode	= fuse_evict_inode,
 	.write_inode	= fuse_write_inode,
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 5ec43ecbceb7..f1bd8e7734ec 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -690,6 +690,10 @@ struct fuse_entry_out {
 	struct fuse_attr attr;
 };
 
+struct fuse_entry_backing_out {
+	uint64_t	backing_fd;
+};
+
 struct fuse_forget_in {
 	uint64_t	nlookup;
 };
-- 
2.49.0.1112.g889b7c5bd8-goog


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v1 2/2] fuse: open/close backing file
  2025-06-17 22:14 [PATCH v1 0/2] RFC: Extend fuse-passthrough to directories Paul Lawrence
  2025-06-17 22:14 ` [PATCH v1 1/2] fuse: Add backing file option to lookup Paul Lawrence
@ 2025-06-17 22:14 ` Paul Lawrence
  2025-06-18 11:01 ` [PATCH v1 0/2] RFC: Extend fuse-passthrough to directories Amir Goldstein
  2 siblings, 0 replies; 19+ messages in thread
From: Paul Lawrence @ 2025-06-17 22:14 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-kernel, linux-fsdevel, Paul Lawrence

Add support for opening and closing the backing file on a passthrough
directory.

* Add backing file to fuse file
* Add fuse_inode_has_backing - this is how we will detect whether to use
  traditional fuse operations or the added backing file operations
* Add backing operations to open, flush and close

Signed-off-by: Paul Lawrence <paullawrence@google.com>
---
 fs/fuse/backing.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++
 fs/fuse/dir.c     | 16 -----------
 fs/fuse/file.c    | 34 ++++++++++++++++--------
 fs/fuse/fuse_i.h  | 39 +++++++++++++++++++++++++++
 4 files changed, 130 insertions(+), 27 deletions(-)

diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
index 1dcc617bf660..04265bd06695 100644
--- a/fs/fuse/backing.c
+++ b/fs/fuse/backing.c
@@ -6,6 +6,64 @@
 
 #include "fuse_i.h"
 
+#include <linux/backing-file.h>
+
+int fuse_open_backing(struct inode *inode, struct file *file, bool isdir)
+{
+	struct fuse_mount *fm = get_fuse_mount(inode);
+	struct fuse_file *ff;
+	int retval;
+	int mask;
+	union fuse_dentry *fd = get_fuse_dentry(file->f_path.dentry);
+	struct file *backing_file;
+	uint32_t flags = file->f_flags & ~(O_CREAT | O_EXCL | O_NOCTTY);
+
+	ff = fuse_file_alloc(fm, true);
+	if (!ff)
+		return -ENOMEM;
+
+	switch (flags & O_ACCMODE) {
+	case O_RDONLY:
+		mask = MAY_READ;
+		break;
+
+	case O_WRONLY:
+		mask = MAY_WRITE;
+		break;
+
+	case O_RDWR:
+		mask = MAY_READ | MAY_WRITE;
+		break;
+
+	default:
+		retval = -EINVAL;
+		goto outerr;
+	}
+
+	retval = inode_permission(&nop_mnt_idmap,
+				  get_fuse_inode(inode)->backing_inode, mask);
+	if (retval)
+		goto outerr;
+
+	backing_file = backing_file_open(&file->f_path, file->f_flags,
+		&fd->backing_path, current_cred());
+
+	if (IS_ERR(backing_file)) {
+		retval = PTR_ERR(backing_file);
+		goto outerr;
+	}
+
+	ff->backing_file = backing_file;
+	ff->nodeid = get_fuse_inode(inode)->nodeid;
+	file->private_data = ff;
+	return 0;
+
+outerr:
+	if (retval)
+		fuse_file_free(ff);
+	return retval;
+}
+
 int fuse_handle_backing(struct fuse_entry_backing *feb,
 	struct inode **backing_inode, struct path *backing_path)
 {
@@ -27,3 +85,13 @@ int fuse_handle_backing(struct fuse_entry_backing *feb,
 
 	return 0;
 }
+
+int fuse_flush_backing(struct file *file, fl_owner_t id)
+{
+	struct fuse_file *fuse_file = file->private_data;
+	struct file *backing_file = fuse_file->backing_file;
+
+	if (backing_file->f_op->flush)
+		return backing_file->f_op->flush(backing_file, id);
+	return 0;
+}
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 909463fae94d..658898f324b5 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -44,18 +44,7 @@ static inline u64 fuse_dentry_time(const struct dentry *entry)
 {
 	return (u64)entry->d_fsdata;
 }
-
 #else
-union fuse_dentry {
-	struct {
-		u64 time;
-#ifdef CONFIG_FUSE_PASSTHROUGH_DIR
-		struct path backing_path;
-#endif
-	};
-	struct rcu_head rcu;
-};
-
 static inline void __fuse_dentry_settime(struct dentry *dentry, u64 time)
 {
 	((union fuse_dentry *) dentry->d_fsdata)->time = time;
@@ -65,11 +54,6 @@ static inline u64 fuse_dentry_time(const struct dentry *entry)
 {
 	return ((union fuse_dentry *) entry->d_fsdata)->time;
 }
-
-static inline union fuse_dentry *get_fuse_dentry(const struct dentry *entry)
-{
-	return entry->d_fsdata;
-}
 #endif
 
 static void fuse_dentry_settime(struct dentry *dentry, u64 time)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 754378dd9f71..0cd0a94073c7 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -21,6 +21,7 @@
 #include <linux/filelock.h>
 #include <linux/splice.h>
 #include <linux/task_io_accounting_ops.h>
+#include <linux/file.h>
 
 static int fuse_send_open(struct fuse_mount *fm, u64 nodeid,
 			  unsigned int open_flags, int opcode,
@@ -105,19 +106,24 @@ static void fuse_file_put(struct fuse_file *ff, bool sync)
 		struct fuse_release_args *ra = &ff->args->release_args;
 		struct fuse_args *args = (ra ? &ra->args : NULL);
 
-		if (ra && ra->inode)
-			fuse_file_io_release(ff, ra->inode);
-
-		if (!args) {
-			/* Do nothing when server does not implement 'open' */
-		} else if (sync) {
-			fuse_simple_request(ff->fm, args);
+		if (ff->backing_file) {
 			fuse_release_end(ff->fm, args, 0);
+			fput(ff->backing_file);
 		} else {
-			args->end = fuse_release_end;
-			if (fuse_simple_background(ff->fm, args,
-						   GFP_KERNEL | __GFP_NOFAIL))
-				fuse_release_end(ff->fm, args, -ENOTCONN);
+			if (ra && ra->inode)
+				fuse_file_io_release(ff, ra->inode);
+
+			if (!args) {
+				/* Do nothing when server does not implement 'open' */
+			} else if (sync) {
+				fuse_simple_request(ff->fm, args);
+				fuse_release_end(ff->fm, args, 0);
+			} else {
+				args->end = fuse_release_end;
+				if (fuse_simple_background(ff->fm, args,
+							GFP_KERNEL | __GFP_NOFAIL))
+					fuse_release_end(ff->fm, args, -ENOTCONN);
+			}
 		}
 		kfree(ff);
 	}
@@ -248,6 +254,9 @@ static int fuse_open(struct inode *inode, struct file *file)
 	if (err)
 		return err;
 
+	if (fuse_inode_has_backing(inode))
+		return fuse_open_backing(inode, file, false);
+
 	if (is_wb_truncate || dax_truncate)
 		inode_lock(inode);
 
@@ -522,6 +531,9 @@ static int fuse_flush(struct file *file, fl_owner_t id)
 	FUSE_ARGS(args);
 	int err;
 
+	if (fuse_inode_has_backing(file->f_inode))
+		return fuse_flush_backing(file, id);
+
 	if (fuse_is_bad(inode))
 		return -EIO;
 
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 1dc04bc6ac49..a27c05810bab 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -96,6 +96,25 @@ struct fuse_submount_lookup {
 	struct fuse_forget_link *forget;
 };
 
+
+/** FUSE specific dentry data */
+#if BITS_PER_LONG < 64 || defined(CONFIG_FUSE_PASSTHROUGH_DIR)
+union fuse_dentry {
+	struct {
+		u64 time;
+#ifdef CONFIG_FUSE_PASSTHROUGH_DIR
+		struct path backing_path;
+#endif
+	};
+	struct rcu_head rcu;
+};
+
+static inline union fuse_dentry *get_fuse_dentry(const struct dentry *entry)
+{
+	return entry->d_fsdata;
+}
+#endif
+
 /** Container for data related to mapping to backing file */
 struct fuse_backing {
 	struct file *file;
@@ -287,6 +306,14 @@ struct fuse_file {
 
 	} readdir;
 
+#ifdef CONFIG_FUSE_PASSTHROUGH_DIR
+	/**
+	 * TODO: Reconcile with passthrough file
+	 * backing file when in bpf mode
+	 */
+	struct file *backing_file;
+#endif
+
 	/** RB node to be linked on fuse_conn->polled_files */
 	struct rb_node polled_node;
 
@@ -1597,4 +1624,16 @@ extern void fuse_sysctl_unregister(void);
 /* backing.c */
 int fuse_handle_backing(struct fuse_entry_backing *feb,
 	struct inode **backing_inode, struct path *backing_path);
+
+
+static inline bool fuse_inode_has_backing(struct inode *inode)
+{
+	struct fuse_inode *fuse_inode = get_fuse_inode(inode);
+
+	return fuse_inode && fuse_inode->backing_inode;
+}
+
+int fuse_open_backing(struct inode *inode, struct file *file, bool isdir);
+int fuse_flush_backing(struct file *file, fl_owner_t id);
+
 #endif /* _FS_FUSE_I_H */
-- 
2.49.0.1112.g889b7c5bd8-goog


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 0/2] RFC: Extend fuse-passthrough to directories
  2025-06-17 22:14 [PATCH v1 0/2] RFC: Extend fuse-passthrough to directories Paul Lawrence
  2025-06-17 22:14 ` [PATCH v1 1/2] fuse: Add backing file option to lookup Paul Lawrence
  2025-06-17 22:14 ` [PATCH v1 2/2] fuse: open/close backing file Paul Lawrence
@ 2025-06-18 11:01 ` Amir Goldstein
  2025-06-19 19:50   ` Paul Lawrence
  2 siblings, 1 reply; 19+ messages in thread
From: Amir Goldstein @ 2025-06-18 11:01 UTC (permalink / raw)
  To: Paul Lawrence; +Cc: Miklos Szeredi, linux-kernel, linux-fsdevel, Bernd Schubert

Hi Paul,

I am very happy that you are getting back to this task
as I find myself never getting to it.

I would like to ask you to try to build on the code that was already
merged to the upstream kernel rather than trying to go back to
the out of tree version concepts.

I've added libfuse maintainer in CC, please remember to CC him
in future emails about proposed FUSE UAPI changes.

On Wed, Jun 18, 2025 at 12:15 AM Paul Lawrence <paullawrence@google.com> wrote:
>
> This is the first part of a much larger patch set that would allow a directory
> to be marked ‘passthrough’. At a high level, the fuse daemon can return an
> optional extra argument to FUSE_LOOKUP that contains an fd.

What's wrong with returning a backing_id?
Why "reinvent" the pre-release wheel?

> Extra fields are
> added to the fuse_dentry, fuse_inode and fuse_file structs to have a backing
> path, inode and file respectively. When fuse is performing an operation, it will
> check for the existence of a backing object and if it exists forward the
> operation to the backing object.

I read your patches and I have no idea why you needed to add fields to
fuse_dentry and why you needed to duplicate the backing file fields in
fuse_inode and fuse_file. Please explain yourself.

>
> These two patches add the core infrastructure, handling of the extra argument
> response to lookup, and forwarding open, flush and close to the backing file.
> This is sufficient to validate the concept.

What I am reading between the lines is that open/close are examples
of passthrough ops that are expected to be extended to more ops later.
Please see my WIP branch [1] for a suggestion to use an ops_mask API
for declaring which inode operations are passthrough.

[1] https://github.com/amir73il/linux/commits/fuse-backing-inode-wip/

>
> The questions I have:
>
> * Is this something that interests the fuse maintainers and community?

Definitely interested in inode ops passthrough.
I am willing to help with review and implementation if needed.

> * Is this approach the correct one?

Which approach?

Miklos and I have been discussing setting up a backing file on
FUSE_LOOKUP for a while.
The same concept was discussed also for returning an iomap
description on FUSE_LOOKUP request [2]

[2] https://lore.kernel.org/linux-fsdevel/20250529164503.GB8282@frogsfrogsfrogs/

So yes, the concept of setting up passthough on lookup is the correct one.

One thing that you need to be aware of is that FUSE_LOOKUP is only one
of several ways to instantiate a fuse dentry/inode.

The commands FUSE_CREATE, FUSE_TMPFILE and FUSE_READDIRPLUS
also instantiate dentries and need to be dealt with as well.

Therefore, I was looking at the direction of extending struct fuse_entry_out
rather than adding a new argument to FUSE_LOOKUP.

> * (if we agree yes to 1 & 2) Detailed analysis of the patches for errors.
>
> A few observations:
>
> I have matched backing objects to their fuse objects. Currently fuse passthrough
> puts a backing file into the fuse inode. I’m not quite sure why this was done -
> it seems to have been a very late change in the passthrough patch sets which
> happened without comment.

It was done to be able to have sane support for mmap passthrough among
other things and be able to have a sane story about inode attributes.

> It does not really make sense for full directory
> passthrough since unopened inodes still need to have backing inodes.

I do not understand this statement.
Why doesn't it make sense?
Why does it make sense to have a backing path per dentry?
Are you expecting to map different hardlink aliases to different backing inodes?
It might have helped if you had a design document explaining the reasoning
behind the implementation choices.

> The one
> advantage I can see is that it reduces the number of opens/closes of the backing
> file. However, this may also be a disadvantage - it moves closes, in particular,
> to an arbitrary point when the inode is flushed from cache.
>

Rather than when? when dentry is flushed from cache?
I do not follow.

> Backing operations need to happen in the context of the daemon, not the caller.

Correct.

> (I am a firm believer of this principle.) This is not yet implemented, and is
> not (currently, and unfortunately) the way Android uses passthough. It is not
> hard to do, and if these patches are otherwise acceptable, will be added.
>

Note that many of the passthrough method implementations were moved
to common "library" code for handling backing_file, which may or may not
be sharable with overlayfs.

I don't think that we need to have all passthough methods implemented in the
generic backing_file "library" and share code with overlayfs, but we
should consider
it for new methods to see if it makes sense.

> There was a long discussion about the security issues of using an fd returned
> from the fuse daemon in the context of fuse passthrough, and the end solution
> was to use an ioctl to set the backing file. I have used the previously-rejected
> approach of passing the fd in a struct in the fuse_daemon response. My defense
> of this approach is
>
> * The fd is simply used to pull out the path and inode
> * All operations are revalidated
> * Thus there is no risk even if a privileged process with a protected fd is
> tricked into passing that fd back in this structure.

Not convinced.
Are you saying that passing an O_PATH fd of /etc/passwd is ok?
when that O_PATH is used to do passthrough open?
I do not follow and I also do not see the problem with sticking with
the already established backing_id and ioctl solution.

I would add that if you make your solution dependent on io_uring
then the security concern goes away, but you did not give a strong
enough reason to have this limitation IMO.

>
> I’m sure we will discuss this at length if this patch set is otherwise deemed
> valuable, and I am certainly not wedded to this approach.
>
> I have written tests to validate this approach using tools/testing/selftests. I
> don’t want this patch set to get derailed by a discussion of the way I wrote the
> tests, so I have not included them. I am very open to any and every suggestion
> as to how (and where) tests should be written for these patches.
>

Apart from specific function tests for passthrough, FUSE_PASSTHROUGH
was tested with fstests, using the scripts included in libfuse to run the
passthrough_hp examples in fstests.

So a basic sanity test for your code is that it does not regress fstests that
are currently passing with passthrough_hp.

Please let me know if anything I wrote is not clear and if there is anything
else that I can help with.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 1/2] fuse: Add backing file option to lookup
  2025-06-17 22:14 ` [PATCH v1 1/2] fuse: Add backing file option to lookup Paul Lawrence
@ 2025-06-18 13:46   ` kernel test robot
  0 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2025-06-18 13:46 UTC (permalink / raw)
  To: Paul Lawrence, Miklos Szeredi
  Cc: oe-kbuild-all, linux-kernel, linux-fsdevel, Paul Lawrence

Hi Paul,

kernel test robot noticed the following build errors:

[auto build test ERROR on mszeredi-fuse/for-next]
[also build test ERROR on linus/master v6.16-rc2 next-20250618]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Paul-Lawrence/fuse-Add-backing-file-option-to-lookup/20250618-061717
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git for-next
patch link:    https://lore.kernel.org/r/20250617221456.888231-2-paullawrence%40google.com
patch subject: [PATCH v1 1/2] fuse: Add backing file option to lookup
config: i386-randconfig-001-20250618 (https://download.01.org/0day-ci/archive/20250618/202506182126.iGTKLLEc-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250618/202506182126.iGTKLLEc-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202506182126.iGTKLLEc-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   fs/fuse/inode.c: In function 'fuse_inode_backing_eq':
>> fs/fuse/inode.c:479:44: error: 'struct fuse_inode' has no member named 'backing_inode'
     479 |                 && fii->backing_inode == fi->backing_inode;
         |                                            ^~
>> fs/fuse/inode.c:480:1: warning: control reaches end of non-void function [-Wreturn-type]
     480 | }
         | ^


vim +479 fs/fuse/inode.c

   471	
   472	static int fuse_inode_backing_eq(struct inode *inode, void *_nodeidp)
   473	{
   474		struct fuse_inode_identifier *fii =
   475			(struct fuse_inode_identifier *) _nodeidp;
   476		struct fuse_inode *fi = get_fuse_inode(inode);
   477	
   478		return fii->nodeid == fi->nodeid
 > 479			&& fii->backing_inode == fi->backing_inode;
 > 480	}
   481	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 0/2] RFC: Extend fuse-passthrough to directories
  2025-06-18 11:01 ` [PATCH v1 0/2] RFC: Extend fuse-passthrough to directories Amir Goldstein
@ 2025-06-19 19:50   ` Paul Lawrence
  2025-06-19 20:29     ` Amir Goldstein
  0 siblings, 1 reply; 19+ messages in thread
From: Paul Lawrence @ 2025-06-19 19:50 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Miklos Szeredi, linux-kernel, linux-fsdevel, Bernd Schubert

Hi Amir,

Thank you for your detailed reply. My intent with this patch was to see if there
was interest (a definite yes) and to see what path would best get us
to our common
goal.

I'm thinking the best approach is to start with your ops_mask API. In
fact, that solves
the biggest single problem with my future patch set, which was that it
was going to be
huge and not realistically divisible, since you need everything for
directory passthrough
to work without the mask. Your way allows us to proceed in nice
logical steps, which is
much, much better. Thank you for that suggestion.

So my follow-up question is: What can I do to help get the
foundational patches you
wrote upstreamed?

In the meantime, a few thoughts on your comments. (Note that one of
the beauties of
your suggestion is that we don't need to agree on any of this to get
started - we can
discuss them in detail when we get to the specific ops that require them.)

1) Yes, let's use backing_id. I won't mention that again.

2) The backing path per dentry comes from the way dentry_open works.
If we are going to
attach a file to a lookup, we have to put something into the
fuse_dentry or the fuse_inode.
This makes more sense once you see points 3 & 4 below - without them,
we have an open
file, so why not just use it?

3) A cute idea that we had that seems to work is to allow negative
dentries as backing
dentries. It appears to work well - for instance, a create first looks
up the (negative) dentry
then creates the file into that dentry. If the lookup puts a negative
dentry as the backing
file, we can now just use vfs_create to create the backing file.

This means that only FUSE_LOOKUP and (I think) FUSE_READDIRPLUS need to have
the ability to accept backing_ids. I think is is both more elegant
conceptually, simpler to
code in the kernel *and* simpler to use in the daemon.

4) Having to open a file for it to be passed into a lookup is
problematic. Imagine
readdirplus on a large folder. We would need to open every single
backing file, and it
would stay open until the dentry was removed from the cache.

Both of these suggest that rather than just passing a backing_id to FUSE_LOOKUP
and FUSE_READDIRPLUS we should be able to pass a backing_id and a relative path.
This is where the idea of putting the backing path into the fuse
dentry comes from.

I don't *think* this creates any security issues, so long as the
relative path is traversed
in the context of the daemon. (We might want to ban '..' and traverses
over file systems.)
Again, these are details we can debate when the patches are ready for
discussion.

But again, let's start with your patch set. What are the next steps in
taking it upstream?
And which are the next ops you would like to see implemented? I would
be happy to take
a stab at one or two.

Paul

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 0/2] RFC: Extend fuse-passthrough to directories
  2025-06-19 19:50   ` Paul Lawrence
@ 2025-06-19 20:29     ` Amir Goldstein
  2025-07-14 15:20       ` Amir Goldstein
  0 siblings, 1 reply; 19+ messages in thread
From: Amir Goldstein @ 2025-06-19 20:29 UTC (permalink / raw)
  To: Paul Lawrence; +Cc: Miklos Szeredi, linux-kernel, linux-fsdevel, Bernd Schubert

On Thu, Jun 19, 2025 at 9:50 PM Paul Lawrence <paullawrence@google.com> wrote:
>
> Hi Amir,
>
> Thank you for your detailed reply. My intent with this patch was to see if there
> was interest (a definite yes) and to see what path would best get us
> to our common
> goal.
>
> I'm thinking the best approach is to start with your ops_mask API. In
> fact, that solves
> the biggest single problem with my future patch set, which was that it
> was going to be
> huge and not realistically divisible, since you need everything for
> directory passthrough
> to work without the mask. Your way allows us to proceed in nice
> logical steps, which is
> much, much better. Thank you for that suggestion.
>
> So my follow-up question is: What can I do to help get the
> foundational patches you
> wrote upstreamed?

Well you can always take them and re-shape them and post them
to see what the maintainers think and address the feedback.
But I can try to beat them to shape myself to at least post v1.

>
> In the meantime, a few thoughts on your comments. (Note that one of
> the beauties of
> your suggestion is that we don't need to agree on any of this to get
> started - we can
> discuss them in detail when we get to the specific ops that require them.)
>
> 1) Yes, let's use backing_id. I won't mention that again.
>
> 2) The backing path per dentry comes from the way dentry_open works.
> If we are going to
> attach a file to a lookup, we have to put something into the
> fuse_dentry or the fuse_inode.

There is already fuse_backing *fb in fuse_inode.
I don't understand why anything else is needed for implementing
passthrough dir ops.

> This makes more sense once you see points 3 & 4 below - without them,
> we have an open
> file, so why not just use it?

We need to make the code simple enough.
Not add things that are not needed.

>
> 3) A cute idea that we had that seems to work is to allow negative
> dentries as backing
> dentries. It appears to work well - for instance, a create first looks
> up the (negative) dentry
> then creates the file into that dentry. If the lookup puts a negative
> dentry as the backing
> file, we can now just use vfs_create to create the backing file.
>

That sounds like trouble.
Overalyfs effectively implements passthrough dir ops.
It doesn't keep negative backing dentries, so I doubt that this is needed.

> This means that only FUSE_LOOKUP and (I think) FUSE_READDIRPLUS need to have
> the ability to accept backing_ids. I think is is both more elegant
> conceptually, simpler to
> code in the kernel *and* simpler to use in the daemon.
>
> 4) Having to open a file for it to be passed into a lookup is
> problematic. Imagine
> readdirplus on a large folder. We would need to open every single
> backing file, and it
> would stay open until the dentry was removed from the cache.

We are talking about opening a O_PATH fd at lookup.
The daemon does not need to keep this O_PATH fd open,
although production daemons today (e.g. virtiofsd) anyway
keep an open O_PATH fd per fuse inode in cache.

Maybe it is a problem, but I am not convinced that it is, so
maybe I need more details about what problems this is causing.
If you are going to pin the backing inode/dentry to cache, then most
of the memory resources are already taken, the extra file does not add
much memory and it is currently not accounted for in any process.

>
> Both of these suggest that rather than just passing a backing_id to FUSE_LOOKUP
> and FUSE_READDIRPLUS we should be able to pass a backing_id and a relative path.
> This is where the idea of putting the backing path into the fuse
> dentry comes from.
>

Sorry this is too much hand waving.
I still don't understand what problem attaching a backing path to every dentry
solves. You will have to walk me through exactly what the problem is with
having the backing file/path attached to the inode.

> I don't *think* this creates any security issues, so long as the
> relative path is traversed
> in the context of the daemon. (We might want to ban '..' and traverses
> over file systems.)

Sorry you lost me.
I do not understand the idea of backing_id and a relative path.
passthrough of READDIRPLUS is complicated.
If you have an idea I need to see a very detailed plan.

> Again, these are details we can debate when the patches are ready for
> discussion.
>
> But again, let's start with your patch set. What are the next steps in
> taking it upstream?
> And which are the next ops you would like to see implemented? I would
> be happy to take
> a stab at one or two.
>

I can post patches for passthrough getxattr/listxattr, those are pretty
simple, but I am not sure if they have merit on their own without
passthrough of getattr, which is more complicated.

Also I am not sure that implementing passthrough of some inode ops
has merit without being able to setup passthrough at lookup time.

I will see if I can find time to post a POC of basic passthrough of inode
ops and setup of backing id at lookup time.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 0/2] RFC: Extend fuse-passthrough to directories
  2025-06-19 20:29     ` Amir Goldstein
@ 2025-07-14 15:20       ` Amir Goldstein
  2025-07-22 21:13         ` Paul Lawrence
  0 siblings, 1 reply; 19+ messages in thread
From: Amir Goldstein @ 2025-07-14 15:20 UTC (permalink / raw)
  To: Paul Lawrence; +Cc: Miklos Szeredi, linux-kernel, linux-fsdevel, Bernd Schubert

On Thu, Jun 19, 2025 at 10:29 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Thu, Jun 19, 2025 at 9:50 PM Paul Lawrence <paullawrence@google.com> wrote:
> >
> > Hi Amir,
> >
> > Thank you for your detailed reply. My intent with this patch was to see if there
> > was interest (a definite yes) and to see what path would best get us
> > to our common
> > goal.
> >
> > I'm thinking the best approach is to start with your ops_mask API. In
> > fact, that solves
> > the biggest single problem with my future patch set, which was that it
> > was going to be
> > huge and not realistically divisible, since you need everything for
> > directory passthrough
> > to work without the mask. Your way allows us to proceed in nice
> > logical steps, which is
> > much, much better. Thank you for that suggestion.
> >
> > So my follow-up question is: What can I do to help get the
> > foundational patches you
> > wrote upstreamed?
>
> Well you can always take them and re-shape them and post them
> to see what the maintainers think and address the feedback.
> But I can try to beat them to shape myself to at least post v1.
>
> >
> > In the meantime, a few thoughts on your comments. (Note that one of
> > the beauties of
> > your suggestion is that we don't need to agree on any of this to get
> > started - we can
> > discuss them in detail when we get to the specific ops that require them.)
> >
> > 1) Yes, let's use backing_id. I won't mention that again.
> >
> > 2) The backing path per dentry comes from the way dentry_open works.
> > If we are going to
> > attach a file to a lookup, we have to put something into the
> > fuse_dentry or the fuse_inode.
>
> There is already fuse_backing *fb in fuse_inode.
> I don't understand why anything else is needed for implementing
> passthrough dir ops.
>
> > This makes more sense once you see points 3 & 4 below - without them,
> > we have an open
> > file, so why not just use it?
>
> We need to make the code simple enough.
> Not add things that are not needed.
>
> >
> > 3) A cute idea that we had that seems to work is to allow negative
> > dentries as backing
> > dentries. It appears to work well - for instance, a create first looks
> > up the (negative) dentry
> > then creates the file into that dentry. If the lookup puts a negative
> > dentry as the backing
> > file, we can now just use vfs_create to create the backing file.
> >
>
> That sounds like trouble.
> Overalyfs effectively implements passthrough dir ops.
> It doesn't keep negative backing dentries, so I doubt that this is needed.
>
> > This means that only FUSE_LOOKUP and (I think) FUSE_READDIRPLUS need to have
> > the ability to accept backing_ids. I think is is both more elegant
> > conceptually, simpler to
> > code in the kernel *and* simpler to use in the daemon.
> >
> > 4) Having to open a file for it to be passed into a lookup is
> > problematic. Imagine
> > readdirplus on a large folder. We would need to open every single
> > backing file, and it
> > would stay open until the dentry was removed from the cache.
>
> We are talking about opening a O_PATH fd at lookup.
> The daemon does not need to keep this O_PATH fd open,
> although production daemons today (e.g. virtiofsd) anyway
> keep an open O_PATH fd per fuse inode in cache.
>
> Maybe it is a problem, but I am not convinced that it is, so
> maybe I need more details about what problems this is causing.
> If you are going to pin the backing inode/dentry to cache, then most
> of the memory resources are already taken, the extra file does not add
> much memory and it is currently not accounted for in any process.
>
> >
> > Both of these suggest that rather than just passing a backing_id to FUSE_LOOKUP
> > and FUSE_READDIRPLUS we should be able to pass a backing_id and a relative path.
> > This is where the idea of putting the backing path into the fuse
> > dentry comes from.
> >
>
> Sorry this is too much hand waving.
> I still don't understand what problem attaching a backing path to every dentry
> solves. You will have to walk me through exactly what the problem is with
> having the backing file/path attached to the inode.
>
> > I don't *think* this creates any security issues, so long as the
> > relative path is traversed
> > in the context of the daemon. (We might want to ban '..' and traverses
> > over file systems.)
>
> Sorry you lost me.
> I do not understand the idea of backing_id and a relative path.
> passthrough of READDIRPLUS is complicated.
> If you have an idea I need to see a very detailed plan.
>
> > Again, these are details we can debate when the patches are ready for
> > discussion.
> >
> > But again, let's start with your patch set. What are the next steps in
> > taking it upstream?
> > And which are the next ops you would like to see implemented? I would
> > be happy to take
> > a stab at one or two.
> >
>
> I can post patches for passthrough getxattr/listxattr, those are pretty
> simple,

Spoke up too soon.
passthrough of getxattr is only allegedly simple - that's before realizing
the complexity of passthrough of get_acl (see ovl_get_acl()).

> but I am not sure if they have merit on their own without
> passthrough of getattr, which is more complicated.
>
> Also I am not sure that implementing passthrough of some inode ops
> has merit without being able to setup passthrough at lookup time.
>
> I will see if I can find time to post a POC of basic passthrough of inode
> ops and setup of backing id at lookup time.
>

I did not get far enough to post a POC, but I did get far enough
to test inode ops passthrough.

This branch [1] has patches that implement passthrough for readdir
and getattr/statx.

I mostly wanted to demonstrate that you do not need to invent any new
infra to support passthrough of inode/dir ops - all the infra is already
there just waiting for a richer API to setup inode ops passthrough
at lookup time and implement the different passthrough methods.
I hope that this is clear?

I ran into an ABI compatibility issue with extending the fuse_entry_out
lookup result, which is also being used in struct fuse_direntplus,
so for the demo, I worked around this problem by setting up the
iops passthrough on first GETATTR.
The ABI compatibility (with existing binaries) is solvable but requires
more work.

In this demo passthrough_fs [2], when run with arguments
--readdirpassthrough --nocache
the first GETATTR on every dir/file sets up a permanent
backing inode for the fuse inode and all the following GETATTR
calls are passed through on this inode (until drop_caches).

Readdir is also passed through, but because passthrough
of LOOKUP is not implemented, it is not a clear win to use
readdir/getattr passthrough in comparison to readdirplus.
Hence, there is no justification to post these patches on their own.
But you can use them as a basis for your work.

If you have any questions, don't hesitate to ask me.

Thanks,
Amir.

[1] https://github.com/amir73il/linux/commits/fuse_passthrough_iops/
[2] https://github.com/amir73il/libfuse/commits/fuse_passthrough_iops/

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 0/2] RFC: Extend fuse-passthrough to directories
  2025-07-14 15:20       ` Amir Goldstein
@ 2025-07-22 21:13         ` Paul Lawrence
  2025-07-23 16:53           ` Amir Goldstein
  0 siblings, 1 reply; 19+ messages in thread
From: Paul Lawrence @ 2025-07-22 21:13 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Miklos Szeredi, linux-kernel, linux-fsdevel, Bernd Schubert

I spent a little time with these patches. I wrote my own to set the
backing file at lookup time by adding an optional second out struct.
Is there a reason we should not do this? It seems the most natural
solution.

After a while, I began to wonder if what I was planning was actually
the same as your vision. I decided to jot down my thoughts to see if
you agree with them. Also I was confused as to why you were adding the
ability to set backing files to GETATTR. So here are my notes.

Design of fuse passthrough for all operations

Goal:

When a fuse filesystem implements a stacking filesystem over an
underlying file system, and a significant fraction of the calls will
simply be passed to the underlying file system, provide a mechanism to
pass those calls through without going to user space. This is
primarily to enhance performance, though it might simplify the daemon
somewhat too.

Current state:

Currently passthrough allows a backing file to be set at file open
time. If a backing file is set, all read/write operations will be
forwarded to the backing file.

Design:

Add ability to set backing file on the found inode in response to a
positive lookup. This file might be opened with O_PATH for performance
reasons. The lookup api would be modified to have an optional second
out struct that contains the backing file id. Note that we will use
the backing file ioctl to create this id to remove any security
concerns.

The ioctl will also take a 64-bit integer to define which operations
will be passed through, the operations mask. This will have one bit
for each of the existing FUSE_OPERATIONS, at least the ones that act
on inodes/files.

Then when fuse fs is considering calling out to the daemon with an
opcode, fuse fs will check if the inode has a backing file and mask.
If it does, and the specific opcode bit is set, fuse fs will instead
call out to a kernel function in backing.c that can perform that
operation on the backing file correctly.

Details:

Question: Will it be possible to change the mask/backing file after
the initial setting? My feeling is that it might well be useful to be
able to set the mask, the file not so much. Situations would be (for
example) a caching file system that turns off read forwarding once the
whole file is downloaded.

FUSE_OPEN will, if the backing file has been set, reopen it as a file
(not a path) if needed. This is whether or not FUSE_OPEN is passed
through.

If FUSE_OPEN is passed through, user space will not get the chance to
assign a file handle ID to the opened file. It will still be possible
to pass FUSE_READ etc to the daemon - the daemon will still have the
node id and be able to read data based on that.

FUSE_LOOKUP can return a 0 node id, but only if *all* operations on
that node as marked as passthrough.

Suggestion: During FUSE_LOOKUP, if FUSE_GETATTR is set in the mask,
ignore the passed in attributes and read them from the backing file.

Random, non-exhastive list of considerations:

FUSE_FORGET can only be marked passthrough if the node id is 0.

FUSE_CREATE returns a new node id and file handle. It would need the
ability to set backing files.

If FUSE_LOOKUP is marked for passthrough, then looked up inodes will
be prepopulated with a backing O_PATH file and a mask will all bits
set.

FUSE_RENAME takes two nodes and names, and moves one to the other. If
one is passthrough and one is not, there is no obvious way of
performing a rename. Either fall back to copy/delete or return EXDEV

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 0/2] RFC: Extend fuse-passthrough to directories
  2025-07-22 21:13         ` Paul Lawrence
@ 2025-07-23 16:53           ` Amir Goldstein
  2025-07-23 18:30             ` Paul Lawrence
  0 siblings, 1 reply; 19+ messages in thread
From: Amir Goldstein @ 2025-07-23 16:53 UTC (permalink / raw)
  To: Paul Lawrence; +Cc: Miklos Szeredi, linux-kernel, linux-fsdevel, Bernd Schubert

On Tue, Jul 22, 2025 at 11:13 PM Paul Lawrence <paullawrence@google.com> wrote:
>
> I spent a little time with these patches. I wrote my own to set the
> backing file at lookup time by adding an optional second out struct.

FUSE_CREATE already returns two out args.
How were you planning to extend it?

> Is there a reason we should not do this? It seems the most natural
> solution.
>
> After a while, I began to wonder if what I was planning was actually
> the same as your vision. I decided to jot down my thoughts to see if
> you agree with them. Also I was confused as to why you were adding the
> ability to set backing files to GETATTR.

It is a demo. It demonstrates that passthrough can be set up sooner than
open time. It was not my intention to keep it this way.

> So here are my notes.
>
> Design of fuse passthrough for all operations
>
> Goal:
>
> When a fuse filesystem implements a stacking filesystem over an
> underlying file system, and a significant fraction of the calls will
> simply be passed to the underlying file system, provide a mechanism to
> pass those calls through without going to user space. This is
> primarily to enhance performance, though it might simplify the daemon
> somewhat too.
>
> Current state:
>
> Currently passthrough allows a backing file to be set at file open
> time. If a backing file is set, all read/write operations will be
> forwarded to the backing file.
>
> Design:
>
> Add ability to set backing file on the found inode in response to a
> positive lookup. This file might be opened with O_PATH for performance
> reasons. The lookup api would be modified to have an optional second
> out struct that contains the backing file id. Note that we will use
> the backing file ioctl to create this id to remove any security
> concerns.
>
> The ioctl will also take a 64-bit integer to define which operations
> will be passed through, the operations mask. This will have one bit
> for each of the existing FUSE_OPERATIONS, at least the ones that act
> on inodes/files.
>
> Then when fuse fs is considering calling out to the daemon with an
> opcode, fuse fs will check if the inode has a backing file and mask.
> If it does, and the specific opcode bit is set, fuse fs will instead
> call out to a kernel function in backing.c that can perform that
> operation on the backing file correctly.
>
> Details:
>
> Question: Will it be possible to change the mask/backing file after
> the initial setting? My feeling is that it might well be useful to be
> able to set the mask, the file not so much. Situations would be (for
> example) a caching file system that turns off read forwarding once the
> whole file is downloaded.
>

Generally, enabling passthrough from a point in time until the end of
inode lifetime is easier than turning it off, but also there are many
dependencies between passthrough ops and inode state so it will
require a lot of care to enable *some* operations mid inode lifetime.

> FUSE_OPEN will, if the backing file has been set, reopen it as a file
> (not a path) if needed. This is whether or not FUSE_OPEN is passed
> through.
>
> If FUSE_OPEN is passed through, user space will not get the chance to
> assign a file handle ID to the opened file. It will still be possible
> to pass FUSE_READ etc to the daemon - the daemon will still have the
> node id and be able to read data based on that.
>

Not sure I understand what you mean, but we do support per file opt-out of
passthrough with FOPEN_DIRECT_IO even when the inode is already
set up for passthrough.

> FUSE_LOOKUP can return a 0 node id, but only if *all* operations on
> that node as marked as passthrough.

Not sure why this is but I will be open to understanding.
Will need an elaborate design of the inode lifetime in this case.

>
> Suggestion: During FUSE_LOOKUP, if FUSE_GETATTR is set in the mask,
> ignore the passed in attributes and read them from the backing file.

My patches already implement that when GETATTR is in the mask.

>
> Random, non-exhastive list of considerations:
>
> FUSE_FORGET can only be marked passthrough if the node id is 0.
>
> FUSE_CREATE returns a new node id and file handle. It would need the
> ability to set backing files.
>
> If FUSE_LOOKUP is marked for passthrough, then looked up inodes will
> be prepopulated with a backing O_PATH file and a mask will all bits
> set.
>
> FUSE_RENAME takes two nodes and names, and moves one to the other. If
> one is passthrough and one is not, there is no obvious way of
> performing a rename. Either fall back to copy/delete or return EXDEV

Good question.

My patches were meant to provide you the basic infrastructure to enter
this playground and show that you do not need backing dentry/inode
beyond what is already available.

I hope this is enough for you to start experimenting and sending RFC patches
with design doc!

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 0/2] RFC: Extend fuse-passthrough to directories
  2025-07-23 16:53           ` Amir Goldstein
@ 2025-07-23 18:30             ` Paul Lawrence
  2025-07-24 19:51               ` Amir Goldstein
  0 siblings, 1 reply; 19+ messages in thread
From: Paul Lawrence @ 2025-07-23 18:30 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Miklos Szeredi, linux-kernel, linux-fsdevel, Bernd Schubert

> FUSE_CREATE already returns two out args.
> How were you planning to extend it?

Excellent point. However, FUSE_CREATE (and FUSE_TMPFILE) already has
fuse_open_out as the second argument, so the backing_id can be passed
in there. Does that sound acceptable? I'll admit that at first this
seemed clunky to me, but the more I think about it, the more natural
it seems - create is really an open, so it follows that paradigm, and
the new paradigm is used only for FUSE_LOOKUP.

FUSE_READDIRPLUS is harder, it seems to me. We would need to break ABI
compatibility - I can't see any sane alternative. A clean way would be
to have a FUSE_READDIR_PASSTHROUGH opcode. This implies that there's a
flag FUSE_PASSTHROUGH_MASK covering all of this new functionality
which would also enable this new opcode. (It could, alternatively,
change the behavior of FUSE_READDIRPLUS but I suspect no one is going
to like that idea.)

Does this seem reasonable?

Thanks,
Paul




On Wed, Jul 23, 2025 at 9:53 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Tue, Jul 22, 2025 at 11:13 PM Paul Lawrence <paullawrence@google.com> wrote:
> >
> > I spent a little time with these patches. I wrote my own to set the
> > backing file at lookup time by adding an optional second out struct.
>
> FUSE_CREATE already returns two out args.
> How were you planning to extend it?
>
> > Is there a reason we should not do this? It seems the most natural
> > solution.
> >
> > After a while, I began to wonder if what I was planning was actually
> > the same as your vision. I decided to jot down my thoughts to see if
> > you agree with them. Also I was confused as to why you were adding the
> > ability to set backing files to GETATTR.
>
> It is a demo. It demonstrates that passthrough can be set up sooner than
> open time. It was not my intention to keep it this way.
>
> > So here are my notes.
> >
> > Design of fuse passthrough for all operations
> >
> > Goal:
> >
> > When a fuse filesystem implements a stacking filesystem over an
> > underlying file system, and a significant fraction of the calls will
> > simply be passed to the underlying file system, provide a mechanism to
> > pass those calls through without going to user space. This is
> > primarily to enhance performance, though it might simplify the daemon
> > somewhat too.
> >
> > Current state:
> >
> > Currently passthrough allows a backing file to be set at file open
> > time. If a backing file is set, all read/write operations will be
> > forwarded to the backing file.
> >
> > Design:
> >
> > Add ability to set backing file on the found inode in response to a
> > positive lookup. This file might be opened with O_PATH for performance
> > reasons. The lookup api would be modified to have an optional second
> > out struct that contains the backing file id. Note that we will use
> > the backing file ioctl to create this id to remove any security
> > concerns.
> >
> > The ioctl will also take a 64-bit integer to define which operations
> > will be passed through, the operations mask. This will have one bit
> > for each of the existing FUSE_OPERATIONS, at least the ones that act
> > on inodes/files.
> >
> > Then when fuse fs is considering calling out to the daemon with an
> > opcode, fuse fs will check if the inode has a backing file and mask.
> > If it does, and the specific opcode bit is set, fuse fs will instead
> > call out to a kernel function in backing.c that can perform that
> > operation on the backing file correctly.
> >
> > Details:
> >
> > Question: Will it be possible to change the mask/backing file after
> > the initial setting? My feeling is that it might well be useful to be
> > able to set the mask, the file not so much. Situations would be (for
> > example) a caching file system that turns off read forwarding once the
> > whole file is downloaded.
> >
>
> Generally, enabling passthrough from a point in time until the end of
> inode lifetime is easier than turning it off, but also there are many
> dependencies between passthrough ops and inode state so it will
> require a lot of care to enable *some* operations mid inode lifetime.
>
> > FUSE_OPEN will, if the backing file has been set, reopen it as a file
> > (not a path) if needed. This is whether or not FUSE_OPEN is passed
> > through.
> >
> > If FUSE_OPEN is passed through, user space will not get the chance to
> > assign a file handle ID to the opened file. It will still be possible
> > to pass FUSE_READ etc to the daemon - the daemon will still have the
> > node id and be able to read data based on that.
> >
>
> Not sure I understand what you mean, but we do support per file opt-out of
> passthrough with FOPEN_DIRECT_IO even when the inode is already
> set up for passthrough.
>
> > FUSE_LOOKUP can return a 0 node id, but only if *all* operations on
> > that node as marked as passthrough.
>
> Not sure why this is but I will be open to understanding.
> Will need an elaborate design of the inode lifetime in this case.
>
> >
> > Suggestion: During FUSE_LOOKUP, if FUSE_GETATTR is set in the mask,
> > ignore the passed in attributes and read them from the backing file.
>
> My patches already implement that when GETATTR is in the mask.
>
> >
> > Random, non-exhastive list of considerations:
> >
> > FUSE_FORGET can only be marked passthrough if the node id is 0.
> >
> > FUSE_CREATE returns a new node id and file handle. It would need the
> > ability to set backing files.
> >
> > If FUSE_LOOKUP is marked for passthrough, then looked up inodes will
> > be prepopulated with a backing O_PATH file and a mask will all bits
> > set.
> >
> > FUSE_RENAME takes two nodes and names, and moves one to the other. If
> > one is passthrough and one is not, there is no obvious way of
> > performing a rename. Either fall back to copy/delete or return EXDEV
>
> Good question.
>
> My patches were meant to provide you the basic infrastructure to enter
> this playground and show that you do not need backing dentry/inode
> beyond what is already available.
>
> I hope this is enough for you to start experimenting and sending RFC patches
> with design doc!
>
> Thanks,
> Amir.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v1 0/2] RFC: Extend fuse-passthrough to directories
  2025-07-23 18:30             ` Paul Lawrence
@ 2025-07-24 19:51               ` Amir Goldstein
  2025-08-04 17:32                 ` [PATCH 0/2] RFC: Set backing file at lookup Paul Lawrence
  0 siblings, 1 reply; 19+ messages in thread
From: Amir Goldstein @ 2025-07-24 19:51 UTC (permalink / raw)
  To: Paul Lawrence; +Cc: Miklos Szeredi, linux-kernel, linux-fsdevel, Bernd Schubert

On Wed, Jul 23, 2025 at 8:30 PM Paul Lawrence <paullawrence@google.com> wrote:
>
> > FUSE_CREATE already returns two out args.
> > How were you planning to extend it?
>
> Excellent point. However, FUSE_CREATE (and FUSE_TMPFILE) already has
> fuse_open_out as the second argument, so the backing_id can be passed
> in there. Does that sound acceptable? I'll admit that at first this
> seemed clunky to me, but the more I think about it, the more natural
> it seems - create is really an open, so it follows that paradigm, and
> the new paradigm is used only for FUSE_LOOKUP.
>

Yes, you are right. FUSE_CREATE already has backing_id anyway.

> FUSE_READDIRPLUS is harder, it seems to me. We would need to break ABI
> compatibility - I can't see any sane alternative. A clean way would be
> to have a FUSE_READDIR_PASSTHROUGH opcode. This implies that there's a
> flag FUSE_PASSTHROUGH_MASK covering all of this new functionality
> which would also enable this new opcode. (It could, alternatively,
> change the behavior of FUSE_READDIRPLUS but I suspect no one is going
> to like that idea.)

Let's leave readdirplus for the very end.
It's not very important for the first version IMO.

>
> Does this seem reasonable?

Yes. sounds good to me.

Thanks,
Amir.

>
> Thanks,
> Paul
>
>
>
>
> On Wed, Jul 23, 2025 at 9:53 AM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > On Tue, Jul 22, 2025 at 11:13 PM Paul Lawrence <paullawrence@google.com> wrote:
> > >
> > > I spent a little time with these patches. I wrote my own to set the
> > > backing file at lookup time by adding an optional second out struct.
> >
> > FUSE_CREATE already returns two out args.
> > How were you planning to extend it?
> >
> > > Is there a reason we should not do this? It seems the most natural
> > > solution.
> > >
> > > After a while, I began to wonder if what I was planning was actually
> > > the same as your vision. I decided to jot down my thoughts to see if
> > > you agree with them. Also I was confused as to why you were adding the
> > > ability to set backing files to GETATTR.
> >
> > It is a demo. It demonstrates that passthrough can be set up sooner than
> > open time. It was not my intention to keep it this way.
> >
> > > So here are my notes.
> > >
> > > Design of fuse passthrough for all operations
> > >
> > > Goal:
> > >
> > > When a fuse filesystem implements a stacking filesystem over an
> > > underlying file system, and a significant fraction of the calls will
> > > simply be passed to the underlying file system, provide a mechanism to
> > > pass those calls through without going to user space. This is
> > > primarily to enhance performance, though it might simplify the daemon
> > > somewhat too.
> > >
> > > Current state:
> > >
> > > Currently passthrough allows a backing file to be set at file open
> > > time. If a backing file is set, all read/write operations will be
> > > forwarded to the backing file.
> > >
> > > Design:
> > >
> > > Add ability to set backing file on the found inode in response to a
> > > positive lookup. This file might be opened with O_PATH for performance
> > > reasons. The lookup api would be modified to have an optional second
> > > out struct that contains the backing file id. Note that we will use
> > > the backing file ioctl to create this id to remove any security
> > > concerns.
> > >
> > > The ioctl will also take a 64-bit integer to define which operations
> > > will be passed through, the operations mask. This will have one bit
> > > for each of the existing FUSE_OPERATIONS, at least the ones that act
> > > on inodes/files.
> > >
> > > Then when fuse fs is considering calling out to the daemon with an
> > > opcode, fuse fs will check if the inode has a backing file and mask.
> > > If it does, and the specific opcode bit is set, fuse fs will instead
> > > call out to a kernel function in backing.c that can perform that
> > > operation on the backing file correctly.
> > >
> > > Details:
> > >
> > > Question: Will it be possible to change the mask/backing file after
> > > the initial setting? My feeling is that it might well be useful to be
> > > able to set the mask, the file not so much. Situations would be (for
> > > example) a caching file system that turns off read forwarding once the
> > > whole file is downloaded.
> > >
> >
> > Generally, enabling passthrough from a point in time until the end of
> > inode lifetime is easier than turning it off, but also there are many
> > dependencies between passthrough ops and inode state so it will
> > require a lot of care to enable *some* operations mid inode lifetime.
> >
> > > FUSE_OPEN will, if the backing file has been set, reopen it as a file
> > > (not a path) if needed. This is whether or not FUSE_OPEN is passed
> > > through.
> > >
> > > If FUSE_OPEN is passed through, user space will not get the chance to
> > > assign a file handle ID to the opened file. It will still be possible
> > > to pass FUSE_READ etc to the daemon - the daemon will still have the
> > > node id and be able to read data based on that.
> > >
> >
> > Not sure I understand what you mean, but we do support per file opt-out of
> > passthrough with FOPEN_DIRECT_IO even when the inode is already
> > set up for passthrough.
> >
> > > FUSE_LOOKUP can return a 0 node id, but only if *all* operations on
> > > that node as marked as passthrough.
> >
> > Not sure why this is but I will be open to understanding.
> > Will need an elaborate design of the inode lifetime in this case.
> >
> > >
> > > Suggestion: During FUSE_LOOKUP, if FUSE_GETATTR is set in the mask,
> > > ignore the passed in attributes and read them from the backing file.
> >
> > My patches already implement that when GETATTR is in the mask.
> >
> > >
> > > Random, non-exhastive list of considerations:
> > >
> > > FUSE_FORGET can only be marked passthrough if the node id is 0.
> > >
> > > FUSE_CREATE returns a new node id and file handle. It would need the
> > > ability to set backing files.
> > >
> > > If FUSE_LOOKUP is marked for passthrough, then looked up inodes will
> > > be prepopulated with a backing O_PATH file and a mask will all bits
> > > set.
> > >
> > > FUSE_RENAME takes two nodes and names, and moves one to the other. If
> > > one is passthrough and one is not, there is no obvious way of
> > > performing a rename. Either fall back to copy/delete or return EXDEV
> >
> > Good question.
> >
> > My patches were meant to provide you the basic infrastructure to enter
> > this playground and show that you do not need backing dentry/inode
> > beyond what is already available.
> >
> > I hope this is enough for you to start experimenting and sending RFC patches
> > with design doc!
> >
> > Thanks,
> > Amir.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 0/2] RFC: Set backing file at lookup
  2025-07-24 19:51               ` Amir Goldstein
@ 2025-08-04 17:32                 ` Paul Lawrence
  2025-08-04 17:32                   ` [PATCH 1/2] fuse: Allow backing file to be set at lookup (WIP) Paul Lawrence
                                     ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Paul Lawrence @ 2025-08-04 17:32 UTC (permalink / raw)
  To: amir73il
  Cc: bernd.schubert, linux-fsdevel, linux-kernel, miklos, paullawrence

Based on our discussion, I put together two simple patches.

The first adds an optional extra parameter to FUSE_LOOKUP outargs. This allows
the daemon to set a backing file at lookup time on a successful lookup.

I then looked at which opcodes do not require a file handle. The simplest seem
to be FUSE_MKDIR and FUSE_RMDIR. So I implemented passthrough handling for these
opcodes in the second patch.

Both patches sit on top of Amir's tree at:

https://github.com/amir73il/linux/commit/ceaf7f16452f6aaf7993279b1c10e727d6bf6a32

Thoughts?

Paul

Paul Lawrence (2):
  fuse: Allow backing file to be set at lookup (WIP)
  fuse: Add passthrough for mkdir and rmdir (WIP)

 fs/fuse/dir.c             | 31 +++++++++++++---
 fs/fuse/fuse_i.h          | 14 ++++++-
 fs/fuse/iomode.c          | 41 ++++++++++++++++++--
 fs/fuse/passthrough.c     | 78 ++++++++++++++++++++++++++++++++++-----
 include/uapi/linux/fuse.h |  6 +++
 5 files changed, 150 insertions(+), 20 deletions(-)

-- 
2.50.1.565.gc32cd1483b-goog


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/2] fuse: Allow backing file to be set at lookup (WIP)
  2025-08-04 17:32                 ` [PATCH 0/2] RFC: Set backing file at lookup Paul Lawrence
@ 2025-08-04 17:32                   ` Paul Lawrence
  2025-08-10 14:21                     ` Amir Goldstein
  2025-08-04 17:32                   ` [PATCH 2/2] fuse: Add passthrough for mkdir and rmdir (WIP) Paul Lawrence
  2025-08-10 13:25                   ` [PATCH 0/2] RFC: Set backing file at lookup Amir Goldstein
  2 siblings, 1 reply; 19+ messages in thread
From: Paul Lawrence @ 2025-08-04 17:32 UTC (permalink / raw)
  To: amir73il
  Cc: bernd.schubert, linux-fsdevel, linux-kernel, miklos, paullawrence

Add optional extra outarg to FUSE_LOOKUP which holds a backing id to set
a backing file at lookup.

Signed-off-by: Paul Lawrence <paullawrence@google.com>
---
 fs/fuse/dir.c             | 23 ++++++++++++++++++----
 fs/fuse/fuse_i.h          |  3 +++
 fs/fuse/iomode.c          | 41 +++++++++++++++++++++++++++++++++++----
 fs/fuse/passthrough.c     | 40 +++++++++++++++++++++++++++++---------
 include/uapi/linux/fuse.h |  4 ++++
 5 files changed, 94 insertions(+), 17 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 62508d212826..c0bef93dd078 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -170,7 +170,8 @@ static void fuse_invalidate_entry(struct dentry *entry)
 
 static void fuse_lookup_init(struct fuse_conn *fc, struct fuse_args *args,
 			     u64 nodeid, const struct qstr *name,
-			     struct fuse_entry_out *outarg)
+			     struct fuse_entry_out *outarg,
+			     struct fuse_entry_passthrough_out *backing)
 {
 	memset(outarg, 0, sizeof(struct fuse_entry_out));
 	args->opcode = FUSE_LOOKUP;
@@ -184,6 +185,12 @@ static void fuse_lookup_init(struct fuse_conn *fc, struct fuse_args *args,
 	args->out_numargs = 1;
 	args->out_args[0].size = sizeof(struct fuse_entry_out);
 	args->out_args[0].value = outarg;
+	if (backing) {
+		args->out_numargs = 2;
+		args->out_args[1].size = sizeof(struct fuse_entry_passthrough_out);
+		args->out_args[1].value = backing;
+		args->out_argvar = true;
+	}
 }
 
 /*
@@ -236,7 +243,7 @@ static int fuse_dentry_revalidate(struct inode *dir, const struct qstr *name,
 		attr_version = fuse_get_attr_version(fm->fc);
 
 		fuse_lookup_init(fm->fc, &args, get_node_id(dir),
-				 name, &outarg);
+				 name, &outarg, NULL);
 		ret = fuse_simple_request(fm, &args);
 		/* Zero nodeid is same as -ENOENT */
 		if (!ret && !outarg.nodeid)
@@ -369,6 +376,7 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
 	struct fuse_forget_link *forget;
 	u64 attr_version, evict_ctr;
 	int err;
+	struct fuse_entry_passthrough_out passthrough;
 
 	*inode = NULL;
 	err = -ENAMETOOLONG;
@@ -384,10 +392,10 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
 	attr_version = fuse_get_attr_version(fm->fc);
 	evict_ctr = fuse_get_evict_ctr(fm->fc);
 
-	fuse_lookup_init(fm->fc, &args, nodeid, name, outarg);
+	fuse_lookup_init(fm->fc, &args, nodeid, name, outarg, &passthrough);
 	err = fuse_simple_request(fm, &args);
 	/* Zero nodeid is same as -ENOENT, but with valid timeout */
-	if (err || !outarg->nodeid)
+	if (err < 0 || !outarg->nodeid)
 		goto out_put_forget;
 
 	err = -EIO;
@@ -406,6 +414,13 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
 		fuse_queue_forget(fm->fc, forget, outarg->nodeid, 1);
 		goto out;
 	}
+
+	// TODO check that if fuse_backing is already set they are consistent
+	if (args.out_args[1].size && !get_fuse_inode(*inode)->fb) {
+		err = fuse_inode_set_passthrough(*inode, passthrough.backing_id);
+		if (err)
+			goto out;
+	}
 	err = 0;
 
  out_put_forget:
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 1e8e732a2f09..aebd338751f1 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1595,9 +1595,12 @@ ssize_t fuse_passthrough_splice_read(struct file *in, loff_t *ppos,
 ssize_t fuse_passthrough_splice_write(struct pipe_inode_info *pipe,
 				      struct file *out, loff_t *ppos,
 				      size_t len, unsigned int flags);
+struct fuse_backing *fuse_backing_id_get(struct fuse_conn *fc, int backing_id);
 ssize_t fuse_passthrough_mmap(struct file *file, struct vm_area_struct *vma);
 int fuse_passthrough_readdir(struct file *file, struct dir_context *ctx);
 
+int fuse_inode_set_passthrough(struct inode *inode, int backing_id);
+
 static inline struct fuse_backing *fuse_inode_passthrough(struct fuse_inode *fi)
 {
 #ifdef CONFIG_FUSE_PASSTHROUGH
diff --git a/fs/fuse/iomode.c b/fs/fuse/iomode.c
index f46dfa040e53..4c23ae640624 100644
--- a/fs/fuse/iomode.c
+++ b/fs/fuse/iomode.c
@@ -166,6 +166,37 @@ static void fuse_file_uncached_io_release(struct fuse_file *ff,
 	fuse_inode_uncached_io_end(fi);
 }
 
+/* Setup passthrough for inode operations without an open file */
+int fuse_inode_set_passthrough(struct inode *inode, int backing_id)
+{
+	struct fuse_conn *fc = get_fuse_conn(inode);
+	struct fuse_inode *fi = get_fuse_inode(inode);
+	struct fuse_backing *fb;
+	int err;
+
+	if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH) || !fc->passthrough_ino)
+		return 0;
+
+	/* backing inode is set once for the lifetime of the inode */
+	if (fuse_inode_passthrough(fi))
+		return 0;
+
+	fb = fuse_backing_id_get(fc, backing_id);
+	err = PTR_ERR(fb);
+	if (IS_ERR(fb))
+		goto fail;
+
+	fi->fb = fb;
+	set_bit(FUSE_I_PASSTHROUGH, &fi->state);
+	fi->iocachectr--;
+	return 0;
+
+fail:
+	pr_debug("failed to setup backing inode (ino=%lu, backing_id=%d, err=%i).\n",
+		 inode->i_ino, backing_id, err);
+	return err;
+}
+
 /*
  * Open flags that are allowed in combination with FOPEN_PASSTHROUGH.
  * A combination of FOPEN_PASSTHROUGH and FOPEN_DIRECT_IO means that read/write
@@ -185,8 +216,10 @@ static int fuse_file_passthrough_open(struct inode *inode, struct file *file)
 	int err;
 
 	/* Check allowed conditions for file open in passthrough mode */
-	if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH) || !fc->passthrough ||
-	    (ff->open_flags & ~FOPEN_PASSTHROUGH_MASK))
+	if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH) || !fc->passthrough)
+		return -EINVAL;
+
+	if (ff->open_flags & ~FOPEN_PASSTHROUGH_MASK && !fuse_inode_backing(get_fuse_inode(inode)))
 		return -EINVAL;
 
 	fb = fuse_passthrough_open(file, inode,
@@ -224,8 +257,8 @@ int fuse_file_io_open(struct file *file, struct inode *inode)
 	 * which is already open for passthrough.
 	 */
 	err = -EINVAL;
-	if (fuse_inode_backing(fi) && !(ff->open_flags & FOPEN_PASSTHROUGH))
-		goto fail;
+	if (fuse_inode_backing(fi))
+		ff->open_flags |= FOPEN_PASSTHROUGH;
 
 	/*
 	 * FOPEN_PARALLEL_DIRECT_WRITES requires FOPEN_DIRECT_IO.
diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c
index de6ece996ff8..cee40e1c6e4a 100644
--- a/fs/fuse/passthrough.c
+++ b/fs/fuse/passthrough.c
@@ -229,7 +229,6 @@ static int fuse_backing_id_free(int id, void *p, void *data)
 {
 	struct fuse_backing *fb = p;
 
-	WARN_ON_ONCE(refcount_read(&fb->count) != 1);
 	fuse_backing_free(fb);
 	return 0;
 }
@@ -348,6 +347,29 @@ int fuse_backing_close(struct fuse_conn *fc, int backing_id)
 	return err;
 }
 
+/*
+ * Get fuse backing object by backing id.
+ *
+ * Returns an fb object with elevated refcount to be stored in fuse inode.
+ */
+struct fuse_backing *fuse_backing_id_get(struct fuse_conn *fc, int backing_id)
+{
+	struct fuse_backing *fb;
+
+	if (backing_id <= 0)
+		return ERR_PTR(-EINVAL);
+
+	rcu_read_lock();
+	fb = idr_find(&fc->backing_files_map, backing_id);
+	fb = fuse_backing_get(fb);
+	rcu_read_unlock();
+
+	if (!fb)
+		return ERR_PTR(-ENOENT);
+
+	return fb;
+}
+
 /*
  * Setup passthrough to a backing file.
  *
@@ -363,18 +385,18 @@ struct fuse_backing *fuse_passthrough_open(struct file *file,
 	struct file *backing_file;
 	int err;
 
-	err = -EINVAL;
-	if (backing_id <= 0)
-		goto out;
-
 	rcu_read_lock();
-	fb = idr_find(&fc->backing_files_map, backing_id);
+	if (backing_id <= 0) {
+		err = -EINVAL;
+		fb = fuse_inode_backing(get_fuse_inode(inode));
+	} else {
+		err = -ENOENT;
+		fb = idr_find(&fc->backing_files_map, backing_id);
+	}
 	fb = fuse_backing_get(fb);
-	rcu_read_unlock();
-
-	err = -ENOENT;
 	if (!fb)
 		goto out;
+	rcu_read_unlock();
 
 	/* Allocate backing file per fuse file to store fuse path */
 	backing_file = backing_file_open(&file->f_path, file->f_flags,
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index ff769766b748..6dbb045c794d 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -695,6 +695,10 @@ struct fuse_entry_out {
 	struct fuse_attr attr;
 };
 
+struct fuse_entry_passthrough_out {
+	int32_t 	backing_id;
+};
+
 struct fuse_forget_in {
 	uint64_t	nlookup;
 };
-- 
2.50.1.565.gc32cd1483b-goog


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/2] fuse: Add passthrough for mkdir and rmdir (WIP)
  2025-08-04 17:32                 ` [PATCH 0/2] RFC: Set backing file at lookup Paul Lawrence
  2025-08-04 17:32                   ` [PATCH 1/2] fuse: Allow backing file to be set at lookup (WIP) Paul Lawrence
@ 2025-08-04 17:32                   ` Paul Lawrence
  2025-08-10 15:40                     ` Amir Goldstein
  2025-08-10 13:25                   ` [PATCH 0/2] RFC: Set backing file at lookup Amir Goldstein
  2 siblings, 1 reply; 19+ messages in thread
From: Paul Lawrence @ 2025-08-04 17:32 UTC (permalink / raw)
  To: amir73il
  Cc: bernd.schubert, linux-fsdevel, linux-kernel, miklos, paullawrence

As proof of concept of setting a backing file at lookup, implement mkdir
and rmdir which work off the nodeid only and do not open the file.

Signed-off-by: Paul Lawrence <paullawrence@google.com>
---
 fs/fuse/dir.c             |  8 +++++++-
 fs/fuse/fuse_i.h          | 11 +++++++++--
 fs/fuse/passthrough.c     | 38 ++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/fuse.h |  2 ++
 4 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index c0bef93dd078..25d6929d600a 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -129,7 +129,7 @@ void fuse_invalidate_attr(struct inode *inode)
 	fuse_invalidate_attr_mask(inode, STATX_BASIC_STATS);
 }
 
-static void fuse_dir_changed(struct inode *dir)
+void fuse_dir_changed(struct inode *dir)
 {
 	fuse_invalidate_attr(dir);
 	inode_maybe_inc_iversion(dir, false);
@@ -951,6 +951,9 @@ static struct dentry *fuse_mkdir(struct mnt_idmap *idmap, struct inode *dir,
 	if (!fm->fc->dont_mask)
 		mode &= ~current_umask();
 
+	if (fuse_inode_passthrough_op(dir, FUSE_MKDIR))
+		return fuse_passthrough_mkdir(idmap, dir, entry, mode);
+
 	memset(&inarg, 0, sizeof(inarg));
 	inarg.mode = mode;
 	inarg.umask = current_umask();
@@ -1058,6 +1061,9 @@ static int fuse_rmdir(struct inode *dir, struct dentry *entry)
 	if (fuse_is_bad(dir))
 		return -EIO;
 
+	if (fuse_inode_passthrough_op(dir, FUSE_RMDIR))
+		return fuse_passthrough_rmdir(dir, entry);
+
 	args.opcode = FUSE_RMDIR;
 	args.nodeid = get_node_id(dir);
 	args.in_numargs = 2;
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index aebd338751f1..d8df2d5a73ac 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1279,6 +1279,7 @@ void fuse_check_timeout(struct work_struct *work);
 #define FUSE_STATX_MODSIZE	(FUSE_STATX_MODIFY | STATX_SIZE)
 
 void fuse_invalidate_attr(struct inode *inode);
+void fuse_dir_changed(struct inode *dir);
 void fuse_invalidate_attr_mask(struct inode *inode, u32 mask);
 
 void fuse_invalidate_entry_cache(struct dentry *entry);
@@ -1521,7 +1522,8 @@ void fuse_file_release(struct inode *inode, struct fuse_file *ff,
 
 /* Passthrough operations for directories */
 #define FUSE_PASSTHROUGH_DIR_OPS \
-	(FUSE_PASSTHROUGH_OP_READDIR)
+	(FUSE_PASSTHROUGH_OP_READDIR | FUSE_PASSTHROUGH_OP_MKDIR | \
+	 FUSE_PASSTHROUGH_OP_RMDIR)
 
 /* Inode passthrough operations for backing file attached to inode */
 #define FUSE_PASSTHROUGH_INODE_OPS \
@@ -1532,7 +1534,8 @@ void fuse_file_release(struct inode *inode, struct fuse_file *ff,
 	((map)->ops_mask & FUSE_PASSTHROUGH_OP(op))
 
 #define FUSE_BACKING_MAP_VALID_OPS \
-	(FUSE_PASSTHROUGH_RW_OPS | FUSE_PASSTHROUGH_INODE_OPS)
+	(FUSE_PASSTHROUGH_RW_OPS | FUSE_PASSTHROUGH_INODE_OPS |\
+	 FUSE_PASSTHROUGH_DIR_OPS)
 
 static inline struct fuse_backing *fuse_inode_backing(struct fuse_inode *fi)
 {
@@ -1626,6 +1629,10 @@ ssize_t fuse_passthrough_getxattr(struct inode *inode, const char *name,
 				  void *value, size_t size);
 ssize_t fuse_passthrough_listxattr(struct dentry *entry, char *list,
 				   size_t size);
+struct dentry *fuse_passthrough_mkdir(struct mnt_idmap *idmap,
+				      struct inode *dir, struct dentry *entry,
+				      umode_t mode);
+int fuse_passthrough_rmdir(struct inode *dir, struct dentry *entry);
 
 #ifdef CONFIG_SYSCTL
 extern int fuse_sysctl_register(void);
diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c
index cee40e1c6e4a..acb06fbbd828 100644
--- a/fs/fuse/passthrough.c
+++ b/fs/fuse/passthrough.c
@@ -7,6 +7,7 @@
 
 #include "fuse_i.h"
 
+#include "linux/namei.h"
 #include <linux/file.h>
 #include <linux/backing-file.h>
 #include <linux/splice.h>
@@ -497,3 +498,40 @@ ssize_t fuse_passthrough_listxattr(struct dentry *entry, char *list,
 	revert_creds(old_cred);
 	return res;
 }
+
+struct dentry *fuse_passthrough_mkdir(struct mnt_idmap *idmap,
+				      struct inode *dir, struct dentry *entry,
+				      umode_t mode)
+{
+	struct fuse_backing *fb = fuse_inode_backing(get_fuse_inode(dir));
+	struct dentry *backing_entry, *new_entry;
+	const struct cred *old_cred;
+
+	old_cred = override_creds(fb->cred);
+	backing_entry = lookup_one_unlocked(idmap, &entry->d_name,
+		fb->file->f_path.dentry);
+	new_entry = vfs_mkdir(idmap, fb->file->f_inode, backing_entry, mode);
+	d_drop(entry);
+	revert_creds(old_cred);
+	fuse_dir_changed(dir);
+	return new_entry;
+}
+
+int fuse_passthrough_rmdir(struct inode *dir, struct dentry *entry)
+{
+	int err;
+	struct dentry *backing_entry;
+	struct fuse_backing *fb = fuse_inode_backing(get_fuse_inode(dir));
+	const struct cred *old_cred;
+
+	old_cred = override_creds(fb->cred);
+	backing_entry = lookup_one_unlocked(&nop_mnt_idmap, &entry->d_name,
+		fb->file->f_path.dentry);
+	err = vfs_rmdir(&nop_mnt_idmap, fb->file->f_inode, backing_entry);
+	dput(backing_entry);
+	if (!err)
+		d_drop(entry);
+	revert_creds(old_cred);
+	return err;
+}
+
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 6dbb045c794d..8181d07b7bf1 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -1135,6 +1135,8 @@ struct fuse_backing_map {
 #define FUSE_PASSTHROUGH_OP_STATX	FUSE_PASSTHROUGH_OP(FUSE_STATX)
 #define FUSE_PASSTHROUGH_OP_GETXATTR	FUSE_PASSTHROUGH_OP(FUSE_GETXATTR)
 #define FUSE_PASSTHROUGH_OP_LISTXATTR	FUSE_PASSTHROUGH_OP(FUSE_LISTXATTR)
+#define FUSE_PASSTHROUGH_OP_MKDIR	FUSE_PASSTHROUGH_OP(FUSE_MKDIR)
+#define FUSE_PASSTHROUGH_OP_RMDIR	FUSE_PASSTHROUGH_OP(FUSE_RMDIR)
 
 /* Device ioctls: */
 #define FUSE_DEV_IOC_MAGIC		229
-- 
2.50.1.565.gc32cd1483b-goog


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/2] RFC: Set backing file at lookup
  2025-08-04 17:32                 ` [PATCH 0/2] RFC: Set backing file at lookup Paul Lawrence
  2025-08-04 17:32                   ` [PATCH 1/2] fuse: Allow backing file to be set at lookup (WIP) Paul Lawrence
  2025-08-04 17:32                   ` [PATCH 2/2] fuse: Add passthrough for mkdir and rmdir (WIP) Paul Lawrence
@ 2025-08-10 13:25                   ` Amir Goldstein
  2025-08-10 14:32                     ` Amir Goldstein
  2 siblings, 1 reply; 19+ messages in thread
From: Amir Goldstein @ 2025-08-10 13:25 UTC (permalink / raw)
  To: Paul Lawrence; +Cc: bernd.schubert, linux-fsdevel, linux-kernel, miklos

On Mon, Aug 4, 2025 at 7:32 PM Paul Lawrence <paullawrence@google.com> wrote:
>
> Based on our discussion, I put together two simple patches.
>
> The first adds an optional extra parameter to FUSE_LOOKUP outargs. This allows
> the daemon to set a backing file at lookup time on a successful lookup.
>
> I then looked at which opcodes do not require a file handle. The simplest seem
> to be FUSE_MKDIR and FUSE_RMDIR. So I implemented passthrough handling for these
> opcodes in the second patch.
>
> Both patches sit on top of Amir's tree at:
>
> https://github.com/amir73il/linux/commit/ceaf7f16452f6aaf7993279b1c10e727d6bf6a32
>

I think you based your patches on ceaf7f16452f^ and patch 1/2 replaces commit
ceaf7f16452f ("fuse: support setting backing inode passthrough on getattr")

Right?

That makes sense to me because that last patch was a hacky API,
but then you made some other changes to my patch which I did not understand why.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] fuse: Allow backing file to be set at lookup (WIP)
  2025-08-04 17:32                   ` [PATCH 1/2] fuse: Allow backing file to be set at lookup (WIP) Paul Lawrence
@ 2025-08-10 14:21                     ` Amir Goldstein
  0 siblings, 0 replies; 19+ messages in thread
From: Amir Goldstein @ 2025-08-10 14:21 UTC (permalink / raw)
  To: Paul Lawrence; +Cc: bernd.schubert, linux-fsdevel, linux-kernel, miklos

On Mon, Aug 4, 2025 at 7:32 PM Paul Lawrence <paullawrence@google.com> wrote:
>
> Add optional extra outarg to FUSE_LOOKUP which holds a backing id to set
> a backing file at lookup.
>
> Signed-off-by: Paul Lawrence <paullawrence@google.com>
> ---
>  fs/fuse/dir.c             | 23 ++++++++++++++++++----
>  fs/fuse/fuse_i.h          |  3 +++
>  fs/fuse/iomode.c          | 41 +++++++++++++++++++++++++++++++++++----
>  fs/fuse/passthrough.c     | 40 +++++++++++++++++++++++++++++---------
>  include/uapi/linux/fuse.h |  4 ++++
>  5 files changed, 94 insertions(+), 17 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 62508d212826..c0bef93dd078 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -170,7 +170,8 @@ static void fuse_invalidate_entry(struct dentry *entry)
>
>  static void fuse_lookup_init(struct fuse_conn *fc, struct fuse_args *args,
>                              u64 nodeid, const struct qstr *name,
> -                            struct fuse_entry_out *outarg)
> +                            struct fuse_entry_out *outarg,
> +                            struct fuse_entry_passthrough_out *backing)
>  {
>         memset(outarg, 0, sizeof(struct fuse_entry_out));
>         args->opcode = FUSE_LOOKUP;
> @@ -184,6 +185,12 @@ static void fuse_lookup_init(struct fuse_conn *fc, struct fuse_args *args,
>         args->out_numargs = 1;
>         args->out_args[0].size = sizeof(struct fuse_entry_out);
>         args->out_args[0].value = outarg;
> +       if (backing) {
> +               args->out_numargs = 2;
> +               args->out_args[1].size = sizeof(struct fuse_entry_passthrough_out);
> +               args->out_args[1].value = backing;
> +               args->out_argvar = true;
> +       }
>  }
>
>  /*
> @@ -236,7 +243,7 @@ static int fuse_dentry_revalidate(struct inode *dir, const struct qstr *name,
>                 attr_version = fuse_get_attr_version(fm->fc);
>
>                 fuse_lookup_init(fm->fc, &args, get_node_id(dir),
> -                                name, &outarg);
> +                                name, &outarg, NULL);
>                 ret = fuse_simple_request(fm, &args);
>                 /* Zero nodeid is same as -ENOENT */
>                 if (!ret && !outarg.nodeid)
> @@ -369,6 +376,7 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
>         struct fuse_forget_link *forget;
>         u64 attr_version, evict_ctr;
>         int err;
> +       struct fuse_entry_passthrough_out passthrough;
>
>         *inode = NULL;
>         err = -ENAMETOOLONG;
> @@ -384,10 +392,10 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
>         attr_version = fuse_get_attr_version(fm->fc);
>         evict_ctr = fuse_get_evict_ctr(fm->fc);
>
> -       fuse_lookup_init(fm->fc, &args, nodeid, name, outarg);
> +       fuse_lookup_init(fm->fc, &args, nodeid, name, outarg, &passthrough);
>         err = fuse_simple_request(fm, &args);
>         /* Zero nodeid is same as -ENOENT, but with valid timeout */
> -       if (err || !outarg->nodeid)
> +       if (err < 0 || !outarg->nodeid)

Why this change?

>                 goto out_put_forget;
>
>         err = -EIO;
> @@ -406,6 +414,13 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
>                 fuse_queue_forget(fm->fc, forget, outarg->nodeid, 1);
>                 goto out;
>         }
> +
> +       // TODO check that if fuse_backing is already set they are consistent
> +       if (args.out_args[1].size && !get_fuse_inode(*inode)->fb) {

This check is not atomic.
fuse_inode_set_passthrough() already does nothing if
fuse_inode_passthrough() is already set.

> +               err = fuse_inode_set_passthrough(*inode, passthrough.backing_id);
> +               if (err)
> +                       goto out;

This needs to queue forget + iput.

> +       }
>         err = 0;
>
>   out_put_forget:
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 1e8e732a2f09..aebd338751f1 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -1595,9 +1595,12 @@ ssize_t fuse_passthrough_splice_read(struct file *in, loff_t *ppos,
>  ssize_t fuse_passthrough_splice_write(struct pipe_inode_info *pipe,
>                                       struct file *out, loff_t *ppos,
>                                       size_t len, unsigned int flags);
> +struct fuse_backing *fuse_backing_id_get(struct fuse_conn *fc, int backing_id);
>  ssize_t fuse_passthrough_mmap(struct file *file, struct vm_area_struct *vma);
>  int fuse_passthrough_readdir(struct file *file, struct dir_context *ctx);
>
> +int fuse_inode_set_passthrough(struct inode *inode, int backing_id);
> +
>  static inline struct fuse_backing *fuse_inode_passthrough(struct fuse_inode *fi)
>  {
>  #ifdef CONFIG_FUSE_PASSTHROUGH
> diff --git a/fs/fuse/iomode.c b/fs/fuse/iomode.c
> index f46dfa040e53..4c23ae640624 100644
> --- a/fs/fuse/iomode.c
> +++ b/fs/fuse/iomode.c
> @@ -166,6 +166,37 @@ static void fuse_file_uncached_io_release(struct fuse_file *ff,
>         fuse_inode_uncached_io_end(fi);
>  }
>
> +/* Setup passthrough for inode operations without an open file */
> +int fuse_inode_set_passthrough(struct inode *inode, int backing_id)
> +{
> +       struct fuse_conn *fc = get_fuse_conn(inode);
> +       struct fuse_inode *fi = get_fuse_inode(inode);
> +       struct fuse_backing *fb;
> +       int err;
> +
> +       if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH) || !fc->passthrough_ino)
> +               return 0;
> +
> +       /* backing inode is set once for the lifetime of the inode */
> +       if (fuse_inode_passthrough(fi))
> +               return 0;
> +
> +       fb = fuse_backing_id_get(fc, backing_id);
> +       err = PTR_ERR(fb);
> +       if (IS_ERR(fb))
> +               goto fail;
> +
> +       fi->fb = fb;
> +       set_bit(FUSE_I_PASSTHROUGH, &fi->state);
> +       fi->iocachectr--;

These need a lock. My patch calls fuse_inode_uncached_io_start()
Why did you change that?

My patch also requires a minimal passthrough op:

        /* Backing inode requires at least GETATTR op passthrough */
        err = -EOPNOTSUPP;
        if (!(fb->ops_mask & FUSE_PASSTHROUGH_OP(FUSE_GETATTR)))
                goto fail;

Why did you remove this? it seems sane to me not to ever have to deal
with inode attr cache when setting up passthrough to inode operations.

I've split my patch to prep patch that adds those helpers and the demo API.
push to branch fuse_passthrough_iops on my github.
Please use the prep patch as is without changing the helper functions
unless you have a good reason to change them and document this reason.

> +       return 0;
> +
> +fail:
> +       pr_debug("failed to setup backing inode (ino=%lu, backing_id=%d, err=%i).\n",
> +                inode->i_ino, backing_id, err);
> +       return err;
> +}
> +
>  /*
>   * Open flags that are allowed in combination with FOPEN_PASSTHROUGH.
>   * A combination of FOPEN_PASSTHROUGH and FOPEN_DIRECT_IO means that read/write
> @@ -185,8 +216,10 @@ static int fuse_file_passthrough_open(struct inode *inode, struct file *file)
>         int err;
>
>         /* Check allowed conditions for file open in passthrough mode */
> -       if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH) || !fc->passthrough ||
> -           (ff->open_flags & ~FOPEN_PASSTHROUGH_MASK))
> +       if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH) || !fc->passthrough)
> +               return -EINVAL;
> +
> +       if (ff->open_flags & ~FOPEN_PASSTHROUGH_MASK && !fuse_inode_backing(get_fuse_inode(inode)))

I don't understand this condition.
If there is already a backing inode, then open_flags are ignored?
This does not make sense to me.
What is the reason to make this change?

>                 return -EINVAL;
>
>         fb = fuse_passthrough_open(file, inode,
> @@ -224,8 +257,8 @@ int fuse_file_io_open(struct file *file, struct inode *inode)
>          * which is already open for passthrough.
>          */
>         err = -EINVAL;
> -       if (fuse_inode_backing(fi) && !(ff->open_flags & FOPEN_PASSTHROUGH))
> -               goto fail;
> +       if (fuse_inode_backing(fi))
> +               ff->open_flags |= FOPEN_PASSTHROUGH;

I agree that it makes some sense for an inode in passthrough mode
to imply FOPEN_PASSTHROUGH, as long as FOPEN_DIRECT_IO
is not cleared, but what do we really gain from making this flag implicit?
If a server is setting up backing inodes, what's the problem with keeping
existing API and requiring the server to use explicit FOPEN_PASSTHROUGH
flag for the inodes that it had mapped?
I understand that you want the server to be able to use backing_id 0
for open for simplicity in this case?
Nevertheless, I see no reason to drop the flag.

>
>         /*
>          * FOPEN_PARALLEL_DIRECT_WRITES requires FOPEN_DIRECT_IO.
> diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c
> index de6ece996ff8..cee40e1c6e4a 100644
> --- a/fs/fuse/passthrough.c
> +++ b/fs/fuse/passthrough.c
> @@ -229,7 +229,6 @@ static int fuse_backing_id_free(int id, void *p, void *data)
>  {
>         struct fuse_backing *fb = p;
>
> -       WARN_ON_ONCE(refcount_read(&fb->count) != 1);

Why remove this assertion?
Did you change the refcounting rules?
Why? and if so, please document the change.

>         fuse_backing_free(fb);
>         return 0;
>  }
> @@ -348,6 +347,29 @@ int fuse_backing_close(struct fuse_conn *fc, int backing_id)
>         return err;
>  }
>
> +/*
> + * Get fuse backing object by backing id.
> + *
> + * Returns an fb object with elevated refcount to be stored in fuse inode.
> + */
> +struct fuse_backing *fuse_backing_id_get(struct fuse_conn *fc, int backing_id)
> +{
> +       struct fuse_backing *fb;
> +
> +       if (backing_id <= 0)
> +               return ERR_PTR(-EINVAL);
> +
> +       rcu_read_lock();
> +       fb = idr_find(&fc->backing_files_map, backing_id);
> +       fb = fuse_backing_get(fb);
> +       rcu_read_unlock();
> +
> +       if (!fb)
> +               return ERR_PTR(-ENOENT);
> +
> +       return fb;
> +}
> +
>  /*
>   * Setup passthrough to a backing file.
>   *
> @@ -363,18 +385,18 @@ struct fuse_backing *fuse_passthrough_open(struct file *file,
>         struct file *backing_file;
>         int err;
>
> -       err = -EINVAL;
> -       if (backing_id <= 0)
> -               goto out;
> -
>         rcu_read_lock();
> -       fb = idr_find(&fc->backing_files_map, backing_id);
> +       if (backing_id <= 0) {
> +               err = -EINVAL;
> +               fb = fuse_inode_backing(get_fuse_inode(inode));
> +       } else {
> +               err = -ENOENT;
> +               fb = idr_find(&fc->backing_files_map, backing_id);
> +       }
>         fb = fuse_backing_get(fb);
> -       rcu_read_unlock();
> -
> -       err = -ENOENT;

Maybe you are over complicating this?

My patch replaced all of the above with:

       fb = fuse_backing_id_get(fc, backing_id);
       if (IS_ERR(fb)) {
               err = PTR_ERR(fb);
               fb = NULL;
        }

I think you want something like:

       if (!backing_id) {
               fb = fuse_backing_get(fuse_inode_passthrough(fi));
       } else {
               fb = fuse_backing_id_get(fc, backing_id);
       }
       if (IS_ERR_OR_NULL(fb)) {
               err = fb ? PTR_ERR(fb) : -ENOENT;
               fb = NULL;
       }

But also, that is a separate API change.
You should do it in its own patch that documents the change
and not in the same patch that implements the backing inode
setup on lookup response.

My patches added fuse_inode_passthrough() exactly so you can
write code that checks if inode has permanent backing inode
*before* you add the API to make this association.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/2] RFC: Set backing file at lookup
  2025-08-10 13:25                   ` [PATCH 0/2] RFC: Set backing file at lookup Amir Goldstein
@ 2025-08-10 14:32                     ` Amir Goldstein
  0 siblings, 0 replies; 19+ messages in thread
From: Amir Goldstein @ 2025-08-10 14:32 UTC (permalink / raw)
  To: Paul Lawrence; +Cc: bernd.schubert, linux-fsdevel, linux-kernel, miklos

On Sun, Aug 10, 2025 at 3:25 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Mon, Aug 4, 2025 at 7:32 PM Paul Lawrence <paullawrence@google.com> wrote:
> >
> > Based on our discussion, I put together two simple patches.
> >
> > The first adds an optional extra parameter to FUSE_LOOKUP outargs. This allows
> > the daemon to set a backing file at lookup time on a successful lookup.
> >
> > I then looked at which opcodes do not require a file handle. The simplest seem
> > to be FUSE_MKDIR and FUSE_RMDIR. So I implemented passthrough handling for these
> > opcodes in the second patch.
> >
> > Both patches sit on top of Amir's tree at:
> >
> > https://github.com/amir73il/linux/commit/ceaf7f16452f6aaf7993279b1c10e727d6bf6a32
> >
>
> I think you based your patches on ceaf7f16452f^ and patch 1/2 replaces commit
> ceaf7f16452f ("fuse: support setting backing inode passthrough on getattr")
>
> Right?
>
> That makes sense to me because that last patch was a hacky API,
> but then you made some other changes to my patch which I did not understand why.

Push a new version of fuse-backing-inode-wip based on
https://github.com/amir73il/libfuse/commits/fuse_passthrough_iops/

Please base your work on the above branch with the helpers instead of
modifying them.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] fuse: Add passthrough for mkdir and rmdir (WIP)
  2025-08-04 17:32                   ` [PATCH 2/2] fuse: Add passthrough for mkdir and rmdir (WIP) Paul Lawrence
@ 2025-08-10 15:40                     ` Amir Goldstein
  0 siblings, 0 replies; 19+ messages in thread
From: Amir Goldstein @ 2025-08-10 15:40 UTC (permalink / raw)
  To: Paul Lawrence; +Cc: bernd.schubert, linux-fsdevel, linux-kernel, miklos

On Mon, Aug 4, 2025 at 7:32 PM Paul Lawrence <paullawrence@google.com> wrote:
>
> As proof of concept of setting a backing file at lookup, implement mkdir
> and rmdir which work off the nodeid only and do not open the file.
>
> Signed-off-by: Paul Lawrence <paullawrence@google.com>
> ---
>  fs/fuse/dir.c             |  8 +++++++-
>  fs/fuse/fuse_i.h          | 11 +++++++++--
>  fs/fuse/passthrough.c     | 38 ++++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/fuse.h |  2 ++
>  4 files changed, 56 insertions(+), 3 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index c0bef93dd078..25d6929d600a 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -129,7 +129,7 @@ void fuse_invalidate_attr(struct inode *inode)
>         fuse_invalidate_attr_mask(inode, STATX_BASIC_STATS);
>  }
>
> -static void fuse_dir_changed(struct inode *dir)
> +void fuse_dir_changed(struct inode *dir)
>  {
>         fuse_invalidate_attr(dir);
>         inode_maybe_inc_iversion(dir, false);
> @@ -951,6 +951,9 @@ static struct dentry *fuse_mkdir(struct mnt_idmap *idmap, struct inode *dir,
>         if (!fm->fc->dont_mask)
>                 mode &= ~current_umask();
>
> +       if (fuse_inode_passthrough_op(dir, FUSE_MKDIR))
> +               return fuse_passthrough_mkdir(idmap, dir, entry, mode);
> +
>         memset(&inarg, 0, sizeof(inarg));
>         inarg.mode = mode;
>         inarg.umask = current_umask();
> @@ -1058,6 +1061,9 @@ static int fuse_rmdir(struct inode *dir, struct dentry *entry)
>         if (fuse_is_bad(dir))
>                 return -EIO;
>
> +       if (fuse_inode_passthrough_op(dir, FUSE_RMDIR))
> +               return fuse_passthrough_rmdir(dir, entry);
> +
>         args.opcode = FUSE_RMDIR;
>         args.nodeid = get_node_id(dir);
>         args.in_numargs = 2;
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index aebd338751f1..d8df2d5a73ac 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -1279,6 +1279,7 @@ void fuse_check_timeout(struct work_struct *work);
>  #define FUSE_STATX_MODSIZE     (FUSE_STATX_MODIFY | STATX_SIZE)
>
>  void fuse_invalidate_attr(struct inode *inode);
> +void fuse_dir_changed(struct inode *dir);
>  void fuse_invalidate_attr_mask(struct inode *inode, u32 mask);
>
>  void fuse_invalidate_entry_cache(struct dentry *entry);
> @@ -1521,7 +1522,8 @@ void fuse_file_release(struct inode *inode, struct fuse_file *ff,
>
>  /* Passthrough operations for directories */
>  #define FUSE_PASSTHROUGH_DIR_OPS \
> -       (FUSE_PASSTHROUGH_OP_READDIR)
> +       (FUSE_PASSTHROUGH_OP_READDIR | FUSE_PASSTHROUGH_OP_MKDIR | \
> +        FUSE_PASSTHROUGH_OP_RMDIR)
>
>  /* Inode passthrough operations for backing file attached to inode */
>  #define FUSE_PASSTHROUGH_INODE_OPS \
> @@ -1532,7 +1534,8 @@ void fuse_file_release(struct inode *inode, struct fuse_file *ff,
>         ((map)->ops_mask & FUSE_PASSTHROUGH_OP(op))
>
>  #define FUSE_BACKING_MAP_VALID_OPS \
> -       (FUSE_PASSTHROUGH_RW_OPS | FUSE_PASSTHROUGH_INODE_OPS)
> +       (FUSE_PASSTHROUGH_RW_OPS | FUSE_PASSTHROUGH_INODE_OPS |\
> +        FUSE_PASSTHROUGH_DIR_OPS)
>
>  static inline struct fuse_backing *fuse_inode_backing(struct fuse_inode *fi)
>  {
> @@ -1626,6 +1629,10 @@ ssize_t fuse_passthrough_getxattr(struct inode *inode, const char *name,
>                                   void *value, size_t size);
>  ssize_t fuse_passthrough_listxattr(struct dentry *entry, char *list,
>                                    size_t size);
> +struct dentry *fuse_passthrough_mkdir(struct mnt_idmap *idmap,
> +                                     struct inode *dir, struct dentry *entry,
> +                                     umode_t mode);
> +int fuse_passthrough_rmdir(struct inode *dir, struct dentry *entry);
>
>  #ifdef CONFIG_SYSCTL
>  extern int fuse_sysctl_register(void);
> diff --git a/fs/fuse/passthrough.c b/fs/fuse/passthrough.c
> index cee40e1c6e4a..acb06fbbd828 100644
> --- a/fs/fuse/passthrough.c
> +++ b/fs/fuse/passthrough.c
> @@ -7,6 +7,7 @@
>
>  #include "fuse_i.h"
>
> +#include "linux/namei.h"
>  #include <linux/file.h>
>  #include <linux/backing-file.h>
>  #include <linux/splice.h>
> @@ -497,3 +498,40 @@ ssize_t fuse_passthrough_listxattr(struct dentry *entry, char *list,
>         revert_creds(old_cred);
>         return res;
>  }
> +
> +struct dentry *fuse_passthrough_mkdir(struct mnt_idmap *idmap,
> +                                     struct inode *dir, struct dentry *entry,
> +                                     umode_t mode)
> +{
> +       struct fuse_backing *fb = fuse_inode_backing(get_fuse_inode(dir));
> +       struct dentry *backing_entry, *new_entry;
> +       const struct cred *old_cred;
> +
> +       old_cred = override_creds(fb->cred);
> +       backing_entry = lookup_one_unlocked(idmap, &entry->d_name,
> +               fb->file->f_path.dentry);
> +       new_entry = vfs_mkdir(idmap, fb->file->f_inode, backing_entry, mode);

vfs_mkdir() needs to be called with inode_lock_nested(dir, I_MUTEX_PARENT)
held and you need to call lookup_one() (not _unlocked) under the same lock
and handle all the error cases properly.

idmap use is incorrect. need to use mnt_idmap(fb->file->f_path.mnt)

Same problems with rmdir.

Please look at existing stacked fs code like overalyfs and ecryptfs for
reference when implementing the passthrough inode operations.

> +       d_drop(entry);

I don't think this is a viable way to implement passthorugh mkdir or
any other creation op.

I think we need to instantiate the fuse inode from the created real
inode attributes.

Is the new created directory auto passthrough or not?

Should the server be notified on the new instantiated inode?

And what about mkdir in a directory which is not passthrough
but server wants to return the created new entry with backing_id?

So many questions are left unanswered in this RFC.

Paul,

I think I have asked for a design overview every time that I looked
at a version of fuse directory ops passthrough patches.

There is so much complexity in the design that needs to be sorted
out before starting with implementation.
I really don't see what you envision as the end goal.
Please post a design overview RFC in the cover letter
if you send another revision of the patches.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2025-08-10 15:41 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-17 22:14 [PATCH v1 0/2] RFC: Extend fuse-passthrough to directories Paul Lawrence
2025-06-17 22:14 ` [PATCH v1 1/2] fuse: Add backing file option to lookup Paul Lawrence
2025-06-18 13:46   ` kernel test robot
2025-06-17 22:14 ` [PATCH v1 2/2] fuse: open/close backing file Paul Lawrence
2025-06-18 11:01 ` [PATCH v1 0/2] RFC: Extend fuse-passthrough to directories Amir Goldstein
2025-06-19 19:50   ` Paul Lawrence
2025-06-19 20:29     ` Amir Goldstein
2025-07-14 15:20       ` Amir Goldstein
2025-07-22 21:13         ` Paul Lawrence
2025-07-23 16:53           ` Amir Goldstein
2025-07-23 18:30             ` Paul Lawrence
2025-07-24 19:51               ` Amir Goldstein
2025-08-04 17:32                 ` [PATCH 0/2] RFC: Set backing file at lookup Paul Lawrence
2025-08-04 17:32                   ` [PATCH 1/2] fuse: Allow backing file to be set at lookup (WIP) Paul Lawrence
2025-08-10 14:21                     ` Amir Goldstein
2025-08-04 17:32                   ` [PATCH 2/2] fuse: Add passthrough for mkdir and rmdir (WIP) Paul Lawrence
2025-08-10 15:40                     ` Amir Goldstein
2025-08-10 13:25                   ` [PATCH 0/2] RFC: Set backing file at lookup Amir Goldstein
2025-08-10 14:32                     ` Amir Goldstein

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).