On 17:38 Mon 23 Aug , Yevgeny Kliteynik wrote: > When calculating p_timer->timeout.tv_sec and p_timer->timeout.tv_nsec, > the carry was ignored, resulting in wrong value in p_timer->timeout.tv_sec, > and value > 10^9 in p_timer->timeout.tv_nsec (illegal value).
Nice catch. And I also see a similar issue in cl_timer_trim(). See a comments about the patch below. > > Signed-off-by: Yevgeny Kliteynik <[email protected]> > > --- > opensm/complib/cl_timer.c | 8 +++++--- > 1 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/opensm/complib/cl_timer.c b/opensm/complib/cl_timer.c > index 2acdb51..09a5584 100644 > --- a/opensm/complib/cl_timer.c > +++ b/opensm/complib/cl_timer.c > @@ -299,7 +299,8 @@ cl_status_t cl_timer_start(IN cl_timer_t * const p_timer, > { > struct timeval curtime; > cl_list_item_t *p_list_item; > - uint32_t delta_time = time_ms; > + uint32_t delta_time_sec = time_ms / 1000; > + uint32_t delta_time_usec = (time_ms % 1000) * 1000; In order to prevent compatibility and porting issues would it be better to use struct timeval's types here: time_t delta_time_sec; suseconds_t delta_time_usec_t delta_time_usec; , instead of fixed size integers? > > CL_ASSERT(p_timer); > CL_ASSERT(p_timer->state == CL_INITIALIZED); > @@ -324,9 +325,10 @@ cl_status_t cl_timer_start(IN cl_timer_t * const p_timer, > /* if (delta_time < 1000.0) {delta_time = 1000;} */ > > /* Calculate the timeout. */ > - p_timer->timeout.tv_sec = curtime.tv_sec + (delta_time / 1000); > + p_timer->timeout.tv_sec = curtime.tv_sec + delta_time_sec + > + ((curtime.tv_usec + delta_time_usec) / 1000000); > p_timer->timeout.tv_nsec = > - (curtime.tv_usec + ((delta_time % 1000) * 1000)) * 1000; > + ((curtime.tv_usec + delta_time_usec) % 1000000) * 1000; Or even something like this: strcut timeval curtime, deltatime; gettimeofday(&curtime, NULL); addtime.tv_sec = time_ms / 1000; addtime.tv_usec = (time_ms % 1000) * 1000; timeradd(&curtime, &addtime, &timer->timeout); ? Sasha > > /* Add the timer to the queue. */ > if (cl_is_qlist_empty(&gp_timer_prov->queue)) { > -- > 1.6.2.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to [email protected] > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
