scsi: Remove 'struct timeval'
Subsystems (3)
cover scsi: Remove 'struct timeval'
This patchset removes 'struct timeval' as 32 bit systems using timeval will break in the year 2038 so the code has been replaced appropriately.
Amitoj Kaur Chawla (2):
scsi: 3w-9xxx: Remove use of struct timeval
scsi: bfa: Replace struct timeval with ktime_t
drivers/scsi/3w-9xxx.c | 16 ++++++---------- drivers/scsi/bfa/bfa_defs_svc.h | 2 +- drivers/scsi/bfa/bfad_im.h | 2 +- 3 files changed, 8 insertions(+), 12 deletions(-)
Signature · 1 line
1.9.1
PATCH 1/2 scsi: 3w-9xxx: Remove use of struct timeval
This driver uses 'struct timeval' which we are trying to remove since 32 bit time types will break in the year 2038.
This patch uses ktime_get() instead of do_gettimeofday() which returns ktime_t type.
This patch also uses ktime_to_ms() to find current time in milliseconds.
--- drivers/scsi/3w-9xxx.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)
drivers/scsi/3w-9xxx.c +6 -10
@@ -90,7 +90,7 @@#include <linux/types.h> #include <linux/delay.h> #include <linux/pci.h> -#include <linux/time.h> +#include <linux/ktime.h> #include <linux/mutex.h> #include <linux/slab.h> #include <asm/io.h>
@@ -361,7 +361,6 @@ out:static void twa_aen_queue_event (TW_Device_Extension *tw_dev,TW_Command_Apache_Header *header) {u32 local_time; -struct timeval time;TW_Event *event;unsigned short aen;char host[16 ];
@@ -384,8 +383,8 @@ static void twa_aen_queue_event(TW_Device_Extension *tw_dev, TW_Command_Apache_Hmemset (event,0 ,sizeof (TW_Event)); event->severity =TW_SEV_OUT (header->status_block.severity__reserved); -do_gettimeofday (&time); - local_time = (u32)(time.tv_sec - (sys_tz.tz_minuteswest *60 )); + local_time = (u32)(ktime_get_real_seconds () - + (sys_tz.tz_minuteswest *60 )); event->time_stamp_sec = local_time; event->aen_code = aen; event->retrieved =TW_AEN_NOT_RETRIEVED ;
@@ -465,7 +464,6 @@ out:static void twa_aen_sync_time (TW_Device_Extension *tw_dev,int request_id) {u32 schedulertime; -struct timeval utc;TW_Command_Full *full_command_packet;TW_Command *command_packet;TW_Param_Apache *param;
@@ -491,8 +489,8 @@ static void twa_aen_sync_time(TW_Device_Extension *tw_dev, int request_id)/* Convert system time in UTC to local time seconds since last Sunday 12:00AM */ -do_gettimeofday (&utc); - local_time = (u32)(utc.tv_sec - (sys_tz.tz_minuteswest *60 )); + local_time = (u32)(ktime_get_real_seconds () - + (sys_tz.tz_minuteswest *60 )); schedulertime = local_time - (3 *86400 ); schedulertime =cpu_to_le32 (schedulertime %604800 );
@@ -640,7 +638,6 @@ static long twa_chrdev_ioctl(struct file *file, unsigned int cmd, unsigned longTW_Command_Full *full_command_packet;TW_Compatibility_Info *tw_compat_info;TW_Event *event; -struct timeval current_time;u32 current_time_ms;TW_Device_Extension *tw_dev = twa_device_extension_list[iminor (inode)];int retval =TW_IOCTL_ERROR_OS_EFAULT ;
@@ -832,8 +829,7 @@ static long twa_chrdev_ioctl(struct file *file, unsigned int cmd, unsigned longbreak ;case TW_IOCTL_GET_LOCK : tw_lock = (TW_Lock *)tw_ioctl->data_buffer; -do_gettimeofday (¤t_time); - current_time_ms = (current_time.tv_sec *1000 ) + (current_time.tv_usec /1000 ); + current_time_ms =ktime_to_ms (ktime_get ());if ((tw_lock->force_flag ==1 ) || (tw_dev->ioctl_sem_lock ==0 ) || (current_time_ms >= tw_dev->ioctl_msec)) { tw_dev->ioctl_sem_lock =1 ;
Signature · 1 line
1.9.1
Re: [Y2038] [PATCH 1/2] scsi: 3w-9xxx: Remove use of struct timeval
Quoted 8 lines · On Wednesday 28 October 2015 09:10:58 Amitoj Kaur Chawla wrote:
This driver uses 'struct timeval' which we are trying to remove since 32 bit time types will break in the year 2038.
This patch uses
ktime_get()instead ofdo_gettimeofday()which returnsktime_ttype.This patch also uses
ktime_to_ms()to find current time in milliseconds.
It looks like you are doing two or three things in this patch, so it would be better to send multiple patches. This driver is a little harder to do than some of the other 'simple' ones I have listed in my table.
drivers/scsi/3w-9xxx.c static void twa_aen_queue_event(TW_Device_Extension *tw_dev, TW_Command_Apache_H · line 390
Quoted 11 lines
drivers/scsi/3w-9xxx.c +2
-2@@ -384,8 +383,8 @@ static void twa_aen_queue_event(TW_Device_Extension *tw_dev, TW_Command_Apache_Hmemset (event,0 ,sizeof (TW_Event)); event->severity =TW_SEV_OUT (header->status_block.severity__reserved); -do_gettimeofday (&time); - local_time = (u32)(time.tv_sec - (sys_tz.tz_minuteswest *60 )); + local_time = (u32)(ktime_get_real_seconds () - + (sys_tz.tz_minuteswest *60 )); event->time_stamp_sec = local_time; event->aen_code = aen; event->retrieved =TW_AEN_NOT_RETRIEVED ;
Unlike what you write in the description, you use ktime_get_real_seconds() rather than ktime_get().
time_stamp_sec is limited to 32 bits, and you handle this correctly, but you do not describe in the changelog what causes the limitation and what this means for the overflow.
drivers/scsi/3w-9xxx.c static void twa_aen_sync_time(TW_Device_Extension *tw_dev, int request_id) · line 495
Quoted 10 lines
drivers/scsi/3w-9xxx.c +2
-2@@ -491,8 +489,8 @@ static void twa_aen_sync_time(TW_Device_Extension *tw_dev, int request_id)/* Convert system time in UTC to local time seconds since last Sunday 12:00AM */ -do_gettimeofday (&utc); - local_time = (u32)(utc.tv_sec - (sys_tz.tz_minuteswest *60 )); + local_time = (u32)(ktime_get_real_seconds () - + (sys_tz.tz_minuteswest *60 )); schedulertime = local_time - (3 *86400 ); schedulertime =cpu_to_le32 (schedulertime %604800 );
I think here you have the math wrong for the case that the 'u32' overflows. Either explain why that is harmless (if so), or use do_div() to get the modulo from the 64-bit seconds count.
drivers/scsi/3w-9xxx.c static long twa_chrdev_ioctl(struct file *file, unsigned int cmd, unsigned long · line 835
Quoted 11 lines
drivers/scsi/3w-9xxx.c +1
-2@@ -832,8 +829,7 @@ static long twa_chrdev_ioctl(struct file *file, unsigned int cmd, unsigned longbreak ;case TW_IOCTL_GET_LOCK : tw_lock = (TW_Lock *)tw_ioctl->data_buffer; -do_gettimeofday (¤t_time); - current_time_ms = (current_time.tv_sec *1000 ) + (current_time.tv_usec /1000 ); + current_time_ms =ktime_to_ms (ktime_get ());if ((tw_lock->force_flag ==1 ) || (tw_dev->ioctl_sem_lock ==0 ) || (current_time_ms >= tw_dev->ioctl_msec)) { tw_dev->ioctl_sem_lock =1 ;
The conversion looks good, but it seems there is a preexisting bug that happens every time the current_time_ms variable overflows, so the current time ends up being less than ioctl_msec. You could try to fix this using the time_before() function, when converting all the ms variables into 'unsigned long'.
Arnd
PATCH 2/2 scsi: bfa: Replace struct timeval with ktime_t
In reply to Amitoj Kaur ChawlaThis driver uses 'struct timeval' which we are trying to remove since 32 bit time types will break in the year 2038 by replacing it with ktime_t.
This patch changes do_gettimeofday() to ktime_get() because ktime_get() returns a ktime_t while do_gettimeofday() returns struct timeval.
--- drivers/scsi/bfa/bfa_defs_svc.h | 2 +- drivers/scsi/bfa/bfad_im.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/scsi/bfa/bfa_defs_svc.h +1 -1
@@ -1454,7 +1454,7 @@ struct bfa_aen_entry_s {enum bfa_aen_category aen_category;u32 aen_type;union bfa_aen_data_u aen_data; -struct timeval aen_tv; +ktime_t aen_time;u32 seq_num;u32 bfad_num; };
drivers/scsi/bfa/bfad_im.h +1 -1
@@ -131,7 +131,7 @@ struct bfad_im_s {/* post fc_host vendor event */ #define bfad_im_post_vendor_event (_entry, _drv, _cnt, _cat, _evt) do { \ - do_gettimeofday(&(_entry)->aen_tv); \ + (_entry)->aen_time = ktime_get(); \ (_entry)->bfad_num = (_drv)->inst_no; \ (_entry)->seq_num = (_cnt); \ (_entry)->aen_category = (_cat); \
Signature · 1 line
1.9.1
Re: [Y2038] [PATCH 2/2] scsi: bfa: Replace struct timeval with ktime_t
Quoted 21 lines · On Wednesday 28 October 2015 09:13:51 Amitoj Kaur Chawla wrote:
drivers/scsi/bfa/bfa_defs_svc.h +1
-1@@ -1454,7 +1454,7 @@ struct bfa_aen_entry_s {enum bfa_aen_category aen_category;u32 aen_type;union bfa_aen_data_u aen_data; -struct timeval aen_tv; +ktime_t aen_time;u32 seq_num;u32 bfad_num; };drivers/scsi/bfa/bfad_im.h +0
-0
drivers/scsi/bfa/bfad_im.h struct bfad_im_s { · line 137
Quoted 8 lines
drivers/scsi/bfa/bfad_im.h +1
-1@@ -131,7 +131,7 @@ struct bfad_im_s {/* post fc_host vendor event */ #define bfad_im_post_vendor_event (_entry, _drv, _cnt, _cat, _evt) do { \ - do_gettimeofday(&(_entry)->aen_tv); \ + (_entry)->aen_time = ktime_get(); \ (_entry)->bfad_num = (_drv)->inst_no; \ (_entry)->seq_num = (_cnt); \ (_entry)->aen_category = (_cat); \
This can't be right: you are only writing to the variable. If there is anyone reading from it elsewhere, that code needs to be changed as well in the same patch. If not, the variable can just be removed.
Arnd