xfs_io: fix operation time reporting

From: Dave Chinner <[email protected]>

CUrrently the 100th/sec units always report zero, such as:

32 MiB, 8192 ops; 0:00:21.00 (1.476 MiB/sec and 377.9260 ops/sec)

                          ^^

This is incorrect. Fix the maths that is wrong by removing all the unnecesary floating point maths and just using basic integer division...

Signed-off-by Dave Chinner <[email protected]>

---
 libxcmd/input.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

libxcmd/input.c +0 -0

index 441bb2fbbf34..6e7a8c9822ee 100644

libxcmd/input.c +1 -3

 void
 timestr(

@@ -165,14 +166,12 @@ timestr(
 	size_t		size,
 	int		format)
 {
-	double		usec = (double)tv->tv_usec / 1000000.0;
-
 	if (format & TERSE_FIXED_TIME) {
 		if (!HOURS(tv->tv_sec)) {
 			snprintf(ts, size, "%u:%02u.%02u",
 				(unsigned int) MINUTES(tv->tv_sec),
 				(unsigned int) SECONDS(tv->tv_sec),
-				(unsigned int) usec * 100);
+				(unsigned int) USEC_TO_100THS(tv->tv_usec));
 			return;
 		}
 		format |= VERBOSE_FIXED_TIME;	/* fallback if hours needed */

libxcmd/input.c +0 -0

 	}
 }
 

Quoted 21 lines · Dave Chinner

From: Dave Chinner <[email protected]>

CUrrently the 100th/sec units always report zero, such as:

32 MiB, 8192 ops; 0:00:21.00 (1.476 MiB/sec and 377.9260 ops/sec)

                          ^^

This is incorrect. Fix the maths that is wrong by removing all the unnecesary floating point maths and just using basic integer division...

Signed-off-by Dave Chinner <[email protected]>

---
 libxcmd/input.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

libxcmd/input.c +0 -0

index 441bb2fbbf34..6e7a8c9822ee 100644

libxcmd/input.c tdiv(double value, struct timeval tv) · line 160

Quoted 10 lines

libxcmd/input.c +4 -3

@@ -154,9 +154,10 @@ tdiv(double value, struct timeval tv)
 	return value / ((double)tv.tv_sec + ((double)tv.tv_usec / 1000000.0));
 }
 
-#define HOURS(sec)	((sec) / (60 * 60))
-#define MINUTES(sec)	(((sec) % (60 * 60)) / 60)
-#define SECONDS(sec)	((sec) % 60)
+#define HOURS(sec)		((sec) / (60 * 60))
+#define MINUTES(sec)		(((sec) % (60 * 60)) / 60)
+#define SECONDS(sec)		((sec) % 60)
+#define USEC_TO_100THS(usec)	((usec) / 1000 / 10)

I guess this works but I expected to convert "microseconds to 100ths" via a conversion like:

usec * 1sec/1000000usec * 100 hundredths/1sec

so just for readability I'd have expected:

#define USEC_TO_100THS(usec) ((usec) / 1000000 * 100)

or possibly just
#define USEC_TO_100THS(usec) ((usec) / 10000)

... I'm confused by your choice of orders of magnitude above even though it works - it seems a bit random to divide it that way, unless I'm missing something?

(I had a physics teacher who drilled THINK UNITS BEFORE YOU THINK NUMBERS" into my head an I still do) :)

Quoted 22 lines

libxcmd/input.c +1 -3

 void
 timestr(

@@ -165,14 +166,12 @@ timestr(
 	size_t		size,
 	int		format)
 {
-	double		usec = (double)tv->tv_usec / 1000000.0;
-
 	if (format & TERSE_FIXED_TIME) {
 		if (!HOURS(tv->tv_sec)) {
 			snprintf(ts, size, "%u:%02u.%02u",
 				(unsigned int) MINUTES(tv->tv_sec),
 				(unsigned int) SECONDS(tv->tv_sec),
-				(unsigned int) usec * 100);
+				(unsigned int) USEC_TO_100THS(tv->tv_usec));
 			return;
 		}
 		format |= VERBOSE_FIXED_TIME;	/* fallback if hours needed */

@@ -183,9 +182,10 @@ timestr(
 			(unsigned int) HOURS(tv->tv_sec),
 			(unsigned int) MINUTES(tv->tv_sec),
 			(unsigned int) SECONDS(tv->tv_sec),

libxcmd/input.c timestr( · line 188

Quoted 6 lines

libxcmd/input.c +3 -2

-			(unsigned int) usec * 100);
+			(unsigned int) USEC_TO_100THS(tv->tv_usec));
 	} else {
-		snprintf(ts, size, "0.%04u sec", (unsigned int) usec * 10000);
+		snprintf(ts, size, "0.%04u sec",
+			(unsigned int) tv->tv_usec / 100);

USEC_TO_10000THS() for consistency ?

Quoted 36 lines · Eric Sandeen

Quoted 21 lines · Dave Chinner

From: Dave Chinner <[email protected]>

CUrrently the 100th/sec units always report zero, such as:

32 MiB, 8192 ops; 0:00:21.00 (1.476 MiB/sec and 377.9260 ops/sec)

                          ^^

This is incorrect. Fix the maths that is wrong by removing all the unnecesary floating point maths and just using basic integer division...

Signed-off-by Dave Chinner <[email protected]>

---
 libxcmd/input.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

libxcmd/input.c +0 -0

index 441bb2fbbf34..6e7a8c9822ee 100644

libxcmd/input.c tdiv(double value, struct timeval tv) · line 160

Quoted 10 lines

libxcmd/input.c +4 -3

@@ -154,9 +154,10 @@ tdiv(double value, struct timeval tv)
 	return value / ((double)tv.tv_sec + ((double)tv.tv_usec / 1000000.0));
 }
 
-#define HOURS(sec)	((sec) / (60 * 60))
-#define MINUTES(sec)	(((sec) % (60 * 60)) / 60)
-#define SECONDS(sec)	((sec) % 60)
+#define HOURS(sec)		((sec) / (60 * 60))
+#define MINUTES(sec)		(((sec) % (60 * 60)) / 60)
+#define SECONDS(sec)		((sec) % 60)
+#define USEC_TO_100THS(usec)	((usec) / 1000 / 10)

I guess this works but I expected to convert "microseconds to 100ths" via a conversion like:

Quoted 1 line

usec * 1sec/1000000usec * 100 hundredths/1sec

That's what the old code tried to do with floating point math and casts, but that didn't work.

Quoted 3 lines

so just for readability I'd have expected:

#define USEC_TO_100THS(usec) ((usec) / 1000000 * 100)

Welcome to Integer Math 101: Integer Division. You'll always get zero, because N / M = 0 when N < M.

Quoted 9 lines

or possibly just
#define USEC_TO_100THS(usec) ((usec) / 10000)

... I'm confused by your choice of orders of magnitude above even though it works - it seems a bit random to divide it that way, unless I'm missing something?

(I had a physics teacher who drilled THINK UNITS BEFORE YOU THINK NUMBERS" into my head an I still do) :)

Yeah, I did. I did us->ms first, then ms->100ths. i.e.:

milliseconds = usec / 1000
hundreths = ms / 10.

This should be obvious to anyone who uses the metric system for units of measurement.