On Thu, Nov 5, 2015 at 8:52 PM, Arnd Bergmann <[email protected]> wrote: > On Thursday 05 November 2015 20:43:02 Amitoj Kaur Chawla wrote: >> On Thu, Nov 5, 2015 at 8:31 PM, Arnd Bergmann <[email protected]> wrote: >> > On Thursday 05 November 2015 19:00:36 Amitoj Kaur Chawla wrote: >> >> On Thu, Nov 5, 2015 at 2:44 PM, Arnd Bergmann <[email protected]> wrote: >> >> > That works, but why not keep the code as it is here? ktime_get_seconds() >> >> > still adds a little overhead if we call it unconditionally, and the >> >> > author of the original code apparently felt that it was worth optimizing >> >> > for. >> >> > >> >> >> >> Oh okay, I sent a v2 thinking that the removal of if condition would >> >> work. But since you're saying that the optimising seems necessary. >> >> I'll change it to >> >> u64 timestamp; >> >> if (fcport->stats_status == BFA_STATUS_OK) >> >> timestamp = ktime_get_seconds(); >> >> >> >> Is this fine? >> > >> > Yes, that looks ok. I'd probably use 'time64_t' instead of 'u64', and >> > 'long' would also work just as well here because it is monotonic time. >> > >> > Regarding the optimization, I think it is not an important one and it >> > could indeed be removed, but only if you can prove that the code without >> > the optimization is as efficient in practice, and explain that in >> > the changelog text. It's usually easier not to change it then, so you >> > don't have to spend the time figuring out whether that is an improvement >> > or not. >> >> Now, it gives me a warning since the if condition makes it a >> possibility that timestamp may be uninitialised for the later >> operation in the code. >> >> Not understanding, shouldn't it have given a warning when declared >> also, as 'struct timeval tv' since do_gettimeofday() was also >> intialising 'tv' only when the if condition held true. > > This is an unreliable warning from gcc, and it sometimes gets > things wrong here for no clear reason. It probably cannot prove > that fcport->stats_status has not been modified between the > two checks. > > > Make sure that the code is actually correct. You can probably avoid > the warning by introducing a local variable like > > bool status_ok = (fcport->stats_status == BFA_STATUS_OK); > if (status_ok) > timestamp = ktime_get_seconds(); > ... > > > Arnd
Oh okay. Thanks, -- Amitoj _______________________________________________ Y2038 mailing list [email protected] https://lists.linaro.org/mailman/listinfo/y2038
