[ofa-general] Re: [PATCH v2] opensm/complib: account for nsec overflow in timeout values

[Ira Weiny]

On Thu, 13 Aug 2009 14:36:20 +0300
Sasha Khapyorsky <sashak at voltaire.com> wrote:

> Hi Ira,
> 
> On 18:37 Thu 06 Aug     , Ira Weiny wrote:
> > 
> > From: Ira Weiny <weiny2 at llnl.gov>
> > Date: Thu, 6 Aug 2009 18:31:46 -0700
> > Subject: [PATCH] opensm/complib: account for nsec overflow in timeout values
> > 
> > 
> > Signed-off-by: Ira Weiny <weiny2 at llnl.gov>
> > ---
> >  opensm/complib/cl_event.c |    8 +++++---
> >  1 files changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/opensm/complib/cl_event.c b/opensm/complib/cl_event.c
> > index d14b2f4..4bc8d37 100644
> > --- a/opensm/complib/cl_event.c
> > +++ b/opensm/complib/cl_event.c
> > @@ -148,9 +148,11 @@ cl_event_wait_on(IN cl_event_t * const p_event,
> >  	} else {
> >  		/* Get the current time */
> >  		if (gettimeofday(&curtime, NULL) == 0) {
> > -			timeout.tv_sec = curtime.tv_sec + (wait_us / 1000000);
> > -			timeout.tv_nsec =
> > -			    (curtime.tv_usec + (wait_us % 1000000)) * 1000;
> > +			uint32_t n_sec = (curtime.tv_usec + (wait_us % 1000000))
> 
> Do you really need fixed size (uint32_t) variable here?

Well I need at least int32_t.  I chose unsigned because we are not trying to go back in time.  I don't like leaving this as "int".  As rare as it might be, a compiler could chose 16bits for an int and that is not big enough, right?

> 
> > +						* 1000;
> > +			timeout.tv_sec = curtime.tv_sec + (wait_us / 1000000)
> > +						+ (n_sec % 1000000000);
> 
> Did you mean (n_sec / 1000000000)?

<sigh> yes... :-(

New patch below,
Ira


From: Ira Weiny <weiny2 at llnl.gov>
Date: Thu, 6 Aug 2009 18:31:46 -0700
Subject: [PATCH] opensm/complib: account for nsec overflow in timeout values


Signed-off-by: Ira Weiny <weiny2 at llnl.gov>
---
 opensm/complib/cl_event.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/opensm/complib/cl_event.c b/opensm/complib/cl_event.c
index d14b2f4..3f17262 100644
--- a/opensm/complib/cl_event.c
+++ b/opensm/complib/cl_event.c
@@ -148,9 +148,11 @@ cl_event_wait_on(IN cl_event_t * const p_event,
 	} else {
 		/* Get the current time */
 		if (gettimeofday(&curtime, NULL) == 0) {
-			timeout.tv_sec = curtime.tv_sec + (wait_us / 1000000);
-			timeout.tv_nsec =
-			    (curtime.tv_usec + (wait_us % 1000000)) * 1000;
+			uint32_t n_sec = (curtime.tv_usec + (wait_us % 1000000))
+						* 1000;
+			timeout.tv_sec = curtime.tv_sec + (wait_us / 1000000)
+						+ (n_sec / 1000000000);
+			timeout.tv_nsec = n_sec % 1000000000;
 
 			wait_ret = pthread_cond_timedwait(&p_event->condvar,
 							  &p_event->mutex,
-- 
1.5.4.5