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

[Luis Henriques <luis@igalia.com>]

On Thu, Jul 02 2026, Amir Goldstein wrote:

> On Wed, Jul 1, 2026 at 2:24 PM Luis Henriques <luis@igalia.com> wrote:
>>
>> 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.
>
> Local filesystems do, remote filesystems not always.

True.  Even ceph, which is the remote filesystem I'm most familiar with,
caches ACLs when setting them only if the client has the right
capabilities for doing so.

>> 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.
>
> Your arguments make sense to me.
> Only it appears so simple so it feels like I am missing something.
> Why wasn't this implemented like that in the first place?

Yeah, maybe your right, maybe I'm missing something as well and there is a
good reason for not doing it.

I was going to CC Christian, but I see you did that already :-)

Maybe he remembers the reasons behind this while working on commit
facd61053cff ("fuse: fixes after adapting to new posix acl api").

Cheers,
-- 
Luís