Bug 662011 - ls -l command make two getxattr() when both call files with ENODATA
Summary: ls -l command make two getxattr() when both call files with ENODATA
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: acl
Version: rawhide
Hardware: All
OS: Linux
low
low
Target Milestone: ---
Assignee: Steve Dickson
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-12-10 10:38 UTC by Lakshmipathi
Modified: 2010-12-10 12:13 UTC (History)
5 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2010-12-10 12:13:10 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Lakshmipathi 2010-12-10 10:38:16 UTC
Description of problem:
ls -l command make two getxattr() 

$touch a
$ strace -e trace="getxattr" ls -l  a
getxattr("a", "system.posix_acl_access", 0x0, 0) = -1 ENODATA (No data available)
getxattr("a", "system.posix_acl_default", 0x0, 0) = -1 ENODATA (No data available)


Version-Release number of selected component (if applicable):
coreutils-7.6-11.fc12.x86_64

How reproducible:
$touch filename.txt
$ strace -e trace="getxattr" ls -l  filename.txt

Actual results:
performs two calls 
getxattr("a", "system.posix_acl_access", 0x0, 0) = -1 ENODATA (No data available)
getxattr("a", "system.posix_acl_default", 0x0, 0) = -1 ENODATA (No data available)


Expected results:
I think it would be better, if call first fails - we need  ignore the second getxattr() call.

getxattr("a", "system.posix_acl_access", 0x0, 0) = -1 ENODATA (No data available)

This would increase the performance on directories which has huge no.of files.



Additional info:

Comment 1 Kamil Dudka 2010-12-10 10:52:29 UTC
The first ENODATA does not imply the second one.  Those need to be checked separately.  Try the following example:

$ mkdir d
$ setfacl -d -m user:root:rwx d
$ strace -e trace=getxattr ls -ld d
getxattr("d", "system.posix_acl_access", 0x0, 0) = -1 ENODATA (No data available)
getxattr("d", "system.posix_acl_default", 0x0, 0) = 44

Comment 2 Lakshmipathi 2010-12-10 11:14:47 UTC
In your example you have created a directory ,what about providing check to make sure if its a file the ignore the second call.

I have ran tests with 100000 files and found that getxattr alone takes around 20% of ls time. If second call is ignored i believe it would increase ls performance by 10%.

=================
% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 45.75    0.047209         390       121           getdents
 20.89    0.021563           0    200000    200000 getxattr
=================

Comment 3 Kamil Dudka 2010-12-10 11:24:56 UTC
That is, however, not easily doable.  coreutils uses file_has_acl() from gnulib.  On Linux it results to call of acl_extended_file() from libacl.  The function operates on file name and has no stat info available.  In order to skip the second call of getxattr(), it would be necessary to insert additional call of stat().  That is totally pointless, since the second call of getxattr() on the same file has zero cost.  It just serves the data from cache.

Your statistics says nothing about the proposed change.  You need to run it through strace -r and check the delay of even calls of getxattr() only.  Alternatively you can force through LD_PRELOAD your own implementation of acl_extended_file() with the stat() inserted and see if there is some measurable performance improvement.

Comment 4 Ondrej Vasik 2010-12-10 11:32:20 UTC
1) F-12 is EOLed ... -> moving to Rawhide
2) these two getxattr calls are not from coreutils, but from libacl
(acl_extended_file() function) - and it's parameter is just name of the file ->
moving to acl. So making the second getxattr only on directories, it would
probably mean to call another stat call - which would mean anothere decrease of
the performance (probably as significant as the second getxattr call) -
reassigning to acl, and ... Kamil will probably close it again NOTABUG.

Comment 5 Lakshmipathi 2010-12-10 12:13:10 UTC
okay.When i get time ,i'll look into libacl code to see anything is possible to handle this case..thanks for comments.


Note You need to log in before you can comment on or make changes to this bug.