On Thu, 8 Dec 2022, Crystal Wood wrote:
> New option -W/--bucket-width allows the user to specify how large of a
> range of latencies is covered by a single bucket, including allowing the
> creation of sub-microsecond buckets.
>
> When the flag is not used, output should be unchanged. However, if a
> bucket width is specified that is not a multiple of one microsecond,
> latencies will be output as fractional microseconds, at nanosecond
> precision. This includes JSON output.
>
> When using this option, it is up to the user to determine what level
> of precision is meaningful relative to measurement error, as is noted
> in the documentation.
>
> Signed-off-by: Crystal Wood <swood@xxxxxxxxxx>
> ---
> I'm hoping that this output format is acceptable, including JSON,
> though I'm not familiar with how the latter is typically consumed.
>
> Another option would be to simplify by not making the output precision
> conditional (probably keeping the decimal for readability's sake, at
> least in the non-JSON output) but I didn't want to risk compatibility
> or user surprise issues.
> ---
> src/oslat/oslat.8 | 9 +++-
> src/oslat/oslat.c | 105 +++++++++++++++++++++++++++++++---------------
> 2 files changed, 80 insertions(+), 34 deletions(-)
>
> diff --git a/src/oslat/oslat.8 b/src/oslat/oslat.8
> index 39b36df0db3f..eb96448bfff1 100644
> --- a/src/oslat/oslat.8
> +++ b/src/oslat/oslat.8
> @@ -7,7 +7,7 @@ oslat \- OS Latency Detector
> .RI "[ \-shvz ] [ \-b " bucket-size " ] [ \-B " bias " ] [ \-c " cpu-list " ] \
> [ \-C " cpu-main-thread " ] [ \-f " rt-prio " ] [ \-\-json " filename " ] \
> [ \-m " workload-mem " ] [\-t " runtime " ] [ \-T " trace-threshold " ] \
> -[ \-w " workload " ]"
> +[ \-w " workload " ] [ \-W " bucket-width " ]"
> .SH DESCRIPTION
> .B oslat
> is an open source userspace polling mode stress program to detect OS level
> @@ -57,6 +57,13 @@ NOTE: please make sure the CPU frequency on all testing cores
> are locked before using this parmater. If you don't know how
> to lock the freq then please don't use this parameter.
> .TP
> +.B \-W, \-\-bucket-width
> +Interval between buckets in nanoseconds
> +
> +NOTE: Widths not a multiple of 1000 cause ns-precision output
> +You are responsible for considering the impact of measurement
> +overhead at the nanosecond scale.
> +.TP
> .B \-h, \-\-help
> Show the help message.
> .TP
> diff --git a/src/oslat/oslat.c b/src/oslat/oslat.c
> index 55302f11986b..e8a642699cc7 100644
> --- a/src/oslat/oslat.c
> +++ b/src/oslat/oslat.c
> @@ -192,6 +192,9 @@ struct global {
> struct timeval tv_start;
> int rtprio;
> int bucket_size;
> + int bucket_width;
> + int unit_per_us;
> + int precision;
> int trace_threshold;
> int runtime;
> /* The core that we run the main thread. Default is cpu0 */
> @@ -325,45 +328,46 @@ static float cycles_to_sec(const struct thread *t, uint64_t cycles)
>
> static void insert_bucket(struct thread *t, stamp_t value)
> {
> - int index, us;
> + int index;
> + unsigned int lat;
> uint64_t extra;
> + double us;
>
> - index = value / t->counter_mhz;
> - assert(index >= 0);
> - us = index + 1;
> - assert(us > 0);
> -
> + lat = (value * g.unit_per_us + t->counter_mhz - 1) / t->counter_mhz;
> + us = (double)lat / g.unit_per_us;
> if (!g.preheat && g.trace_threshold && us >= g.trace_threshold) {
> - char *line = "%s: Trace threshold (%d us) triggered on cpu %d with %u us!\n"
> + char *line = "%s: Trace threshold (%d us) triggered on cpu %d with %.*f us!\n"
> "Stopping the test.\n";
> - tracemark(line, g.app_name, g.trace_threshold, t->core_i, us);
> - err_quit(line, g.app_name, g.trace_threshold, t->core_i, us);
> + tracemark(line, g.app_name, g.trace_threshold, t->core_i,
> + g.precision, us);
> + err_quit(line, g.app_name, g.trace_threshold, t->core_i,
> + g.precision, us);
> }
>
> /* Update max latency */
> - if (us > t->maxlat)
> - t->maxlat = us;
> + if (lat > t->maxlat)
> + t->maxlat = lat;
>
> - if (us < t->minlat)
> - t->minlat = us;
> + if (lat < t->minlat)
> + t->minlat = lat;
>
> if (g.bias) {
> /* t->bias will be set after pre-heat if user enabled it */
> - us -= g.bias;
> + lat -= g.bias;
> /*
> * Negative should hardly happen, but if it happens, we assume we're in
> - * the smallest bucket, which is 1us. Same to index.
> + * the smallest bucket.
> */
> - if (us <= 0)
> - us = 1;
> - index -= g.bias;
> - if (index < 0)
> - index = 0;
> + if (lat <= 0)
> + lat = 1;
> }
>
> + index = lat / g.bucket_width;
> + assert(index >= 0);
> +
> /* Too big the jitter; put into the last bucket */
> if (index >= g.bucket_size) {
> - /* Keep the extra bit (in us) */
> + /* Keep the extra bit (in bucket width multiples) */
> extra = index - g.bucket_size;
> if (t->overflow_sum + extra < t->overflow_sum) {
> /* The uint64_t even overflowed itself; bail out */
> @@ -455,6 +459,19 @@ static void *thread_main(void *arg)
> printf("%s\n", end); \
> } while (0)
>
> +#define putfieldp(label, val, end) do { \
> + printf("%12s:\t", label); \
> + for (i = 0; i < g.n_threads; ++i) \
> + printf(" %.*f", g.precision, \
> + (double)(val) / g.unit_per_us); \
> + printf("%s\n", end); \
> + } while (0)
> +
> +static double bucket_to_lat(int bucket)
> +{
> + return (g.bias + (bucket + 1) * (double)g.bucket_width) / g.unit_per_us;
> +}
> +
> void calculate(struct thread *t)
> {
> int i, j;
> @@ -465,11 +482,11 @@ void calculate(struct thread *t)
> /* Calculate average */
> sum = count = 0;
> for (j = 0; j < g.bucket_size; j++) {
> - sum += 1.0 * t[i].buckets[j] * (g.bias+j+1);
> + sum += t[i].buckets[j] * bucket_to_lat(j);
> count += t[i].buckets[j];
> }
> /* Add the extra amount of huge spikes in */
> - sum += t->overflow_sum;
> + sum += t->overflow_sum * g.bucket_width;
> t[i].average = sum / count;
> }
> }
> @@ -501,16 +518,16 @@ static void write_summary(struct thread *t)
> print_dotdotdot = 0;
> }
>
> - snprintf(bucket_name, sizeof(bucket_name), "%03"PRIu64
> - " (us)", g.bias+j+1);
> + snprintf(bucket_name, sizeof(bucket_name), "%03.*f (us)",
> + g.precision, bucket_to_lat(j));
> putfield(bucket_name, t[i].buckets[j], PRIu64,
> (j == g.bucket_size - 1) ? " (including overflows)" : "");
> }
>
> - putfield("Minimum", t[i].minlat, PRIu64, " (us)");
> + putfieldp("Minimum", t[i].minlat, " (us)");
> putfield("Average", t[i].average, ".3lf", " (us)");
> - putfield("Maximum", t[i].maxlat, PRIu64, " (us)");
> - putfield("Max-Min", t[i].maxlat - t[i].minlat, PRIu64, " (us)");
> + putfieldp("Maximum", t[i].maxlat, " (us)");
> + putfieldp("Max-Min", t[i].maxlat - t[i].minlat, " (us)");
> putfield("Duration", cycles_to_sec(&(t[i]), t[i].runtime),
> ".3f", " (sec)");
> printf("\n");
> @@ -537,8 +554,8 @@ static void write_summary_json(FILE *f, void *data)
> if (t[i].buckets[j] == 0)
> continue;
> fprintf(f, "%s", comma ? ",\n" : "\n");
> - fprintf(f, " \"%" PRIu64 "\": %" PRIu64,
> - g.bias+j+1, t[i].buckets[j]);
> + fprintf(f, " \"%.*f\": %" PRIu64,
> + g.precision, bucket_to_lat(j), t[i].buckets[j]);
> comma = 1;
> }
> if (comma)
> @@ -610,6 +627,10 @@ static void usage(int error)
> "-v, --version Display the version of the software.\n"
> "-w, --workload Specify a kind of workload, default is no workload\n"
> " (options: no, memmove)\n"
> + "-W, --bucket-width Interval between buckets in nanoseconds\n"
> + " NOTE: Widths not a multiple of 1000 cause ns-precision output\n"
> + " You are responsible for considering the impact of measurement\n"
> + " overhead at the nanosecond scale.\n"
> "-z, --zero-omit Don't display buckets in the output histogram if all zeros.\n"
> );
> exit(error);
> @@ -630,7 +651,7 @@ static int workload_select(char *name)
> }
>
> enum option_value {
> - OPT_BUCKETSIZE=1, OPT_CPU_LIST, OPT_CPU_MAIN_THREAD,
> + OPT_BUCKETSIZE=1, OPT_BUCKETWIDTH, OPT_CPU_LIST, OPT_CPU_MAIN_THREAD,
> OPT_DURATION, OPT_JSON, OPT_RT_PRIO, OPT_HELP, OPT_TRACE_TH,
> OPT_WORKLOAD, OPT_WORKLOAD_MEM, OPT_BIAS,
> OPT_QUIET, OPT_SINGLE_PREHEAT, OPT_ZERO_OMIT,
> @@ -644,6 +665,7 @@ static void parse_options(int argc, char *argv[])
> int option_index = 0;
> static struct option options[] = {
> { "bucket-size", required_argument, NULL, OPT_BUCKETSIZE },
> + { "bucket-width", required_argument, NULL, OPT_BUCKETWIDTH },
> { "cpu-list", required_argument, NULL, OPT_CPU_LIST },
> { "cpu-main-thread", required_argument, NULL, OPT_CPU_MAIN_THREAD},
> { "duration", required_argument, NULL, OPT_DURATION },
> @@ -660,7 +682,7 @@ static void parse_options(int argc, char *argv[])
> { "version", no_argument, NULL, OPT_VERSION },
> { NULL, 0, NULL, 0 },
> };
> - int i, c = getopt_long(argc, argv, "b:Bc:C:D:f:hm:qsw:T:vz",
> + int i, c = getopt_long(argc, argv, "b:Bc:C:D:f:hm:qsw:W:T:vz",
> options, &option_index);
> long ncores;
>
> @@ -677,6 +699,20 @@ static void parse_options(int argc, char *argv[])
> exit(1);
> }
> break;
> + case OPT_BUCKETWIDTH:
> + case 'W':
> + g.bucket_width = strtol(optarg, NULL, 10);
> + if (g.bucket_size <= 0) {
I think this should be g.bucket_width
> + printf("Illegal bucket width: %s\n", optarg);
> + exit(1);
> + }
> + if (g.bucket_width % 1000) {
> + g.unit_per_us = 1000;
> + g.precision = 3;
> + } else {
> + g.bucket_width /= 1000;
> + }
> + break;
> case OPT_BIAS:
> case 'B':
> g.enable_bias = 1;
> @@ -811,7 +847,8 @@ static void record_bias(struct thread *t)
> bias = t[i].minlat;
> }
> g.bias = bias;
> - printf("Global bias set to %" PRId64 " (us)\n", bias);
> + printf("Global bias set to %.*f (us)\n", g.precision,
> + (double)bias / g.unit_per_us);
> }
>
> int main(int argc, char *argv[])
> @@ -835,6 +872,8 @@ int main(int argc, char *argv[])
> g.app_name = argv[0];
> g.rtprio = 0;
> g.bucket_size = BUCKET_SIZE;
> + g.bucket_width = 1;
> + g.unit_per_us = 1;
> g.runtime = 1;
> g.workload = &workload_list[WORKLOAD_DEFAULT];
> g.workload_mem_size = WORKLOAD_MEM_SIZE;
> --
> 2.38.1
>
>
A quick first look through and run, see the one comment above
near "case 'W'"
and then
checkpatch reports some minor easily fixed problems
../linux/scripts/checkpatch.pl oslat.patch
ERROR: code indent should use tabs where possible
#100: FILE: src/oslat/oslat.c:342:
+^I^I g.precision, us);$
ERROR: code indent should use tabs where possible
#102: FILE: src/oslat/oslat.c:344:
+^I^I g.precision, us);$
ERROR: spaces required around that '=' (ctx:VxV)
#227: FILE: src/oslat/oslat.c:654:
+ OPT_BUCKETSIZE=1, OPT_BUCKETWIDTH, OPT_CPU_LIST,
OPT_CPU_MAIN_THREAD,
^
I'll have a closer look on Monday, but thanks for your patch!
John
