[PATCH] util: Fix missing carry of one second, as nanoseconds, in timespec_diff_us()

[PATCH] util: Fix missing carry of one second, as nanoseconds, in timespec_diff_us()

[Stefano Brivio]

If the nanoseconds of the minuend timestamp are less than the
nanoseconds of the subtrahend timestamp, we need to carry one second
in the subtraction.

I subtracted this second from the minuend, but didn't actually carry
it in the subtraction of nanoseconds, and logged timestamps would jump
back whenever we switched to the first branch of timespec_diff_us()
from the second one.

Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util.c b/util.c
index 6e64279..eede4e5 100644
--- a/util.c
+++ b/util.c
@@ -249,7 +249,7 @@ void sock_probe_mem(struct ctx *c)
 int64_t timespec_diff_us(const struct timespec *a, const struct timespec *b)
 {
 	if (a->tv_nsec < b->tv_nsec) {
-		return (b->tv_nsec - a->tv_nsec) / 1000 +
+		return (a->tv_nsec + 1000000000 - b->tv_nsec) / 1000 +
 		       (a->tv_sec - b->tv_sec - 1) * 1000000;
 	}
 
-- 
@@ -249,7 +249,7 @@ void sock_probe_mem(struct ctx *c)
 int64_t timespec_diff_us(const struct timespec *a, const struct timespec *b)
 {
 	if (a->tv_nsec < b->tv_nsec) {
-		return (b->tv_nsec - a->tv_nsec) / 1000 +
+		return (a->tv_nsec + 1000000000 - b->tv_nsec) / 1000 +
 		       (a->tv_sec - b->tv_sec - 1) * 1000000;
 	}
 
-- 
2.43.0

Re: [PATCH] util: Fix missing carry of one second, as nanoseconds, in timespec_diff_us()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1463 bytes --]

On Fri, Sep 06, 2024 at 12:49:39PM +0200, Stefano Brivio wrote:
> If the nanoseconds of the minuend timestamp are less than the
> nanoseconds of the subtrahend timestamp, we need to carry one second
> in the subtraction.
> 
> I subtracted this second from the minuend, but didn't actually carry
> it in the subtraction of nanoseconds, and logged timestamps would jump
> back whenever we switched to the first branch of timespec_diff_us()
> from the second one.

Perhaps more to the point, the subtraction was the wrong way around
before.  I think we're too used to math mod 2^n, where reversing the
order of the arguments would be equivalent to adding an extra 2^n.

> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> ---
>  util.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/util.c b/util.c
> index 6e64279..eede4e5 100644
> --- a/util.c
> +++ b/util.c
> @@ -249,7 +249,7 @@ void sock_probe_mem(struct ctx *c)
>  int64_t timespec_diff_us(const struct timespec *a, const struct timespec *b)
>  {
>  	if (a->tv_nsec < b->tv_nsec) {
> -		return (b->tv_nsec - a->tv_nsec) / 1000 +
> +		return (a->tv_nsec + 1000000000 - b->tv_nsec) / 1000 +
>  		       (a->tv_sec - b->tv_sec - 1) * 1000000;
>  	}
>  

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] util: Fix missing carry of one second, as nanoseconds, in timespec_diff_us()

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1677 bytes --]

On Fri, Sep 06, 2024 at 08:56:06PM +1000, David Gibson wrote:
> On Fri, Sep 06, 2024 at 12:49:39PM +0200, Stefano Brivio wrote:
> > If the nanoseconds of the minuend timestamp are less than the
> > nanoseconds of the subtrahend timestamp, we need to carry one second
> > in the subtraction.
> > 
> > I subtracted this second from the minuend, but didn't actually carry
> > it in the subtraction of nanoseconds, and logged timestamps would jump
> > back whenever we switched to the first branch of timespec_diff_us()
> > from the second one.
> 
> Perhaps more to the point, the subtraction was the wrong way around
> before.  I think we're too used to math mod 2^n, where reversing the
> order of the arguments would be equivalent to adding an extra 2^n.

Oops, and also

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> 
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> > ---
> >  util.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/util.c b/util.c
> > index 6e64279..eede4e5 100644
> > --- a/util.c
> > +++ b/util.c
> > @@ -249,7 +249,7 @@ void sock_probe_mem(struct ctx *c)
> >  int64_t timespec_diff_us(const struct timespec *a, const struct timespec *b)
> >  {
> >  	if (a->tv_nsec < b->tv_nsec) {
> > -		return (b->tv_nsec - a->tv_nsec) / 1000 +
> > +		return (a->tv_nsec + 1000000000 - b->tv_nsec) / 1000 +
> >  		       (a->tv_sec - b->tv_sec - 1) * 1000000;
> >  	}
> >  
> 



-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] util: Fix missing carry of one second, as nanoseconds, in timespec_diff_us()

[Stefano Brivio]

On Fri, 6 Sep 2024 20:56:06 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Sep 06, 2024 at 12:49:39PM +0200, Stefano Brivio wrote:
> > If the nanoseconds of the minuend timestamp are less than the
> > nanoseconds of the subtrahend timestamp, we need to carry one second
> > in the subtraction.
> > 
> > I subtracted this second from the minuend, but didn't actually carry
> > it in the subtraction of nanoseconds, and logged timestamps would jump
> > back whenever we switched to the first branch of timespec_diff_us()
> > from the second one.  
> 
> Perhaps more to the point, the subtraction was the wrong way around
> before.

Ah, right, I just slightly tweaked the commit message:

--
util: Fix order of operands and carry of one second in timespec_diff_us()

If the nanoseconds of the minuend timestamp are less than the
nanoseconds of the subtrahend timestamp, we need to carry one second
in the subtraction.

I subtracted this second from the minuend, but didn't actually carry
it in the subtraction of nanoseconds, and logged timestamps would jump
back whenever we switched to the first branch of timespec_diff_us()
from the second one.

Most likely, the reason why I didn't carry the second is that I
instinctively thought that swapping the operands would have the same
effect. But it doesn't, in general: that only happens with arithmetic
in modulo powers of two. Undo the swap as well.
--

I kept your Reviewed-by.

-- 
Stefano