Re: "ls -l": Avoid unnecessary getxattr() overhead

[Pádraig Brady]

On 02/12/2012 01:01 AM, Sven Breuner wrote:
> Pádraig Brady wrote on 02/10/2012 02:03 AM:
>> On 02/09/2012 09:17 PM, Sven Breuner wrote:
>>> Current git version patched...
>>>
>>> $ time ~/tmp/ls-8.15.34-31eee_patched -l>/dev/null
>>>
>>> real    0m21.254s
>>> user    0m0.565s
>>> sys     0m2.496s
>>>
>>> $ strace -c ~/tmp/ls-8.15.34-31eee_patched -l>/dev/null
>>> % time     seconds  usecs/call     calls    errors syscall
>>> ------ ----------- ----------- --------- --------- ----------------
>>>   45.50    0.313386           5     61441           lstat
>>>   43.63    0.300524           5     61441     61441 getxattr
>>>   10.60    0.072986         973        75           getdents
>>
>> So you're still getting all those errors.
>> Is that from file_has_acl() or cap_get_file()?
>> Oh it must be from file_has_acl() as you've
>> not specified --color. What is the errno in that case?
>>
>> Note you can avoid the cap_get_file() calls at least,
>> that would be enabled with --color, by unsetting capability
>> coloring, which you can test by running the following
>> before your ls command.
>>
>> eval $(dircolors | sed 's/ca[^:]*:/ca=:/')
> 
> Thanks, Pádraig - you're right. Indeed, I didn't consider that coloring was 
> disabled during my tests and that turning it on might lead to more fs calls.
> 
> So here's the strace of a test with colors enabled:
> (ls with Jim's selinux optimization patch applied)
> 
> $ strace -c ~/tmp/ls-8.15.34-31eee_patched -l --color=always >/dev/null
> % time     seconds  usecs/call     calls    errors syscall
> ------ ----------- ----------- --------- --------- ----------------
>  56.63    0.139981           1    122882    122882 getxattr
>  38.43    0.094985         176       540           getdents
>   3.03    0.007478           0     61441           lstat
>   1.35    0.003341           0     61441     61441 capget
> [...]
> 
> => Two getxattr calls per file in the directory, both returning errors.
> 
> Using your suggestion regarding cap_get_file:
> 
> $ eval $(dircolors | sed 's/ca[^:]*:/ca=:/')
> $ strace -c  ~/tmp/ls-8.15.34-31eee_patched -l  --color=always >/dev/null
> % time     seconds  usecs/call     calls    errors syscall
> ------ ----------- ----------- --------- --------- ----------------
>  43.96    0.173020           3     61441           lstat
>  37.75    0.148590           2     61441     61441 getxattr
>  18.04    0.070994         131       540           getdents
> [...]
> 
> => Back to one getxattr call per file in the directory.
> 
> Even though it's nice to have the option to avoid the cap_get_file and 
> associated getxattr call, I guess it would be best to optimize the default 
> case. So that means there are two getxattr calls per directory entry left 
> that could be optimized.
> 
> Here's an strace excerpt with colors enabled and default color settings on 
> rhel 6.2:
> $ strace ~/tmp/ls-8.15.34-31eee_patched -l --color=always
> [...]
> lstat("1", {st_mode=S_IFREG|0644, st_size=0, ...}) = 0
> capget(0x20080522, 0, NULL)             = -1 EFAULT (Bad address)
> getxattr("1", "security.capability", 0x7fffffffda30, 20) = -1 EOPNOTSUPP 
> (Operation not supported)
> getxattr("1", "system.posix_acl_access", 0x0, 0) = -1 EOPNOTSUPP (Operation 
> not supported)
> 
> lstat("dir1", {st_mode=S_IFDIR|0755, st_size=0, ...}) = 0
> getxattr("dir1", "system.posix_acl_access", 0x0, 0) = -1 EOPNOTSUPP 
> (Operation not supported)
> 
> => Two getxattr calls per file in the directory, one getxattr per subdirectory

Thanks for the nice summary.
I think it would be worth applying the same avoidance
technique to file_has_acl() and cap_get_file() so,
given their non ubiquity and high overhead.

cheers,
Pádraig.