Re: [PATCH v6 2/2] fuse: add new function to invalidate cache for all inodes

[Luis Henriques <luis@igalia.com>]

On Wed, Feb 19 2025, Miklos Szeredi wrote:

> On Wed, 19 Feb 2025 at 12:23, Luis Henriques <luis@igalia.com> wrote:
>
>> +static int fuse_notify_update_epoch(struct fuse_conn *fc)
>> +{
>> +       struct fuse_mount *fm;
>> +       struct inode *inode;
>> +
>> +       inode = fuse_ilookup(fc, FUSE_ROOT_ID, &fm);
>> +       if (!inode) || !fm)
>> +               return -ENOENT;
>> +
>> +       iput(inode);
>> +       atomic_inc(&fc->epoch);
>> +       shrink_dcache_sb(fm->sb);
>
> This is just an optimization and could be racy, kicking out valid
> cache (harmlessly of course).  I'd leave it out of the first version.

OK, will do.

> There could be more than one fuse_mount instance.  Wondering if epoch
> should be per-fm not per-fc...

Good question.  Because the cache is shared among the several fuse_mount
instances the epoch may eventually affect all of them even if it's a
per-fm attribute.  But on the other hand, different mounts could focus on
a different set of filesystem subtrees so... yeah, I'll probably leave it
in fc for now while thinking about it some more.

>> @@ -204,6 +204,12 @@ static int fuse_dentry_revalidate(struct inode *dir, const struct qstr *name,
>>         int ret;
>>
>>         inode = d_inode_rcu(entry);
>> +       if (inode) {
>> +               fm = get_fuse_mount(inode);
>> +               if (entry->d_time < atomic_read(&fm->fc->epoch))
>> +                       goto invalid;
>> +       }
>
> Negative dentries need to be invalidated too.

Ack.

>> @@ -446,6 +452,12 @@ static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
>>                 goto out_err;
>>
>>         entry = newent ? newent : entry;
>> +       if (inode) {
>> +               struct fuse_mount *fm = get_fuse_mount(inode);
>> +               entry->d_time = atomic_read(&fm->fc->epoch);
>> +       } else {
>> +               entry->d_time = 0;
>> +       }
>
> Again, should do the same for positive and negative dentries.
>
> Need to read out fc->epoch before sending the request to the server,
> otherwise might get a stale dentry with an updated epoch.

Ah, good point.

> This also needs to be done in fuse_create_open(), create_new_entry()
> and fuse_direntplus_link().

Yeah I suspected there were a few other places where this would be
required.  I'll look closer into that.

Thanks a lot for your feedback, Miklos.  I'll work on this new approach,
so that I can send a real patch soon.

Cheers,
-- 
Luís