On 2/10/25 20:33, Joanne Koong wrote: > On 2/10/25 6:33 AM, 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 a new option that allows userspace to invalidate all the >> inodes with a single notification operation. In addition to >> invalidate all >> the inodes, it also shrinks the sb dcache. >> >> Signed-off-by: Luis Henriques <luis@xxxxxxxxxx> >> --- >> * 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@xxxxxxxxxxxxx >> >> fs/fuse/inode.c | 67 +++++++++++++++++++++++++++++++++++---- >> include/uapi/linux/fuse.h | 3 ++ >> 2 files changed, 63 insertions(+), 7 deletions(-) >> >> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c >> index e9db2cb8c150..45b9fbb54d42 100644 >> --- a/fs/fuse/inode.c >> +++ b/fs/fuse/inode.c >> @@ -547,25 +547,78 @@ struct inode *fuse_ilookup(struct fuse_conn *fc, >> u64 nodeid, >> return NULL; >> } >> +static void inval_single_inode(struct inode *inode, struct >> fuse_conn *fc) >> +{ >> + struct fuse_inode *fi; >> + >> + fi = get_fuse_inode(inode); >> + spin_lock(&fi->lock); >> + fi->attr_version = atomic64_inc_return(&fc->attr_version); >> + spin_unlock(&fi->lock); >> + fuse_invalidate_attr(inode); >> + forget_all_cached_acls(inode); >> +} >> + >> +static int fuse_reverse_inval_all(struct fuse_conn *fc) >> +{ >> + struct fuse_mount *fm; >> + struct super_block *sb; >> + struct inode *inode, *old_inode = NULL; >> + >> + inode = fuse_ilookup(fc, FUSE_ROOT_ID, NULL); >> + if (!inode) >> + return -ENOENT; >> + >> + fm = get_fuse_mount(inode); > > I think if you pass in &fm as the 3rd arg to fuse_ilookup(), it'll pass > back the fuse mount and we won't need get_fuse_mount(). > >> + iput(inode); >> + if (!fm) >> + return -ENOENT; >> + sb = fm->sb; >> + >> + spin_lock(&sb->s_inode_list_lock); >> + list_for_each_entry(inode, &sb->s_inodes, i_sb_list) { >> + spin_lock(&inode->i_lock); >> + if ((inode->i_state & (I_FREEING|I_WILL_FREE|I_NEW)) || >> + !atomic_read(&inode->i_count)) { > > Will inode->i_count ever be 0? AFAIU, inode->i_count tracks the inode > refcount, so if this is 0, doesn't this mean it wouldn't be on the sb- >>s_inodes list? > >> + spin_unlock(&inode->i_lock); >> + continue; >> + } >> + >> + __iget(inode); >> + spin_unlock(&inode->i_lock); >> + spin_unlock(&sb->s_inode_list_lock); > > Maybe worth adding a comment here since there can be inodes added after > the s_inode_list_lock is dropped and before it's acquired again that > when inodes get added to the head of sb->s_inodes, it's always for I_NEW > inodes. > >> + iput(old_inode); > > Maybe a dumb question but why is old_inode needed? Why can't iput()just > be called right after inval_single_inode()? I had wondered the same in v1. Issue is that there is a list iteration that releases the locks - if the put would be done immediately it could not continue on "old_inode" as it might not exist anymore. > >> + >> + inval_single_inode(inode, fc); >> + >> + old_inode = inode; >> + cond_resched(); > > Could you explain why a cond_resched() is needed here? Give other threads a chance to work? The list might be huge? Thanks, Bernd
