On Thu, Nov 5, 2015 at 5:19 AM, Arnd Bergmann <[email protected]> wrote:
> On Wednesday 04 November 2015 23:51:07 Amitoj Kaur Chawla wrote:
>> 32 bit systems using 'struct timeval' will break in the year 2038, so
>> we modify the code appropriately.
>>
>> This patch replaces the use of struct timeval and do_gettimeofday()
>> with 64 bit ktime_get_seconds() since we only need to find elapsed
>> seconds.
>>
>> stats_reset_time variable has been converted from a u32 to u64 type to
>> store the 64 bit value returned by ktime_get_seconds().
>>
>> Signed-off-by: Amitoj Kaur Chawla <[email protected]>
>
> The changelog looks good, but you seem to have introduced a new
> bug:
>
>> @@ -3347,9 +3345,8 @@ __bfa_cb_fcport_stats_get(void *cbarg, bfa_boolean_t
>> complete)
>> union bfa_fcport_stats_u *ret;
>>
>> if (complete) {
>> - struct timeval tv;
>> + u64 timestamp = ktime_get_seconds();
>> if (fcport->stats_status == BFA_STATUS_OK)
>> - do_gettimeofday(&tv);
>>
>> list_for_each_safe(qe, qen, &fcport->stats_pending_q) {
>> bfa_q_deq(&fcport->stats_pending_q, &qe);
>
> The 'if' condition now applies to the list_for_each_safe(), so the
> behavior of the code changes significantly.
>
I'll remove the if condition totally since it doesn't make sense
anymore to keep it.
>> diff --git a/drivers/scsi/bfa/bfa_svc.h b/drivers/scsi/bfa/bfa_svc.h
>> index ef07365..0c25ee3 100644
>> --- a/drivers/scsi/bfa/bfa_svc.h
>> +++ b/drivers/scsi/bfa/bfa_svc.h
>> @@ -504,7 +504,7 @@ struct bfa_fcport_s {
>> struct list_head stats_pending_q;
>> struct list_head statsclr_pending_q;
>> bfa_boolean_t stats_qfull;
>> - u32 stats_reset_time; /* stats reset time stamp */
>> + u64 stats_reset_time; /* stats reset time stamp */
>> bfa_boolean_t diag_busy; /* diag busy status */
>> bfa_boolean_t beacon; /* port beacon status */
>> bfa_boolean_t link_e2e_beacon; /* link beacon status */
>
> Let's talk about types here: u64 is technically correct, but the
> return type of ktime_get_seconds() is 'time64_t', so if you want to
> stay with 64 bits just use that.
>
I'll use time64_t next time.
> Leaving the type as u32 is also fine as I explained earlier because
> monotonic times do not overflow 32-bit variables in practice.
>
> Arnd
Oh okay, I'll leave stats_reset_time as is.
--
Amitoj
_______________________________________________
Y2038 mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/y2038
