On Thu, Oct 31, 2024 at 08:14:59AM -1000, Linus Torvalds wrote: > On Wed, 30 Oct 2024 at 20:42, Linus Torvalds > <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > > > > I was kind of hoping that such cases would use 'cache_no_acl()' which > > makes that inode->i_acl be NULL. Wouldn't that be the right model > > anyway for !IS_POSIXACL()? > > Alternatively, just initialize it to NULL in inode_init_always_gfp(), eg like > > - inode->i_acl = inode->i_default_acl = ACL_NOT_CACHED; > + inode->i_acl = inode->i_default_acl = > + (sb->s_flags & SB_POSIXACL) ? ACL_NOT_CACHED : NULL; IIRC, the reason we do not do that was the possibility of mount -o remount,acl Not that it makes much sense, but it's currently supported and POSIX ACLs don't make much sense to start with... Anyway, patch looks sane; I still think that adding || !IS_POSIXACL(inode) wouldn't hurt (yes, it's a dereference of ->i_sb in case when ->i_acl is ACL_NOT_CACHED, but we are going to dereference it shortly after in case we don't take the fast path. OTOH, that probably matters only for fairly specific loads - massive accesses to procfs and sysfs, mostly.
