ls: Fix excessive ls -al syscalls for ACL metadata #8660

kimono-koans · 2025-09-18T20:15:47Z

Reduces syscalls by searching for the specific ACL xattr key instead of requesting a list of all keys and values.

➜  ~ strace -c ./program/coreutils/target/release/ls -al
...
% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ------------------
 20.88    0.001559          11       130           listxattr
 16.02    0.001196          35        34           getdents64
 14.43    0.001077          13        79         9 statx
 10.89    0.000813         813         1           execve
  6.47    0.000483          17        28         1 openat
  4.65    0.000347          11        31           close
  4.21    0.000314          24        13           mmap
  4.07    0.000304          23        13           read
  3.99    0.000298          11        27           fstat
  3.03    0.000226          16        14         6 readlink
  1.81    0.000135          33         4         4 connect
  1.62    0.000121          30         4           socket
  1.25    0.000093          93         1           write
  1.11    0.000083          13         6           newfstatat
  0.90    0.000067          11         6           brk
  0.64    0.000048           6         7           rt_sigaction
  0.60    0.000045           9         5           mprotect
  0.58    0.000043           8         5           lseek
  0.58    0.000043           8         5           ioctl
  0.50    0.000037          18         2           munmap
  0.46    0.000034          17         2           pread64
  0.31    0.000023           7         3           sigaltstack
  0.25    0.000019          19         1         1 access
  0.15    0.000011           5         2           getrandom
  0.13    0.000010          10         1           arch_prctl
  0.11    0.000008           8         1           sched_getaffinity
  0.11    0.000008           8         1           set_tid_address
  0.11    0.000008           8         1           set_robust_list
  0.11    0.000008           8         1           rseq
  0.07    0.000005           2         2           prlimit64
  0.00    0.000000           0         1           poll
------ ----------- ----------- --------- --------- ------------------
100.00    0.007466          17       431        21 total

➜  ~ strace -c ./program/coreutils/target/release/ls -al
...
% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ------------------
 19.08    0.000962          28        34           getdents64
 17.12    0.000863          10        79         9 statx
 16.20    0.000817         817         1           execve
  8.67    0.000437          15        28         1 openat
  6.41    0.000323          24        13           mmap
  5.28    0.000266           4        66        66 getxattr
  4.20    0.000212           7        27           fstat
  4.17    0.000210          16        13           read
  3.91    0.000197           6        31           close
  3.35    0.000169          12        14         6 readlink
  2.42    0.000122          24         5           mprotect
  1.94    0.000098          16         6           brk
  1.21    0.000061           8         7           rt_sigaction
  1.03    0.000052          10         5           ioctl
  0.87    0.000044          22         2           munmap
  0.65    0.000033          16         2           pread64
  0.46    0.000023          11         2           getrandom
  0.42    0.000021           3         6           newfstatat
  0.38    0.000019           9         2           prlimit64
  0.36    0.000018          18         1         1 access
  0.36    0.000018           6         3           sigaltstack
  0.34    0.000017          17         1           poll
  0.34    0.000017           3         5           lseek
  0.22    0.000011          11         1           sched_getaffinity
  0.18    0.000009           9         1           arch_prctl
  0.16    0.000008           8         1           set_tid_address
  0.16    0.000008           8         1           rseq
  0.14    0.000007           7         1           set_robust_list
  0.00    0.000000           0         1           write
  0.00    0.000000           0         4           socket
  0.00    0.000000           0         4         4 connect
------ ----------- ----------- --------- --------- ------------------
100.00    0.005042          13       367        87 total

…s/coreutils into reduce_syscalls_acls

github-actions · 2025-09-18T21:21:12Z

GNU testsuite comparison:

GNU test failed: tests/ls/acl. tests/ls/acl is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/ls/capability. tests/ls/capability is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/ls/no-cap is now being skipped but was previously passing.

github-actions · 2025-09-18T21:45:06Z

GNU testsuite comparison:

GNU test failed: tests/ls/capability. tests/ls/capability is passing on 'main'. Maybe you have to rebase?
Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/ls/no-cap is now being skipped but was previously passing.

github-actions · 2025-09-18T22:40:21Z

GNU testsuite comparison:

GNU test failed: tests/ls/capability. tests/ls/capability is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/ls/no-cap is now being skipped but was previously passing.

github-actions · 2025-09-19T04:53:32Z

GNU testsuite comparison:

GNU test failed: tests/ls/capability. tests/ls/capability is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/ls/no-cap is now being skipped but was previously passing.

github-actions · 2025-09-19T23:17:29Z

GNU testsuite comparison:

GNU test failed: tests/ls/capability. tests/ls/capability is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Note: The gnu test tests/ls/no-cap is now being skipped but was previously passing.

sylvestre · 2025-09-20T11:18:49Z

excellent, happy to merge this when it is ready :)

codspeed-hq · 2025-09-20T17:21:57Z

CodSpeed Performance Report

Merging #8660 will not alter performance

_{Comparing kimono-koans:reduce_syscalls_acls (3d0fd7d) with main (5efafd6)}

Summary

✅ 55 untouched
⏩ 1 skipped¹

1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports. ↩

github-actions · 2025-09-20T17:29:38Z

GNU testsuite comparison:

GNU test failed: tests/ls/capability. tests/ls/capability is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Note: The gnu test tests/ls/no-cap is now being skipped but was previously passing.

github-actions · 2025-09-22T01:18:33Z

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

github-actions · 2025-09-22T02:56:25Z

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Note: The gnu test tests/ls/no-cap is now being skipped but was previously passing.

kimono-koans · 2025-09-22T05:37:42Z

excellent, happy to merge this when it is ready :)

I think this one is ready! Sorry for so much toil beforehand.

sylvestre · 2025-09-22T21:09:56Z

Note: The gnu test tests/ls/no-cap is now being skipped but was previously passing.

seems that it regressed this test

github-actions · 2025-09-23T00:31:31Z

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/ls/no-cap is now being skipped but was previously passing.

kimono-koans · 2025-09-23T00:44:38Z

seems that it regressed this test

How does one know of this is resolved, if this test is skipped?

src/uucore/src/lib/features/fsxattr.rs

github-actions · 2025-09-23T03:05:17Z

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/ls/no-cap is now being skipped but was previously passing.

github-actions · 2025-09-23T16:46:37Z

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Note: The gnu test tests/ls/no-cap is now being skipped but was previously passing.

sylvestre · 2025-09-23T18:57:22Z

but was previously passing.

is the key here :)

kimono-koans · 2025-09-23T21:19:42Z

but was previously passing.

is the key here :)

I don't believe this test was ever passing. Test is rightly skipped because we do not use capget to obtain a files capabilities. Test, to succeed, requires use of the GNU implementation's behavior of calling capget syscall, which AFAICT no library used by uutils currently supports. From the relevant part:

touch file || framework_failure_

setcap 'cap_net_bind_service=ep' file ||
  skip_ "setcap doesn't work"

LS_COLORS=ca=1; export LS_COLORS
strace -e capget ls --color=always > /dev/null 2> out || fail=1
$EGREP 'capget\(' out || skip_ "your ls doesn't call capget"

LS_COLORS=ca=:; export LS_COLORS
strace -e capget ls --color=always > /dev/null 2> out || fail=1
$EGREP 'capget\(' out && fail=1

uutils ls actually passes this test, in that it doesn't print colors for files with capabilities, when the LS_COLORS key ca= is set as empty. It does this by reading the capability key from xattrs. I suppose this may not work on certain filesystems without xattrs (when is the last time you used one of those?), but AFAICT there are no good Rust libraries which support the old capget method for files.

However, note that the current behavior (master) is the wrong behavior. Current behavior is to simply to check if the file had ACLs (wrong) by checking whether there were any xattrs were on the file (wrong), not even to check the correct key. This PR is an improvement.

See current master:

// Check if the file has capabilities
    #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))]
    {
        // Skip checking capabilities if LS_COLORS=ca=:
        let capabilities = style_manager
            .colors
            .style_for_indicator(Indicator::Capabilities);

        let has_capabilities = if capabilities.is_none() {
            false
        } else {
            uucore::fsxattr::has_acl(path.p_buf.as_path())
        };

        // If the file has capabilities, use a specific style for `ca` (capabilities)
        if has_capabilities {
            return style_manager.apply_style(capabilities, name, wrap);
        }
    }
...
pub fn has_acl<P: AsRef<Path>>(file: P) -> bool {
    // don't use exacl here, it is doing more getxattr call then needed
    xattr::list_deref(file).is_ok_and(|acl| {
        // if we have extra attributes, we have an acl
        acl.count() > 0
    })
}

In any event, this GNU test should be skipped or ignored, because we should not force PRs to use precise GNU syscall behavior, when we can use more modern methods, with perhaps other benefits. See also the failing test in my other PR: tests/ls/stat-free-color. There, the idea behind the GNU test, is not to use statx, but who cares how GNU ls does what it does, if the end result is fewer syscalls and less system time used?

sylvestre · 2025-09-24T06:05:55Z

the idea behind the GNU test, is not to use statx, but who cares how GNU ls does what it does, if the end result is fewer syscalls and less system time used?

Agreed

is this PR ready? :)

kimono-koans · 2025-09-24T07:15:25Z

the idea behind the GNU test, is not to use statx, but who cares how GNU ls does what it does, if the end result is fewer syscalls and less system time used?

Agreed

is this PR ready? :)

Yes I think so! Most of these changes were/are about correctness.

The big improvement in # of syscalls will be in the next PR however: #8663. And in any update to the xattr crate, when it is ready: #8660 (comment)

github-actions · 2025-09-24T07:38:14Z

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/tail/overlay-headers is no longer failing!
Note: The gnu test tests/ls/no-cap is now being skipped but was previously passing.

sylvestre · 2025-09-24T07:39:28Z

I guess it has a positive impact on performance, did you run benchmarking (hyperfine) on this ? thanks

kimono-koans · 2025-09-24T07:58:36Z

