RE: [PATCH] IB/hfi1: Fix a wrapping test (make it less strict)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> Subject: Re: [PATCH] IB/hfi1: Fix a wrapping test (make it less strict)
> 
> On Mon, Oct 23, 2017 at 02:33:46PM -0400, Doug Ledford wrote:
> > > > > @@ -3781,7 +3781,7 @@ static int
> > > > > __subn_get_opa_hfi1_cong_log(struct
> > > > > opa_smp *smp, u32 am,
> > > > >   		 * required to wrap the counter are supposed to
> > > > >   		 * be zeroed (CA10-49 IBTA, release 1.2.1, V1).
> > > > >   		 */
> > > > > -		if ((u64)(ts - cce->timestamp) > (2 * UINT_MAX))
> > > > > +		if ((u64)(ts - cce->timestamp) > (2ULL *
> > > > > UINT_MAX))
> 
> This is really weird looking. Both ts and cce->timestamp are s64, why
> do the convoluted conversion to unsigned? And surely UINT_MAX is not
> the right thing..
> 
> if ((ts - cce->timestamp)/2 > 0xFFFFFFFF)
> 
> ?
> 
> ktime_get is defined to be monotonic, so ts - cce->timestamp should
> never go negative.
> 

I agree that this is an issue.

My proposal:
- Change s64 to u64 for ts and in timestamp in opa_hfi1_cong_log_event_internal
- Change the calls:
        ktime_to_ns(ktime_get()) / 1024
        -- to --
        ktime_get_ns() / 1024
- Change to use Jason's test from above with UINT_MAX

Dan, we can do the patch or you can send a v2?

Mike

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux