linux-kernel - Re: [RFC PATCH] fuse: add new function to invalidate cache for all inodes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <71aaa745-59a2-4627-9de2-c3359710d7b2@bsbernd.com>
Date: Wed, 15 Jan 2025 19:17:34 +0100
From: Bernd Schubert <bernd@...ernd.com>
To: Luis Henriques <luis@...lia.com>
Cc: Miklos Szeredi <miklos@...redi.hu>, linux-fsdevel@...r.kernel.org,
 linux-kernel@...r.kernel.org, Matt Harvey <mharvey@...ptrading.com>
Subject: Re: [RFC PATCH] fuse: add new function to invalidate cache for all
 inodes



On 1/15/25 19:07, Luis Henriques wrote:
> Hi Bernd,
> 
> On Wed, Jan 15 2025, Bernd Schubert wrote:
> 
>> On 1/15/25 17:32, 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 superblock dcache.
>>
>> Out of interest, what is the use case?
> 
> This is for a read-only filesystem.  However, the filesystem objects
> (files, directories, ...) may change dramatically in an atomic way, so
> that a totally different set of objects replaces the old one.
> 
> Obviously, this patch would help with the process of getting rid of the
> old generation of the filesystem.
> 
>>>
>>> Signed-off-by: Luis Henriques <luis@...lia.com>
>>> ---
>>> Just an additional note that this patch could eventually be simplified if
>>> Dave Chinner patch to iterate through the superblock inodes[1] is merged.
>>>
>>> [1] https://lore.kernel.org/r/20241002014017.3801899-3-david@fromorbit.com
>>>
>>>  fs/fuse/inode.c           | 53 +++++++++++++++++++++++++++++++++++++++
>>>  include/uapi/linux/fuse.h |  3 +++
>>>  2 files changed, 56 insertions(+)
>>>
>>> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
>>> index 3ce4f4e81d09..1fd9a5f303da 100644
>>> --- a/fs/fuse/inode.c
>>> +++ b/fs/fuse/inode.c
>>> @@ -546,6 +546,56 @@ struct inode *fuse_ilookup(struct fuse_conn *fc, u64 nodeid,
>>>  	return NULL;
>>>  }
>>>  
>>> +static int fuse_reverse_inval_all(struct fuse_conn *fc)
>>> +{
>>> +	struct fuse_mount *fm;
>>> +	struct super_block *sb;
>>> +	struct inode *inode, *old_inode = NULL;
>>> +	struct fuse_inode *fi;
>>> +
>>> +	inode = fuse_ilookup(fc, FUSE_ROOT_ID, NULL);
>>> +	if (!inode)
>>> +		return -ENOENT;
>>> +
>>> +	fm = get_fuse_mount(inode);
>>> +	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) {
>>
>> Maybe list_for_each_entry_safe() and then you can iput(inode) before the
>> next iteration?
> 
> I can rework this loop, but are you sure it's safe to use that?  (Genuine
> question!)
> 
> I could only find two places where list_for_each_entry_safe() is being
> used to walk through the sb inodes.  And they both use an auxiliary list
> that holds the inodes to be processed later.  All other places use the
> pattern I'm following here.
> 
> Or did I misunderstood your suggestion?


Actually my mistake, yeah you cannot use list_for_each_entry_safe() 
because you are giving up the list lock and the next entry, which
is already obtained by _safe might not be valid anymore.

Sorry for the noise!


Thanks,
Bernd