Re: [Y2038] [PATCH] scsi: bfa: Remove struct timeval

[Arnd Bergmann <arnd@arndb.de>]

On Sunday 01 November 2015 19:13:46 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 ktime_get_real_seconds() which returns a 64 bit seconds value.
> 
> This patch also replaces u32 timestamp with u64 timestamp in
> bfi_ioc_ctrl_req_s structure to store the 64 bit seconds value
> returned by ktime_get_real_seconds()
> 
> Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
> ---

This one is also wrong, but for other reasons as the earlier
patches:

> @@ -1808,13 +1809,11 @@ static void
>  bfa_ioc_send_enable(struct bfa_ioc_s *ioc)
>  {
>  	struct bfi_ioc_ctrl_req_s enable_req;
> -	struct timeval tv;
>  
>  	bfi_h2i_set(enable_req.mh, BFI_MC_IOC, BFI_IOC_H2I_ENABLE_REQ,
>  		    bfa_ioc_portid(ioc));
>  	enable_req.clscode = cpu_to_be16(ioc->clscode);
> -	do_gettimeofday(&tv);
> -	enable_req.tv_sec = be32_to_cpu(tv.tv_sec);
> +	enable_req.tv_sec = be64_to_cpu(ktime_get_real_seconds());
>  	bfa_ioc_mbox_send(ioc, &enable_req, sizeof(struct bfi_ioc_ctrl_req_s));
>  }
>  
> diff --git a/drivers/scsi/bfa/bfi.h b/drivers/scsi/bfa/bfi.h
> index 9ef91f9..aaa68bc 100644
> --- a/drivers/scsi/bfa/bfi.h
> +++ b/drivers/scsi/bfa/bfi.h
> @@ -454,7 +454,7 @@ struct bfi_ioc_ctrl_req_s {
>  	struct bfi_mhdr_s	mh;
>  	u16			clscode;
>  	u16			rsvd;
> -	u32		tv_sec;
> +	u64			tv_sec;
>  };
>  #define bfi_ioc_enable_req_t struct bfi_ioc_ctrl_req_s;
>  #define bfi_ioc_disable_req_t struct bfi_ioc_ctrl_req_s;

Here, you are dealing with firmware data structure, so that gets interpreted
by code running on the SCSI host, and changing the structure layout breaks
the interface.

I would add a comment here that describes how and when the time will still
overflow, and add a cast to (u32). Please also describe this in the
patch description.

You really don't need to list every single change you do in the changelog,
but instead should describe what problems are with the original code and
why your change is done this way.

	Arnd