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

On Wed, Jul 1, 2026 at 2:24 PM Luis Henriques <[email protected]> wrote:
>
> On Wed, Jul 01 2026, Amir Goldstein wrote:
>
> > On Wed, Jul 1, 2026 at 1:05 PM Luis Henriques <[email protected]> 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 <[email protected]>
> >> ---
> >>  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.

>
> 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?

Thanks,
Amir.