[PATCH v2] xfs: add per-mount read/write I/O completion counters

Eric Peterson posted 1 patch 3 weeks, 3 days ago
fs/xfs/xfs_file.c  | 11 +++++++++--
fs/xfs/xfs_stats.c |  3 ++-
fs/xfs/xfs_stats.h |  2 ++
3 files changed, 13 insertions(+), 3 deletions(-)
[PATCH v2] xfs: add per-mount read/write I/O completion counters
Posted by Eric Peterson 3 weeks, 3 days ago
From: Eric Peterson <eric.peterson@hpe.com>

Add two per-mount statistics counters, xs_read_completions and
xs_write_completions, to complement the existing xs_read_calls and
xs_write_calls counters. The existing counters count I/O submissions
(entries); the new counters count I/O completions. The pair (calls,
completions) lets a consumer compute outstanding I/O as a queue depth
(calls - completions) and, via Little's law, derive an approximate
response time in userspace without any hot-path timestamping.

Block device stats expose device queue depth, but that is a different
quantity from filesystem outstanding I/O. There are cases where the
filesystem queue depth is not what the block layer sees:

1. Cache hits never reach the block layer. Under a heavy read workload
   with a warm cache, a large share of ops are serviced from the page
   cache and are never seen at the block level. Device queue depth can
   sit near zero while the filesystem is servicing a very high op rate.

2. Filesystem ops don't map 1:1 to block I/O. A single read or write can
   produce one block I/O, several (metadata, readahead, writeback
   coalescing), or none at all. So device queue depth isn't the
   filesystem's outstanding-operation count.

3. Work can be outstanding inside the filesystem before any block I/O is
   issued - waiting on locks, log space/reservation, delalloc, etc.
   Such I/O has entered the filesystem but is invisible at the bdev.

The counters are plain monotonic increments (no clock reads), so they
add negligible cost to the read/write path. Per-op timestamping was
deliberately not used: a clock read on the hot path costs ~20-30 ns on
TSC but hundreds of ns to ~1 us on HPET, which would be a regression for
general users. Queue depth from completion counters is an approximation
(instantaneous depth, not time-weighted); this is a deliberate design
choice, not a placeholder.

Completions are accounted at exactly the same sites where XFS already
accounts the xs_*_bytes counters, so their semantics match the existing
byte counters per path:

  - Reads are counted at the frame in xfs_file_read_iter and
    xfs_file_splice_read.
  - Buffered writes are counted at the frame, i.e. when data reaches
    the page cache, mirroring how xs_write_bytes is accounted for
    buffered writes -- not at physical writeback.
  - DAX writes are counted at the frame after the synchronous
    dax_iomap_rw copy returns, mirroring xs_write_bytes for DAX.
  - Direct I/O writes are counted at true completion in
    xfs_dio_write_end_io, which is async-safe and fires for both sync
    and async DIO, mirroring xs_write_bytes for DIO.

Caveat: async O_DIRECT reads are counted at submission, not completion,
because XFS has no read end_io today (iomap_dio_rw is called with NULL
ops for reads). This matches the existing read-byte semantics. Adding a
read end_io for async-DIO-read precision is a larger change, deliberately
deferred.

The counters are uint32_t and wrap like the existing xs_*_calls
counters; userspace diffs handle wrap.

The per-mount stats file gains a new appended "rwcmpl" line printing
write and read completions. The existing "rw" line is unchanged, so
positional parsers of "rw" are unaffected:

  rw     <write_calls>       <read_calls>
  rwcmpl <write_completions> <read_completions>

Signed-off-by: Eric Peterson <eric.peterson@hpe.com>
---

v2:
- Expand the commit message with the rationale for why filesystem
  outstanding I/O differs from block-device queue depth (cache
  hits, no 1:1 op-to-block mapping, and work outstanding inside the
  filesystem before any block I/O). No code change from v1.
  (Carlos Maiolino)

Notes for reviewers (not part of the commit log):

* Placement: the new "rwcmpl" group is inserted between "rw" and
  "attr" in the xstats[] table. The "rw" line itself is unchanged,
  and "rwcmpl" is appended after it, but lines below "rw" in
  /proc/fs/xfs/stat shift by one for strictly positional parsers. I
  can instead append the group at the END of the table if preferred.

* checkpatch --strict reports two CHECKs preferring u32 over
  uint32_t for the new fields. They are kept as uint32_t to match
  struct __xfsstats, whose every field is uint32_t; changing only
  these two would break local consistency.

* Testing: fstests -g auto shows baseline and patched fail the
  identical tests -- zero regressions. The rwcmpl interface was
  verified on hardware (rw >= rwcmpl, counters advance under load).
 fs/xfs/xfs_file.c  | 11 +++++++++--
 fs/xfs/xfs_stats.c |  3 ++-
 fs/xfs/xfs_stats.h |  2 ++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 426a67b813..3ecd4ed534 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -347,8 +347,10 @@ xfs_file_read_iter(
 	else
 		ret = xfs_file_buffered_read(iocb, to);

-	if (ret > 0)
+	if (ret > 0) {
 		XFS_STATS_ADD(mp, xs_read_bytes, ret);
+		XFS_STATS_INC(mp, xs_read_completions);
+	}
 	return ret;
 }

@@ -375,8 +377,10 @@ xfs_file_splice_read(
 	xfs_ilock(ip, XFS_IOLOCK_SHARED);
 	ret = filemap_splice_read(in, ppos, pipe, len, flags);
 	xfs_iunlock(ip, XFS_IOLOCK_SHARED);
-	if (ret > 0)
+	if (ret > 0) {
 		XFS_STATS_ADD(mp, xs_read_bytes, ret);
+		XFS_STATS_INC(mp, xs_read_completions);
+	}
 	return ret;
 }

@@ -663,6 +667,7 @@ xfs_dio_write_end_io(
 	 * for it on submission.
 	 */
 	XFS_STATS_ADD(ip->i_mount, xs_write_bytes, size);
+	XFS_STATS_INC(ip->i_mount, xs_write_completions);

 	/*
 	 * We can allocate memory here while doing writeback on behalf of
@@ -1032,6 +1037,7 @@ xfs_file_dax_write(

 	if (ret > 0) {
 		XFS_STATS_ADD(ip->i_mount, xs_write_bytes, ret);
+		XFS_STATS_INC(ip->i_mount, xs_write_completions);

 		/* Handle various SYNC-type writes */
 		ret = generic_write_sync(iocb, ret);
@@ -1098,6 +1104,7 @@ xfs_file_buffered_write(

 	if (ret > 0) {
 		XFS_STATS_ADD(ip->i_mount, xs_write_bytes, ret);
+		XFS_STATS_INC(ip->i_mount, xs_write_completions);
 		/* Handle various SYNC-type writes */
 		ret = generic_write_sync(iocb, ret);
 	}
diff --git a/fs/xfs/xfs_stats.c b/fs/xfs/xfs_stats.c
index c13d600732..5b276666b6 100644
--- a/fs/xfs/xfs_stats.c
+++ b/fs/xfs/xfs_stats.c
@@ -40,7 +40,8 @@ int xfs_stats_format(struct xfsstats __percpu *stats, char *buf)
 		{ "log",		xfsstats_offset(xs_try_logspace)},
 		{ "push_ail",		xfsstats_offset(xs_xstrat_quick)},
 		{ "xstrat",		xfsstats_offset(xs_write_calls)	},
-		{ "rw",			xfsstats_offset(xs_attr_get)	},
+		{ "rw",			xfsstats_offset(xs_write_completions)	},
+		{ "rwcmpl",		xfsstats_offset(xs_attr_get)	},
 		{ "attr",		xfsstats_offset(xs_iflush_count)},
 		{ "icluster",		xfsstats_offset(xs_inodes_active) },
 		{ "vnodes",		xfsstats_offset(xb_get)		},
diff --git a/fs/xfs/xfs_stats.h b/fs/xfs/xfs_stats.h
index 57c32b86c3..608d12d0c6 100644
--- a/fs/xfs/xfs_stats.h
+++ b/fs/xfs/xfs_stats.h
@@ -93,6 +93,8 @@ struct __xfsstats {
 	uint32_t		xs_xstrat_split;
 	uint32_t		xs_write_calls;
 	uint32_t		xs_read_calls;
+	uint32_t		xs_write_completions;
+	uint32_t		xs_read_completions;
 	uint32_t		xs_attr_get;
 	uint32_t		xs_attr_set;
 	uint32_t		xs_attr_remove;
-- 
2.39.5