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

[Bernhard Voelker]

On 02/16/2012 08:39 PM, Jim Meyering wrote:
> Sven Breuner wrote:
> ...
>> (Tested with similar results on cifs, ext4, fhgfs.)
> 
> Thanks for the testing.
> 
> Here are two patches to eliminate all unnecessary getxattr calls
> (you've already seen the first) and one more to add a test script.
> Sven, it should be interesting to see your speed-up numbers ;-)
> 
> The test is interesting in that it uses LD_PRELOAD, and is expected
> to be skipped (or fail) on systems that don't support that.
> Which makes me realize that I don't yet have code to make the test
> skip when $CC and ld succeed but where LD_PRELOAD is not supported.
> 
> At first I wanted to write a root-only test that would
> create a file system of some type for which *getxattr would always
> fail, but with linux-kernel-based systems, that may be a challenge,
> since all non-network FS types seem to have some amount of support, now.
> I am avoiding tests involving network file systems: that would be
> a little too invasive for my taste.
> 
> But then Daniel Berrangé suggested to use LD_PRELOAD.  That's better

That's clever.

> because the test does not require root privileges (most mount-related
> tests would), but comes with a few challenges.  I'll skip the
> portability ones for now.  The part that I found surprising is that
> from the .so wrapper calls, while I could indeed count the number of
> intercepted *getxattr calls, I could *not* access that static count
> from an __attribute__((destructor)) function that would simply print
> that static variable.  The printed number was always zero, because, as I
> found, when the destructor is invoked, the static variable has a different
> address than the one seen by the intercepted calls.  My current method
> is to write a single line to file descriptor 33 from each intercepted call,
> redirect that FD, and to count the lines of the resulting file.
> Definitely worthy of the "FIXME" comment in that script.

Well, the test could open "x" and write directly into it, but that
doesn't make a big difference (only the test would be slightly slower).

Alternatively, it could write a certain line to stdout or stderr,
and for comparison, this pattern would have to be counted.
There's a raw patch attached to demonstrate it.


> From ac6dc056f8a2ed00b86c63af50e949e1ac30dae0 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <address@hidden>
> Date: Sat, 11 Feb 2012 12:36:57 +0100
> Subject: [PATCH 1/3] ls: optimize for when getfilecon would often fail (~30%
>  speed-up)

> +static dev_t selinux_challenged_device;

Having worked with different compilers, I would rather like to see such
variables to be initialized.

Likewise in:


> Subject: [PATCH 2/3] ls: also cache ACL- and CAP-querying syscall failures

> +static int
> +file_has_acl_cache (char const *file, struct fileinfo *f)
> +{
> +  /* st_dev of the most recently processed device for which we've
> +     found that file_has_acl fails indicating lack of support.  */
> +  static dev_t unsupported_device;

and here:

> +static bool
> +has_capability_cache (char const *file, struct fileinfo *f)
> +{
> +  /* st_dev of the most recently processed device for which we've
> +     found that has_capability fails indicating lack of support.  */
> +  static dev_t unsupported_device;


Have a nice day,
Berny

getxattr2.patch
Description: Text Data