All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] scsi: bfa: Remove struct timeval
@ 2015-11-01 13:43 Amitoj Kaur Chawla
  2015-11-03 23:43 ` [Y2038] " Arnd Bergmann
  0 siblings, 1 reply; 2+ messages in thread
From: Amitoj Kaur Chawla @ 2015-11-01 13:43 UTC (permalink / raw)
  To: outreachy-kernel; +Cc: y2038

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>
---
 drivers/scsi/bfa/bfa_ioc.c | 5 ++---
 drivers/scsi/bfa/bfi.h     | 2 +-
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_ioc.c b/drivers/scsi/bfa/bfa_ioc.c
index 98f7e8c..c52c3c3 100644
--- a/drivers/scsi/bfa/bfa_ioc.c
+++ b/drivers/scsi/bfa/bfa_ioc.c
@@ -15,6 +15,7 @@
  * General Public License for more details.
  */
 
+#include <linux/ktime.h>
 #include "bfad_drv.h"
 #include "bfad_im.h"
 #include "bfa_ioc.h"
@@ -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;
-- 
1.9.1



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [Y2038] [PATCH] scsi: bfa: Remove struct timeval
  2015-11-01 13:43 [PATCH] scsi: bfa: Remove struct timeval Amitoj Kaur Chawla
@ 2015-11-03 23:43 ` Arnd Bergmann
  0 siblings, 0 replies; 2+ messages in thread
From: Arnd Bergmann @ 2015-11-03 23:43 UTC (permalink / raw)
  To: y2038; +Cc: Amitoj Kaur Chawla, outreachy-kernel

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


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2015-11-03 23:43 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-01 13:43 [PATCH] scsi: bfa: Remove struct timeval Amitoj Kaur Chawla
2015-11-03 23:43 ` [Y2038] " Arnd Bergmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.