On 2021/11/23 23:27, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@xxxxxxx>
>
> Commit 56440e63ac17 ("fio: report percentiles for slat, clat, lat") changed
> many things. One of the changes, from the commit message:
> "- for the new cmdprio_percentage latencies, if lat_percentiles=1,
> *total* latency percentiles will be tracked. Otherwise, *completion*
> latency percentiles will be tracked."
>
> In other words, the commit changed the per prio stats from always tracking
> (and reporting) clat latency, to instead either track (and report) clat or
> lat latency.
>
> Considering that a certain latency type reports two things:
> 1) min/max/avg latency for the the specific latency type
> 2) latency percentiles for the specific latency type
>
> If disable_clat/disable_lat is used, neither 1) nor 2) will be reported.
> If clat_percentiles/lat_percentiles is false, 2) will not be reported.
>
> Therefore it is unintuitive that setting lat_percentiles=1, an option
> usually used to enable/disable percentile reporting, also affects which
> type of latency that will be tracked (and reported) for per prio stats.
>
> The fact that the variables are named e.g. clat_prio_stat, regardless of
> the type of latency being tracked does not help.
>
> Anyway, let's document the way that the current implementation works,
> so that a user can know how per priority stats are handled, without having
> to read the source, since the commit that introduced this behavior forgot
> to update the documentation.
>
> Fixes: 56440e63ac17 ("fio: report percentiles for slat, clat, lat")
> Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxx>
> ---
> HOWTO | 6 +++++-
> fio.1 | 5 ++++-
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/HOWTO b/HOWTO
> index a3b3acfe..8c9e4135 100644
> --- a/HOWTO
> +++ b/HOWTO
> @@ -2169,7 +2169,11 @@ with the caveat that when used on the command line, they must come after the
> Default: 0. A single value applies to reads and writes. Comma-separated
> values may be specified for reads and writes. For this option to be
> effective, NCQ priority must be supported and enabled, and `direct=1'
> - option must be used. fio must also be run as the root user.
> + option must be used. fio must also be run as the root user. Unlike
> + slat/clat/lat stats, which can be tracked and reported independently, per
> + priority stats only track and report a single type of latency. By default,
> + completion latency (clat) will be reported, if :option:`lat_percentiles` is
> + set, total latency (lat) will be reported.
>
> .. option:: cmdprio_class=int[,int] : [io_uring] [libaio]
>
> diff --git a/fio.1 b/fio.1
> index a6469541..a3ebb67d 100644
> --- a/fio.1
> +++ b/fio.1
> @@ -1967,7 +1967,10 @@ Set the percentage of I/O that will be issued with the highest priority.
> Default: 0. A single value applies to reads and writes. Comma-separated
> values may be specified for reads and writes. For this option to be effective,
> NCQ priority must be supported and enabled, and `direct=1' option must be
> -used. fio must also be run as the root user.
> +used. fio must also be run as the root user. Unlike slat/clat/lat stats, which
> +can be tracked and reported independently, per priority stats only track and
> +report a single type of latency. By default, completion latency (clat) will be
> +reported, if \fBlat_percentiles\fR is set, total latency (lat) will be reported.
> .TP
> .BI (io_uring,libaio)cmdprio_class \fR=\fPint[,int]
> Set the I/O priority class to use for I/Os that must be issued with a
>
Looks good.
Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
--
Damien Le Moal
Western Digital Research