I guess it has a positive impact on performance, did you run benchmarking (hyperfine) on this ? thanks

Right now any performance differences are modest (worth 1 or 2 ms in hyperfine?). I have a feeling this will have a big impact when I/others merge a few more PRs.

After spending some time with it, my guess is that, oddly, any performance difference is not down to syscalls. These seem to happen pretty fast.

Much more likely it is silly allocation behavior. Like create a bunch of small Strings instead of creating one String, etc. I have another PR where we were previously doing an insert and a remove on a HashMap for each directory we entered, needlessly. Stuff like that.

github-actions · 2025-09-25T16:48:44Z

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/tail/overlay-headers (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/ls/no-cap is now being skipped but was previously passing.

kimono-koans added 6 commits September 18, 2025 15:11

Initial commit

44efeaa

Merge branch 'uutils:main' into reduce_syscalls_acls

41fb8f9

Add default ACL

18ee2b8

Merge branch 'reduce_syscalls_acls' of https://github.com/kimono-koan…

95f5c05

…s/coreutils into reduce_syscalls_acls

Revert, do not count a default acl key as an ACL because GNU does not

80df9ee

Wrapped Result<Option<Vec<_>>> must be non-empty to indicate an ACL

6616412

Add back default ACL

d68298d

kimono-koans added 3 commits September 18, 2025 16:59

Cleanup

65f2b1f

Avoid 2nd syscall for non-directories

c5be794

Cleanup

22cf205

kimono-koans changed the title ~~Fix excessive ls -al syscalls~~ ls: Fix excessive ls -al syscalls for ACL metadata Sep 19, 2025

Fix buggy tests

9818dc0

Merge branch 'main' into reduce_syscalls_acls

cb75dac

kimono-koans added 7 commits September 20, 2025 11:21

Lazily obtain ACL, only really useful in LongFormat

ff6748e

Cleanup

59198d3

Cleanup

d80eb15

Cleanup

a97d3af

Fix broken test

d7271bf

Cleanup

c9a71e0

Cleanup

35e9ff8

Cleanup

59410bd

This comment was marked as outdated.

Sign in to view

Fix lints

2d139bc

kimono-koans added 3 commits September 21, 2025 21:22

Fix lints

b4a1709

Cleanup

dc21fe5

Fix lints

b755002

kimono-koans added 3 commits September 22, 2025 17:37

Fix GNU test

0820d44

Fix lint

001bc61

Fix lints

8761312

Stebalien reviewed Sep 23, 2025

View reviewed changes

src/uucore/src/lib/features/fsxattr.rs Outdated Show resolved Hide resolved

kimono-koans added 3 commits September 22, 2025 21:28

Remove LazyLock for ACL, Cap keys

6496129

Fix lints

e3b3a75

Merge branch 'main' into reduce_syscalls_acls

b63aa01

Merge branch 'main' into reduce_syscalls_acls

1c62b50

Merge branch 'main' into reduce_syscalls_acls

d79a01c

Merge branch 'main' into reduce_syscalls_acls

3d0fd7d

Uh oh!

ls: Fix excessive ls -al syscalls for ACL metadata #8660

Are you sure you want to change the base?

ls: Fix excessive ls -al syscalls for ACL metadata #8660

Uh oh!

Conversation

kimono-koans commented Sep 18, 2025

Uh oh!

github-actions bot commented Sep 18, 2025

Uh oh!

github-actions bot commented Sep 18, 2025

Uh oh!

github-actions bot commented Sep 18, 2025

Uh oh!

github-actions bot commented Sep 19, 2025

Uh oh!

github-actions bot commented Sep 19, 2025

Uh oh!

sylvestre commented Sep 20, 2025

Uh oh!

codspeed-hq bot commented Sep 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #8660 will not alter performance

Summary

Footnotes

Uh oh!

github-actions bot commented Sep 20, 2025

Uh oh!

This comment was marked as outdated.

github-actions bot commented Sep 22, 2025

Uh oh!

github-actions bot commented Sep 22, 2025

Uh oh!

kimono-koans commented Sep 22, 2025

Uh oh!

sylvestre commented Sep 22, 2025

Uh oh!

github-actions bot commented Sep 23, 2025

Uh oh!

kimono-koans commented Sep 23, 2025

Uh oh!

Uh oh!

github-actions bot commented Sep 23, 2025

Uh oh!

github-actions bot commented Sep 23, 2025

Uh oh!

sylvestre commented Sep 23, 2025

Uh oh!

kimono-koans commented Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sylvestre commented Sep 24, 2025

Uh oh!

kimono-koans commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 24, 2025

Uh oh!

sylvestre commented Sep 24, 2025

Uh oh!

kimono-koans commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 25, 2025

Uh oh!

Uh oh!

codspeed-hq bot commented Sep 20, 2025 •

edited

Loading

kimono-koans commented Sep 23, 2025 •

edited

Loading

kimono-koans commented Sep 24, 2025 •

edited

Loading

kimono-koans commented Sep 24, 2025 •

edited

Loading