Re: [PATCH] fuse: cache POSIX ACLs when setting them

[Luis Henriques <luis@igalia.com>]

On Wed, Jul 01 2026, Amir Goldstein wrote:

> On Wed, Jul 1, 2026 at 1:05 PM Luis Henriques <luis@igalia.com> wrote:
>>
>> When setting an ACL in an inode we can immediately add it to the cache.
>> This is a small optimisation, as currently an ACL is only added to the
>> cache when reading it again, which involves an extra GETXATTR hop into
>> user-space.
>>
>> Signed-off-by: Luis Henriques <luis@igalia.com>
>> ---
>>  fs/fuse/acl.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/fs/fuse/acl.c b/fs/fuse/acl.c
>> index 31fb50e16aed..c2584bb75ec7 100644
>> --- a/fs/fuse/acl.c
>> +++ b/fs/fuse/acl.c
>> @@ -155,6 +155,8 @@ int fuse_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
>>                  */
>>                 forget_all_cached_acls(inode);
>>                 fuse_invalidate_attr(inode);
>> +               if (!ret)
>> +                       set_cached_acl(inode, type, acl);
>
> This is undoing what forget_all_cached_acls() explicitly tries to do.
> I think the reason is that kernel code cannot assume the server did
> not modify ACL before storing them, not sure, but NFS does the same thing
> with nfs_zap_acl_cache().

In my understanding, forget_all_cached_acls() is cleaning any previously
cached ACLs (even if the SETXATTR failed!).  So it made sense to me to
cache the new ACL just like other filesystems seem to be doing.

On the other hand, having a server modifying the ACL without notifying the
kernel didn't sound right to me, specially because it has explicitly set
the FUSE_POSIX_ACL flag -- otherwise this code wouldn't be executed.  And
that's why I assumed it would be acceptable to have this optimisation:
because user-space asked the VFS to do permission checking *and* caching
ACLs.

I did actually took a look into the NFS code, but it seems to be doing it's
own ACL caching. But yeah, I can't claim to have fully understood what
(and why) NFS is really doing :-)

Cheers,
-- 
Luís