Re: [PATCH] file-has-acl: revert unintended change in behavior of ls -L

[Kamil Dudka]

On Monday 03 October 2011 20:13:53 Bruno Haible wrote:
> Kamil Dudka wrote:
> > >   a) A non-symlink, non-directory. Here acl_extended_file_nofollow and
> > >      acl_extended_file are equivalent.
> >
> > If I understand this, you expect non-directories cannot be mount points,
> > thus the call cannot trigger the mount, right?
>
> That's what I was assuming, yes. My kernel knowledge is rusty, though. Is
> it now possible to use regular files as mount points?

Hopefully not, asking just to correctly understand the logic behind.

> > > So I'm asking to change
> > >
> > >     ret = acl_extended_file_wrap (name);
> > >
> > > to
> > >
> > >     if (S_ISDIR (sb->st_mode))
> > >       ret = acl_extended_file_wrap (name);
> > >     else
> > >       ret = acl_extended_file (name);
> >
> > Is optimization the only purpose of this change?  I would then prefer to
> > get it working first and optimize it later on.
>
> I'm trying to explain to you that
>   - your patch is introducing a performance regression in case a), which is
>     the most frequent case,

That's true in case you are on Linux, compile against a new enough version
of libacl and have no risk of unintentional mount points to be trigerred.  
Otherwise the cost of unnecessary mount points would be higher than cost
of the additional lstat syscalls.

>   - your patch does not completely work yet: case f).

I have asked autofs guys if they consider this to be an issue:

https://bugzilla.redhat.com/720325#c23

> > The last patch does not break anything that worked before, does it?
>
> It degrades the performance of "ls -l" in the most frequent case.
>
> > Case f) has never worked, nobody complained.  Partial fix is better than
> > no fix.
>
> I had the impression that you wanted to fix "ls -l", not fix some bugs and
> leave others open.
>
> And I wanted to help coreutils by stating which are the 9 cases which IMO
> should have a test case, each.

My only intention is to fix real problems we are facing without introducing 
new problems.  Nothing more.

Kamil