[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAOQ4uxjL6=__pg5RKjv+39M5BWKCpENwiNr5ZwCXeSXHHE=m1Q@mail.gmail.com>
Date: Sun, 10 Aug 2025 17:40:58 +0200
From: Amir Goldstein <amir73il@...il.com>
To: Paul Lawrence <paullawrence@...gle.com>
Cc: bernd.schubert@...tmail.fm, linux-fsdevel@...r.kernel.org,
linux-kernel@...r.kernel.org, miklos@...redi.hu
Subject: Re: [PATCH 2/2] fuse: Add passthrough for mkdir and rmdir (WIP)
On Mon, Aug 4, 2025 at 7:32 PM Paul Lawrence <paullawrence@...gle.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@...gle.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.
Powered by blists - more mailing lists