Message-ID: <87msdwrh72.fsf@igalia.com>
Date: Fri, 07 Mar 2025 15:30:57 +0000
From: Luis Henriques <luis@...lia.com>
To: Miklos Szeredi <miklos@...redi.hu>
Cc: Bernd Schubert <bschubert@....com>,  Dave Chinner <david@...morbit.com>,
  Matt Harvey <mharvey@...ptrading.com>,  linux-fsdevel@...r.kernel.org,
  linux-kernel@...r.kernel.org
Subject: Re: [PATCH v8] fuse: add more control over cache invalidation
 behaviour

Hi Miklos,

On Wed, Feb 26 2025, Luis Henriques wrote:

> Currently userspace is able to notify the kernel to invalidate the cache
> for an inode.  This means that, if all the inodes in a filesystem need to
> be invalidated, then userspace needs to iterate through all of them and do
> this kernel notification separately.
>
> This patch adds the concept of 'epoch': each fuse connection will have the
> current epoch initialized and every new dentry will have it's d_time set to
> the current epoch value.  A new operation will then allow userspace to
> increment the epoch value.  Every time a dentry is d_revalidate()'ed, it's
> epoch is compared with the current connection epoch and invalidated if it's
> value is different.

Any further feedback on this patch, or is it already OK for being merged?
And what about the extra call to shrink_dcache_sb(), do you think that
would that be acceptable?  Maybe that could be conditional, by for example
setting a flag.

Cheers,
-- 
Luís

> Signed-off-by: Luis Henriques <luis@...lia.com>
> ---
> Nothing huge since v7, I just realized I forgot to bump the version number
> while cleaning up a patch to prepare a libfuse MR.
>
> * Changes since v7
> - Bump FUSE interface minor number
>
> * Changes since v6
> - Major patch re-write, following a different approach suggested by Miklos
>   and Dave.
>
> * Changes since v5
> - Added missing iput() in function fuse_reverse_inval_all()
>
> * Changes since v4
> - Replaced superblock inodes iteration by a single call to
>   invalidate_inodes().  Also do the shrink_dcache_sb() first. (Dave Chinner)
>
> * Changes since v3
> - Added comments to clarify semantic changes in fuse_reverse_inval_inode()
>   when called with FUSE_INVAL_ALL_INODES (suggested by Bernd).
> - Added comments to inodes iteration loop to clarify __iget/iput usage
>   (suggested by Joanne)
> - Dropped get_fuse_mount() call -- fuse_mount can be obtained from
>   fuse_ilookup() directly (suggested by Joanne)
>
> (Also dropped the RFC from the subject.)
>
> * Changes since v2
> - Use the new helper from fuse_reverse_inval_inode(), as suggested by Bernd.
> - Also updated patch description as per checkpatch.pl suggestion.
>
> * Changes since v1
> As suggested by Bernd, this patch v2 simply adds an helper function that
> will make it easier to replace most of it's code by a call to function
> super_iter_inodes() when Dave Chinner's patch[1] eventually gets merged.
>
> [1] https://lore.kernel.org/r/20241002014017.3801899-3-david@fromorbit.com
>
>  fs/fuse/dev.c             | 16 ++++++++++++++++
>  fs/fuse/dir.c             | 22 +++++++++++++++++++---
>  fs/fuse/fuse_i.h          |  3 +++
>  fs/fuse/inode.c           |  1 +
>  fs/fuse/readdir.c         |  3 +++
>  include/uapi/linux/fuse.h |  6 +++++-
>  6 files changed, 47 insertions(+), 4 deletions(-)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 5b5f789b37eb..e31a55ac3887 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -1902,6 +1902,19 @@ static int fuse_notify_resend(struct fuse_conn *fc)
>  	return 0;
>  }
>  
> +/*
> + * Increments the fuse connection epoch.  This will result of dentries from
> + * previous epochs to be invalidated.
> + *
> + * XXX optimization: add call to shrink_dcache_sb()?
> + */
> +static int fuse_notify_inc_epoch(struct fuse_conn *fc)
> +{
> +	atomic_inc(&fc->epoch);
> +
> +	return 0;
> +}
> +
>  static int fuse_notify(struct fuse_conn *fc, enum fuse_notify_code code,
>  		       unsigned int size, struct fuse_copy_state *cs)
>  {
> @@ -1930,6 +1943,9 @@ static int fuse_notify(struct fuse_conn *fc, enum fuse_notify_code code,
>  	case FUSE_NOTIFY_RESEND:
>  		return fuse_notify_resend(fc);
>  
> +	case FUSE_NOTIFY_INC_EPOCH:
> +		return fuse_notify_inc_epoch(fc);
> +
>  	default:
>  		fuse_copy_finish(cs);
>  		return -EINVAL;
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 198862b086ff..5291deeb191f 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -200,9 +200,14 @@ static int fuse_dentry_revalidate(struct inode *dir, const struct qstr *name,
>  {
>  	struct inode *inode;
>  	struct fuse_mount *fm;
> +	struct fuse_conn *fc;
>  	struct fuse_inode *fi;
>  	int ret;
>  
> +	fc = get_fuse_conn_super(dir->i_sb);
> +	if (entry->d_time < atomic_read(&fc->epoch))
> +		goto invalid;
> +
>  	inode = d_inode_rcu(entry);
>  	if (inode && fuse_is_bad(inode))
>  		goto invalid;
> @@ -415,16 +420,20 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
>  static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
>  				  unsigned int flags)
>  {
> -	int err;
>  	struct fuse_entry_out outarg;
> +	struct fuse_conn *fc;
>  	struct inode *inode;
>  	struct dentry *newent;
> +	int err, epoch;
>  	bool outarg_valid = true;
>  	bool locked;
>  
>  	if (fuse_is_bad(dir))
>  		return ERR_PTR(-EIO);
>  
> +	fc = get_fuse_conn_super(dir->i_sb);
> +	epoch = atomic_read(&fc->epoch);
> +
>  	locked = fuse_lock_inode(dir);
>  	err = fuse_lookup_name(dir->i_sb, get_node_id(dir), &entry->d_name,
>  			       &outarg, &inode);
> @@ -446,6 +455,7 @@ static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
>  		goto out_err;
>  
>  	entry = newent ? newent : entry;
> +	entry->d_time = epoch;
>  	if (outarg_valid)
>  		fuse_change_entry_timeout(entry, &outarg);
>  	else
> @@ -619,7 +629,6 @@ static int fuse_create_open(struct mnt_idmap *idmap, struct inode *dir,
>  			    struct dentry *entry, struct file *file,
>  			    unsigned int flags, umode_t mode, u32 opcode)
>  {
> -	int err;
>  	struct inode *inode;
>  	struct fuse_mount *fm = get_fuse_mount(dir);
>  	FUSE_ARGS(args);
> @@ -629,11 +638,13 @@ static int fuse_create_open(struct mnt_idmap *idmap, struct inode *dir,
>  	struct fuse_entry_out outentry;
>  	struct fuse_inode *fi;
>  	struct fuse_file *ff;
> +	int epoch, err;
>  	bool trunc = flags & O_TRUNC;
>  
>  	/* Userspace expects S_IFREG in create mode */
>  	BUG_ON((mode & S_IFMT) != S_IFREG);
>  
> +	epoch = atomic_read(&fm->fc->epoch);
>  	forget = fuse_alloc_forget();
>  	err = -ENOMEM;
>  	if (!forget)
> @@ -702,6 +713,7 @@ static int fuse_create_open(struct mnt_idmap *idmap, struct inode *dir,
>  	}
>  	kfree(forget);
>  	d_instantiate(entry, inode);
> +	entry->d_time = epoch;
>  	fuse_change_entry_timeout(entry, &outentry);
>  	fuse_dir_changed(dir);
>  	err = generic_file_open(inode, file);
> @@ -788,12 +800,14 @@ static int create_new_entry(struct mnt_idmap *idmap, struct fuse_mount *fm,
>  	struct fuse_entry_out outarg;
>  	struct inode *inode;
>  	struct dentry *d;
> -	int err;
>  	struct fuse_forget_link *forget;
> +	int epoch, err;
>  
>  	if (fuse_is_bad(dir))
>  		return -EIO;
>  
> +	epoch = atomic_read(&fm->fc->epoch);
> +
>  	forget = fuse_alloc_forget();
>  	if (!forget)
>  		return -ENOMEM;
> @@ -836,9 +850,11 @@ static int create_new_entry(struct mnt_idmap *idmap, struct fuse_mount *fm,
>  		return PTR_ERR(d);
>  
>  	if (d) {
> +		d->d_time = epoch;
>  		fuse_change_entry_timeout(d, &outarg);
>  		dput(d);
>  	} else {
> +		entry->d_time = epoch;
>  		fuse_change_entry_timeout(entry, &outarg);
>  	}
>  	fuse_dir_changed(dir);
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index fee96fe7887b..06eecc125f89 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -611,6 +611,9 @@ struct fuse_conn {
>  	/** Number of fuse_dev's */
>  	atomic_t dev_count;
>  
> +	/** Current epoch for up-to-date dentries */
> +	atomic_t epoch;
> +
>  	struct rcu_head rcu;
>  
>  	/** The user id for this mount */
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index e9db2cb8c150..5d2d29fad658 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -959,6 +959,7 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
>  	init_rwsem(&fc->killsb);
>  	refcount_set(&fc->count, 1);
>  	atomic_set(&fc->dev_count, 1);
> +	atomic_set(&fc->epoch, 1);
>  	init_waitqueue_head(&fc->blocked_waitq);
>  	fuse_iqueue_init(&fc->iq, fiq_ops, fiq_priv);
>  	INIT_LIST_HEAD(&fc->bg_queue);
> diff --git a/fs/fuse/readdir.c b/fs/fuse/readdir.c
> index 17ce9636a2b1..46b7146f2c0d 100644
> --- a/fs/fuse/readdir.c
> +++ b/fs/fuse/readdir.c
> @@ -161,6 +161,7 @@ static int fuse_direntplus_link(struct file *file,
>  	struct fuse_conn *fc;
>  	struct inode *inode;
>  	DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
> +	int epoch;
>  
>  	if (!o->nodeid) {
>  		/*
> @@ -190,6 +191,7 @@ static int fuse_direntplus_link(struct file *file,
>  		return -EIO;
>  
>  	fc = get_fuse_conn(dir);
> +	epoch = atomic_read(&fc->epoch);
>  
>  	name.hash = full_name_hash(parent, name.name, name.len);
>  	dentry = d_lookup(parent, &name);
> @@ -256,6 +258,7 @@ static int fuse_direntplus_link(struct file *file,
>  	}
>  	if (fc->readdirplus_auto)
>  		set_bit(FUSE_I_INIT_RDPLUS, &get_fuse_inode(inode)->state);
> +	dentry->d_time = epoch;
>  	fuse_change_entry_timeout(dentry, o);
>  
>  	dput(dentry);
> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> index 5e0eb41d967e..15991728a894 100644
> --- a/include/uapi/linux/fuse.h
> +++ b/include/uapi/linux/fuse.h
> @@ -229,6 +229,9 @@
>   *    - FUSE_URING_IN_OUT_HEADER_SZ
>   *    - FUSE_URING_OP_IN_OUT_SZ
>   *    - enum fuse_uring_cmd
> + *
> + *  7.43
> + *  - add FUSE_NOTIFY_INC_EPOCH
>   */
>  
>  #ifndef _LINUX_FUSE_H
> @@ -264,7 +267,7 @@
>  #define FUSE_KERNEL_VERSION 7
>  
>  /** Minor version number of this interface */
> -#define FUSE_KERNEL_MINOR_VERSION 42
> +#define FUSE_KERNEL_MINOR_VERSION 43
>  
>  /** The node ID of the root inode */
>  #define FUSE_ROOT_ID 1
> @@ -666,6 +669,7 @@ enum fuse_notify_code {
>  	FUSE_NOTIFY_RETRIEVE = 5,
>  	FUSE_NOTIFY_DELETE = 6,
>  	FUSE_NOTIFY_RESEND = 7,
> +	FUSE_NOTIFY_INC_EPOCH = 8,
>  	FUSE_NOTIFY_CODE_MAX,
>  };
>  
>

Powered by blists - more mailing lists