* [PATCH] xfs: add per-mount read/write I/O completion counters
@ 2026-08-28 3:34 Eric Peterson
2026-08-30 21:26 ` Dave Chinner
2026-09-09 1:09 ` Eric Sandeen
0 siblings, 2 replies; 10+ messages in thread
From: Eric Peterson @ 2026-08-28 3:34 UTC (permalink / raw)
To: Carlos Maiolino, linux-xfs; +Cc: linux-kernel, Eric Peterson
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.
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); 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.
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>
---
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 on v6.12.74 shows baseline and patched
fail the identical 6/1277 tests -- zero regressions. The rwcmpl
interface was verified on hardware (rw >= rwcmpl, counters
advance under load). This for-next port applies cleanly with no
drift and compiles clean; a runtime -g quick smoke on for-next was
omitted as the logic is identical to the tested v6.12.74 patch.
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH] xfs: add per-mount read/write I/O completion counters 2026-08-28 3:34 [PATCH] xfs: add per-mount read/write I/O completion counters Eric Peterson @ 2026-08-30 21:26 ` Dave Chinner 2026-08-31 0:47 ` Eric Peterson 2026-09-02 5:32 ` Eric Peterson 2026-09-09 1:09 ` Eric Sandeen 1 sibling, 2 replies; 10+ messages in thread From: Dave Chinner @ 2026-08-30 21:26 UTC (permalink / raw) To: Eric Peterson; +Cc: Carlos Maiolino, linux-xfs, linux-kernel, Eric Peterson On Thu, Aug 27, 2026 at 09:34:29PM -0600, Eric Peterson wrote: > 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. I'm not sure the fs is the right place for this - the bdev has long exposed enough information for filesystem-wide queue depths to be monitored directly. e.g: $ man iostat |grep -A 1 aqu-sz aqu-sz The average queue length of the requests that were issued to the device. $ pminfo -t disk.dev.avg_qlen disk.dev.avg_qlen [average read and write queue length] $ and so on. Hence this really doesn't seem like something we should be trying to infer from indirect filesystem stats. Why can't you use the bdev stats to get the actual filesystem wide queue depth information? -Dave. -- Dave Chinner dgc@kernel.org ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: add per-mount read/write I/O completion counters 2026-08-30 21:26 ` Dave Chinner @ 2026-08-31 0:47 ` Eric Peterson 2026-08-31 6:43 ` Carlos Maiolino 2026-08-31 9:38 ` [PATCH] " Dave Chinner 2026-09-02 5:32 ` Eric Peterson 1 sibling, 2 replies; 10+ messages in thread From: Eric Peterson @ 2026-08-31 0:47 UTC (permalink / raw) To: Dave Chinner Cc: Eric Peterson, Carlos Maiolino, linux-xfs, linux-kernel, Eric Peterson On Mon, Aug 31, 2026 at 07:26:52AM +1000, Dave Chinner wrote: > Hence this really doesn't seem like something we should be trying to > infer from indirect filesystem stats. Why can't you use the bdev > stats to get the actual filesystem wide queue depth information? The block device measures the device queue, which is a different quantity than filesystem outstanding I/O - not just a lower-layer view of the same thing. Below are three cases where 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 block-device queue depth answers "how deep is the device queue," which is not the same as "how much work is outstanding in the filesystem." When the filesystem is just one layer an I/O passes through, the block stats fold the layers together and structurally cannot isolate the filesystem's own contribution. To be clear about scope: I'm not proposing a queue-depth feature in the kernel. The change just adds read/write completion counters to pair with the existing call (submission) counters, so userspace can compute outstanding I/O and derive a response-time estimate itself. The kernel side is only exposing the complementary raw signal that's currently missing - calls are counted, completions are not. Being upfront: what userspace derives from this is an instantaneous approximation, not a precise time-weighted queue length. It's meant as a cheap, always-on aggregate, not a replacement for accurate per-op tooling. Does exposing the completion side of the existing call counters seem reasonable on that basis? -Eric ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: add per-mount read/write I/O completion counters 2026-08-31 0:47 ` Eric Peterson @ 2026-08-31 6:43 ` Carlos Maiolino 2026-09-02 4:25 ` [PATCH v2] " Eric Peterson 2026-08-31 9:38 ` [PATCH] " Dave Chinner 1 sibling, 1 reply; 10+ messages in thread From: Carlos Maiolino @ 2026-08-31 6:43 UTC (permalink / raw) To: Eric Peterson; +Cc: Dave Chinner, linux-xfs, linux-kernel, Eric Peterson On Sun, Aug 30, 2026 at 06:47:00PM -0600, Eric Peterson wrote: > On Mon, Aug 31, 2026 at 07:26:52AM +1000, Dave Chinner wrote: > > Hence this really doesn't seem like something we should be trying to > > infer from indirect filesystem stats. Why can't you use the bdev > > stats to get the actual filesystem wide queue depth information? > > The block device measures the device queue, which is a different > quantity than filesystem outstanding I/O - not just a lower-layer view > of the same thing. > > Below are three cases where 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. Could you please put those in the commit description? For historic purposes would be good to keep track why this has been added (or not). > > The block-device queue depth answers "how deep is the device queue," > which is not the same as "how much work is outstanding in the > filesystem." When the filesystem is just one layer an I/O passes > through, the block stats fold the layers together and structurally > cannot isolate the filesystem's own contribution. > > To be clear about scope: I'm not proposing a queue-depth feature in > the kernel. The change just adds read/write completion counters to pair > with the existing call (submission) counters, so userspace can compute > outstanding I/O and derive a response-time estimate itself. The kernel > side is only exposing the complementary raw signal that's currently > missing - calls are counted, completions are not. > > Being upfront: what userspace derives from this is an instantaneous > approximation, not a precise time-weighted queue length. It's meant as > a cheap, always-on aggregate, not a replacement for accurate per-op > tooling. > > Does exposing the completion side of the existing call counters seem > reasonable on that basis? > Particularly I liked the idea and the justification seems fair although I'd want to see the justification for the counter in the patch description. Carlos > -Eric > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] xfs: add per-mount read/write I/O completion counters 2026-08-31 6:43 ` Carlos Maiolino @ 2026-09-02 4:25 ` Eric Peterson 0 siblings, 0 replies; 10+ messages in thread From: Eric Peterson @ 2026-09-02 4:25 UTC (permalink / raw) To: Carlos Maiolino, linux-xfs; +Cc: Dave Chinner, linux-kernel, eric.peterson 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: add per-mount read/write I/O completion counters 2026-08-31 0:47 ` Eric Peterson 2026-08-31 6:43 ` Carlos Maiolino @ 2026-08-31 9:38 ` Dave Chinner 1 sibling, 0 replies; 10+ messages in thread From: Dave Chinner @ 2026-08-31 9:38 UTC (permalink / raw) To: Eric Peterson; +Cc: Carlos Maiolino, linux-xfs, linux-kernel, Eric Peterson On Sun, Aug 30, 2026 at 06:47:00PM -0600, Eric Peterson wrote: > On Mon, Aug 31, 2026 at 07:26:52AM +1000, Dave Chinner wrote: > > Hence this really doesn't seem like something we should be trying to > > infer from indirect filesystem stats. Why can't you use the bdev > > stats to get the actual filesystem wide queue depth information? > > The block device measures the device queue, which is a different > quantity than filesystem outstanding I/O - not just a lower-layer view > of the same thing. > > Below are three cases where filesystem queue depth is not what the block > layer sees: I do know the difference. Assume I understand what you are saying, and that you don't need to explain how the IO stack works to me... > To be clear about scope: I'm not proposing a queue-depth feature in > the kernel. The change just adds read/write completion counters to pair > with the existing call (submission) counters, so userspace can compute > outstanding I/O and derive a response-time estimate itself. The kernel > side is only exposing the complementary raw signal that's currently > missing - calls are counted, completions are not. I know, I just don't see how it can be used for a response time metric that any way useful for behavioural correlation because of the sampling method. > Being upfront: what userspace derives from this is an instantaneous > approximation, not a precise time-weighted queue length. It's meant as > a cheap, always-on aggregate, not a replacement for accurate per-op > tooling. And that's exactly why I'm having trouble understanding how this new metric means anything useful. Ignoring temporal sampling jitter of multiple per-cpu counters, if you sample read + completions it at some instant, all it tells you is what is happening at that instant. What happens the other 999.9ms of that second is not captured by this new "in-flight" metric? For example, if I sample read submissions at 10Hz (annotated manually with rough deltas between samples): $ pmval -r -t 0.1 xfs.read metric: xfs.read host: devoid semantics: cumulative counter units: count samples: all 294454912 294454912 S (0 IO in flight) 294454912 294454912 294454912 294454912 294454912 294454912 294454912 294454912 294455553 +650 294455553 S (0 IO in flight) 294455555 +2 294455555 294455555 294455555 294455555 294455555 294455555 294455555 294455559 +4 294455559 S (0 IO in flight) 294455559 294455559 294455559 294455559 294455559 294455559 294455559 294455559 294455559 294455559 S (0 IO in flight) 294455561 +2 294455561 294455561 294455564 294455564 294456360 +800 294457344 +1000 294457344 294457346 +2 294457352 +6 S (at most 6 IO in flight) 294457352 294458065 +700 294458285 294458285 294458285 294458285 294458285 294458285 294458285 294458285 S (0 IO in flight) You can see that there are some 100ms periods where nothing happens, whilst others have 650-1000 buffered reads. In all the cases where there are periods with no submission, the in-flight calculation will be zero. In the busy periods, it will likely be some non-zero number, but it won't give any indication of IO behaviour in that entire period. If we pick a 1s sample time (marked with "S" above), only one of those sample points had any chance of there being IO in flight. If I pick a sampling pattern that hits one of those high IO periods, it gives an unrealisticly high in flight value for the sampling period, given that for most of the rest of the second around that burst there was almost no read activity. Hence I don't see how sampling a point in time "in-flight" metric slowly provides reliable insight into application behaviour. To address that, one would need to sample and calculate the inflight metric at high resolution to be able to catch the concurrency of IO in those high IOPS bursts. However, the faster you sample to catch bursts, the closer the read submission rate approaches the in-flight IO rate. i.e. if I sample at 1000Hz instead of 10Hz, it'll capture the fact that there are bursts much faster bursts than 8-10 read IOs per millisecond, yet the in-flight counter still won't reflect that - it might still not register any IO being in flight at all because at the sample instant there was no IO in flight.... Hence I'm asking how this new metric is supposed to be used and correlated to observed/measured application behaviour. i.e. what insight does it give you into application performance that can only be derived from this point in time snapshot? -Dave. -- Dave Chinner dgc@kernel.org ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: add per-mount read/write I/O completion counters 2026-08-30 21:26 ` Dave Chinner 2026-08-31 0:47 ` Eric Peterson @ 2026-09-02 5:32 ` Eric Peterson 2026-09-07 22:53 ` Dave Chinner 1 sibling, 1 reply; 10+ messages in thread From: Eric Peterson @ 2026-09-02 5:32 UTC (permalink / raw) To: Dave Chinner Cc: Carlos Maiolino, linux-xfs, linux-kernel, eric.peterson, Eric Peterson On Mon, Aug 31, 2026 at 09:38 UTC, Dave Chinner wrote: > Hence I'm asking how this new metric is supposed to be used and > correlated to observed/measured application behaviour. i.e. what > insight does it give you into application performance that can only > be derived from this point in time snapshot? My apologies - it wasn't my intention to come across as patronizing. I was unsure what background was or wasn't common ground, so I erred on the side of more detail. You're right about the sampling limitation: a slowly-sampled point-in-time queue depth value cannot characterize bursty, sub-interval concurrency. If the goal is to resolve what happens inside a 10ms burst, this is the wrong tool - per-op tooling (tracepoints, histograms) is the right one, and this is not meant to replace it. The important part is that this is a property of the sampling rate, not of the counters. Nyquist-Shannon says that to observe a phenomenon at timescale T you have to sample at >= 2/T; if you sample slower than the behavior you care about, it will be missed. This is true of any sampled counter, including the existing submission counter - in your 10Hz pmval example, xfs.read has exactly the same property. The sampling rate is a policy choice for the user to match to what they're trying to observe. Answering your question, it lets userspace characterize filesystem queue depth over time. The places where this is useful are the ones where the desired signal persists across multiple sample periods, leading to a representative measurement: - Sustained/steady-state load. Database, NFS server, VM image store, etc. Outstanding I/O is stable across many sample periods. Most capacity and health monitoring lives here. - Long-horizon trends. Can show if queue depth is creeping up over hours or days as load grows or cache becomes insufficient. Leaving per-op tracing running for this kind of timescale is the wrong tool for the job; persistent, low-cost sampling is the better choice. - Sustained-backlog alerting. Consistent elevated depth can indicate saturation, a stuck consumer, or cache thrash. Filtering out small transients avoids adding noise. - Coarse steady-state latency. When load is steady, sustained depth over sustained completion rate gives an average latency - enough precision to tell 0.5ms from 5ms, but not tail latency. Histograms would be the correct tool if higher resolution is required. For higher precision you'd want a time-weighted queue depth, but that requires two clock reads on every I/O in the hot path, and the cost grows with I/O load. This trade-off is the core motivation: the counter is a near-free, always-on aggregate for the common steady-state and trend cases. It does not replace per-op tooling where higher precision is required. -Eric ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: add per-mount read/write I/O completion counters 2026-09-02 5:32 ` Eric Peterson @ 2026-09-07 22:53 ` Dave Chinner 0 siblings, 0 replies; 10+ messages in thread From: Dave Chinner @ 2026-09-07 22:53 UTC (permalink / raw) To: Eric Peterson; +Cc: Carlos Maiolino, linux-xfs, linux-kernel, eric.peterson On Tue, Sep 01, 2026 at 11:32:30PM -0600, Eric Peterson wrote: > On Mon, Aug 31, 2026 at 09:38 UTC, Dave Chinner wrote: > > Hence I'm asking how this new metric is supposed to be used and > > correlated to observed/measured application behaviour. i.e. what > > insight does it give you into application performance that can only > > be derived from this point in time snapshot? > > My apologies - it wasn't my intention to come across as patronizing. > I was unsure what background was or wasn't common ground, so I erred on > the side of more detail. > > You're right about the sampling limitation: a slowly-sampled > point-in-time queue depth value cannot characterize bursty, > sub-interval concurrency. If the goal is to resolve what happens inside > a 10ms burst, this is the wrong tool - per-op tooling (tracepoints, > histograms) is the right one, and this is not meant to replace it. > > The important part is that this is a property of the sampling rate, not > of the counters. Nyquist-Shannon says that to observe a phenomenon at > timescale T you have to sample at >= 2/T; if you sample slower than the > behavior you care about, it will be missed. This is true of any sampled > counter, including the existing submission counter - in your 10Hz pmval > example, xfs.read has exactly the same property. The sampling rate is a > policy choice for the user to match to what they're trying to observe. I know what nyquist sampling implies - that's exactly why I gave that example to demonstrate how point in time sampling of instantaneous values is not representative. In more detail, the nyquist sampling theorem only holds when -every sample is representative of the overall waveform-. You also need to know what the highest frequency of the waveform is to be able to reconstruct the behaviour. The problem with using nyquist here is that the queue depth is an instantaneous value metric - it is not a repeating waveform. Hence nyquist says it is impossible to recover a valid signal from the metric because the sample rate must be 2x instantaneous. Something like a summing value (i.e. read count) does allow nyquist theorem to be applied, because the delta between samples leads to a meaningful waveform - the read rate per sample period. But you cannot convert an instantaneous value calculated from summing variables into a rate metric - it is always an instantaneous value and that means the delta between samples is meaningless when placed in a time series. i.e. there is no frequency component that nyquist sampling theorem can recover from it. Let's go further. Last email I said "ignoring per-cpu summing jitter". Did you think about that at all? Look at how the samples are presented to userspace: for each counter group { for each counter in group { for each cpu { sum counter } print counter val } } Think about that for a moment. What happens when you have hundreds of CPUs (just call it N)? Yeah, summing each counter is an expensive operation, involving accessing N cachelines for each counter. If each of those stats is being actively modified whilst the sum is in progress, we take cache miss on each CPU for each counter. Let's be charitable and call that 100ns per CPU. For a thousand CPUs, that means it takes 100us to sample that counter. Now, read completion was placed directly after reads, so there is a window of 100us between the sampling of each per cpu value. Consider that a buffered read could take as little as a few microseconds to run. That means that across the sampling of read + read completion, each CPU could start and complete multiple buffered IOs. In that situation, we have on a single CPU: read 1 .... sample read X read 1 complete read 2 .... read 2 complete read 3 .... read 3 complete .... read N .... sample read_complete X + (N - 1) In this situation, the "queue depth" on this CPU which is (read - read_complete) returns -(N - 1). What does a -negative queue depth- mean? And what happens when there is sufficient other read/read_complete differences on other CPUs that this negative sample cancels out all the other positive "queue depths"? IOWs, we can't even trust the instantaneous value of the queue depth calculation to be a valid representation of the state of the filesystem at a single point in time. Hence any downstream use of the value (regardless of the sample rate) is not going to be any more trustworthy than the instantaneous value.... > Answering your question, it lets userspace characterize filesystem > queue depth over time. The places where this is useful are the ones > where the desired signal persists across multiple sample periods, > leading to a representative measurement: .... All of these use cases are based on the assumption that the underlying metric and the sampling method produces a valid and/or meaningful representation of the current filesystem state. I am not convinced that this is a valid assumption because of the nature of the sampling - an instantaneous value sample cannot be representative of overall behaviour, regardless of the sampling rate. However, if you really, really want this counter added then I won't oppose it based on the fact I think it cannot be used the way you want to use it - I'll just ignore it like I do all the other useless stats we still keep around from the days of Irix for userspace compatibility reasons. Cheers, Dave. -- Dave Chinner dgc@kernel.org ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: add per-mount read/write I/O completion counters 2026-08-28 3:34 [PATCH] xfs: add per-mount read/write I/O completion counters Eric Peterson 2026-08-30 21:26 ` Dave Chinner @ 2026-09-09 1:09 ` Eric Sandeen 2026-09-11 0:00 ` Eric Peterson 1 sibling, 1 reply; 10+ messages in thread From: Eric Sandeen @ 2026-09-09 1:09 UTC (permalink / raw) To: Eric Peterson, Carlos Maiolino, linux-xfs; +Cc: linux-kernel, Eric Peterson On 8/27/26 10:34 PM, Eric Peterson wrote: > 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. After reading through this thread, it occurs to me that it might be worth asking my favorite question: what problem are you trying to solve? Please don't answer with LLM output. Thanks, -Eric > 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); 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. > > 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> > --- > > 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 on v6.12.74 shows baseline and patched > fail the identical 6/1277 tests -- zero regressions. The rwcmpl > interface was verified on hardware (rw >= rwcmpl, counters > advance under load). This for-next port applies cleanly with no > drift and compiles clean; a runtime -g quick smoke on for-next was > omitted as the logic is identical to the tested v6.12.74 patch. > 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; ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xfs: add per-mount read/write I/O completion counters 2026-09-09 1:09 ` Eric Sandeen @ 2026-09-11 0:00 ` Eric Peterson 0 siblings, 0 replies; 10+ messages in thread From: Eric Peterson @ 2026-09-11 0:00 UTC (permalink / raw) To: Eric Sandeen, linux-xfs Cc: Carlos Maiolino, Dave Chinner, linux-kernel, eric.peterson, Eric Peterson Eric, I have an application that sends IO through several different layers. In my case: Ethernet adapter -> NFS -> XFS -> Block device -> SSD I already have visibility of qdepth/RT and IOPS at the ethernet adapter, block dev, and SSD, but I'm missing information about what is going on in the middle. I wanted a "light touch" way of measuring what is going on in the middle so that I could have hard data about the performance impact of my changes. As part of that "light touch", I wanted to avoid a method that would artificially inflate metrics due to the increased load from instrumentation. In addition, my approach was to follow the existing code structure of counters that were already present. Regarding Dave's points about edge cases he illustrated, I am in agreement with them on the technical basis. If precision is decided to be more important than overhead, then this is not the right course of action. However, I disagree that the edge cases make this a "useless" way of taking measurements. As I shared, there are still several applications that would benefit from this. Regarding whether these counters get merged or not, I already have them present in my kernel builds. My personal needs are met. I chose to submit a patch because I believed others could also find benefit from them. If the cost of the stat was cheap enough, and given the right understanding of the caveats of this method, it was worth sharing. -Other Eric ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-09-11 0:00 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-28 3:34 [PATCH] xfs: add per-mount read/write I/O completion counters Eric Peterson 2026-08-30 21:26 ` Dave Chinner 2026-08-31 0:47 ` Eric Peterson 2026-08-31 6:43 ` Carlos Maiolino 2026-09-02 4:25 ` [PATCH v2] " Eric Peterson 2026-08-31 9:38 ` [PATCH] " Dave Chinner 2026-09-02 5:32 ` Eric Peterson 2026-09-07 22:53 ` Dave Chinner 2026-09-09 1:09 ` Eric Sandeen 2026-09-11 0:00 ` Eric Peterson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox