On Wed, 30 Oct 2024 at 20:05, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, Oct 30, 2024 at 06:16:22PM -1000, Linus Torvalds wrote:
>
> > +static inline bool no_acl_inode(struct inode *inode)
> > +{
> > +#ifdef CONFIG_FS_POSIX_ACL
> > + return likely(!READ_ONCE(inode->i_acl));
> > +#else
> > + return true;
> > +#endif
> > +}
>
> Hmm... Shouldn't there be || !IS_POSIXACL(inode) in there?
I was going for "intentionally minimalistic". IOW, this was meant to
be an optimization for a presumed common case, but fall back to the
generic code quickly and simply.
Put another way: is the !IS_POSIXACL() actually a common situation
worth optimizing for?
Do real people actually use "noacl"? My gut feel is that it was one of
those mistakes that some random odd case is using, but not something
worth really optimizing for.
But maybe some common situation ends up using it even without "noacl"
- like /proc, perhaps?
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()?
Anyway, this is all obviously a "matter of tuning the optimization".
And I don't actually know if it's even worth doing in the first place.
