Re: [PATCH v4 0/2] ls: use statx() when it's available

[Andreas Dilger]

On Sep 19, 2019, at 9:59 AM, Jeff Layton <address@hidden> wrote:
> 
> v4:
> - set appropriate STATX_* bits for time_type, sort_type and
>  print_block_size
> 
> v3:
> - syntax cleanups. make syntax-check now passes
> 
> v2:
> - add wrappers for stat_for_ino and fstat_for_ino, don't factor out loop
>  detection
> - style cleanups
> 
> Limiting the distribution list on this posting since the changes from
> the last set are very minor.

I'm not sure what the approval process for patches to fileutils are,
but the whole patch series looks good to me.  You can add (if appropriate):

Reviewed-by: Andreas Dilger <address@hidden>

> Original patch description follows:
> 
> This patchset converts the ls command to use statx instead of stat when
> available. This allows ls to indicate interest in only certain inode
> metadata.
> 
> This is potentially a win on networked/clustered/distributed
> filesystems. In cases where we'd have to do a full, heavyweight stat()
> call we can now do a much lighter statx() call.
> 
> As a real-world example, consider a filesystem like CephFS where one
> client is actively writing to a file and another client does an
> ls --color in the same directory. --color means that we need to fetch
> the mode of the file.
> 
> Doing that with a stat() call means that we have to fetch the size and
> mtime in addition to the mode. The MDS in that situation will have to
> revoke caps in order to ensure that it has up-to-date values to report,
> which disrupts the writer.
> 
> This has a measurable affect on performance. I ran a fio sequential
> write test on one cephfs client and had a second client do "ls --color"
> in a tight loop on the directory that held the file:
> 
> Baseline -- no activity on the second client:
> 
>  WRITE: bw=76.7MiB/s (80.4MB/s), 76.7MiB/s-76.7MiB/s (80.4MB/s-80.4MB/s), 
> io=4600MiB (4824MB), run=60016-60016msec
> 
> Without this patch series, we see a noticable performance hit:
> 
>  WRITE: bw=70.4MiB/s (73.9MB/s), 70.4MiB/s-70.4MiB/s (73.9MB/s-73.9MB/s), 
> io=4228MiB (4433MB), run=60012-60012msec
> 
> With this patch series, we gain most of that ground back:
> 
>  WRITE: bw=75.9MiB/s (79.6MB/s), 75.9MiB/s-75.9MiB/s (79.6MB/s-79.6MB/s), 
> io=4555MiB (4776MB), run=60019-60019msec
> 
> Jeff Layton (2):
>  stat: move struct statx to struct stat conversion routines to new
>    header
>  ls: use statx instead of stat when available
> 
> src/ls.c    | 145 +++++++++++++++++++++++++++++++++++++++++++++++++---
> src/stat.c  |  32 +-----------
> src/statx.h |  52 +++++++++++++++++++
> 3 files changed, 191 insertions(+), 38 deletions(-)
> create mode 100644 src/statx.h
> 
> --
> 2.21.0
> 


Cheers, Andreas

signature.asc
Description: Message signed with OpenPGP