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 <[email protected]>
> 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
diff --git a/tests/ls/getxattr-speedup b/tests/ls/getxattr-speedup
index f3dc5bb..9127a28 100755
--- a/tests/ls/getxattr-speedup
+++ b/tests/ls/getxattr-speedup
@@ -21,7 +21,8 @@
. "${srcdir=.}/init.sh"; path_prepend_ ../src
print_ver_ ls
-fd=33
+speeduptext="__SPEEDUP__"
+speeduptextlen=$( echo "$speeduptext" | wc -c ) # also count newline.
# Replace each getxattr and lgetxattr call with a call to these stubs.
# Each writes a single line to the specified FD so that we can later
@@ -32,29 +33,27 @@ cat > k.c <<'EOF' || framework_failure_
static void
write_1 (void)
{
- int fd = @FD@;
- write (fd, "x\n", 2);
+ write (1, SPEEDUPTEXT "\n", SPEEDUPTEXTLEN);
}
ssize_t getxattr (const char *path, const char *name, void *value, size_t size)
{ write_1 (); errno = ENOTSUP; return -1; }
ssize_t lgetxattr(const char *path, const char *name, void *value, size_t size)
{ write_1 (); errno = ENOTSUP; return -1; }
EOF
-sed "s/@FD@/$fd/" k.c > k || framework_failure_
-mv k k.c || framework_failure_
# Then compile/link it:
-$CC -fPIC -O2 -c k.c || framework_failure_ 'failed to compile with -fPIC'
+$CC -DSPEEDUPTEXT="\"$speeduptext\"" -DSPEEDUPTEXTLEN="$speeduptextlen" \
+ -fPIC -O2 -c k.c || framework_failure_ 'failed to compile with -fPIC'
ld -G k.o -o k.so || framework_failure_ 'failed to invoke ld -G ...'
# create a few files
seq 100 | xargs touch || framework_failure_
# Finally, run the test, redirecting
-eval "LD_PRELOAD=$PWD/k.so ls --color=always -l . $fd>x" || fail=1
+eval "LD_PRELOAD=$PWD/k.so ls --color=always -l . >x" || fail=1
# Ensure that there were no more than 3 *getxattr calls.
-n_calls=$(wc -l < x)
+n_calls=$( grep "^$speeduptext\$" x | wc -l)
test "$n_calls" -le 3 || fail=1
Exit $fail
