Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1361526 > unrolled thread

[PATCH] firewire: nosy: Replace timeval with timespec64

Back to article view | Back to linux.kernel

Contents

  [PATCH] firewire: nosy: Replace timeval with timespec64 Tina Ruchandani <ruchandani.tina@gmail.com> - 2016-03-21 07:00 +0100
    Re: [PATCH] firewire: nosy: Replace timeval with timespec64 Arnd Bergmann <arnd@arndb.de> - 2016-03-21 21:00 +0100
      Re: [PATCH] firewire: nosy: Replace timeval with timespec64 Stefan Richter <stefanr@s5r6.in-berlin.de> - 2016-03-21 21:20 +0100
        Re: [PATCH] firewire: nosy: Replace timeval with timespec64 Arnd Bergmann <arnd@arndb.de> - 2016-03-21 22:40 +0100
          Re: [PATCH] firewire: nosy: Replace timeval with timespec64 Stefan Richter <stefanr@s5r6.in-berlin.de> - 2016-03-22 15:30 +0100

#1361526 — [PATCH] firewire: nosy: Replace timeval with timespec64

'struct timeval' uses a 32 bit field for its 'seconds' value which
will overflow in year 2038 and beyond. This patch replaces the use
of timeval in nosy.c with timespec64 which doesn't suffer from y2038
issue. The code is correct as is - since it is only using the
microseconds portion of timeval. However, this patch does the
replacement as part of a larger effort to remove all instances of
'struct timeval' from the kernel (that would help identify cases
where the code is actually broken).

Signed-off-by: Tina Ruchandani <ruchandani.tina@gmail.com>
---
 drivers/firewire/nosy.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/firewire/nosy.c b/drivers/firewire/nosy.c
index 8a46077..631c977 100644
--- a/drivers/firewire/nosy.c
+++ b/drivers/firewire/nosy.c
@@ -446,14 +446,16 @@ static void
 bus_reset_irq_handler(struct pcilynx *lynx)
 {
 	struct client *client;
-	struct timeval tv;
+	struct timespec64 ts64;
+	u32    timestamp;

-	do_gettimeofday(&tv);
+	ktime_get_real_ts64(&ts64);
+	timestamp = ts64.tv_nsec / NSEC_PER_USEC;

 	spin_lock(&lynx->client_list_lock);

 	list_for_each_entry(client, &lynx->client_list, link)
-		packet_buffer_put(&client->buffer, &tv.tv_usec, 4);
+		packet_buffer_put(&client->buffer, &timestamp, 4);

 	spin_unlock(&lynx->client_list_lock);
 }
--
2.8.0.rc3.226.g39d4020

[toc] | [next] | [standalone]

#1362123

On Sunday 20 March 2016 22:59:11 Tina Ruchandani wrote:
> 'struct timeval' uses a 32 bit field for its 'seconds' value which
> will overflow in year 2038 and beyond. This patch replaces the use
> of timeval in nosy.c with timespec64 which doesn't suffer from y2038
> issue. The code is correct as is - since it is only using the
> microseconds portion of timeval. However, this patch does the
> replacement as part of a larger effort to remove all instances of
> 'struct timeval' from the kernel (that would help identify cases
> where the code is actually broken).
> 
> Signed-off-by: Tina Ruchandani <ruchandani.tina@gmail.com>
> ---
>  drivers/firewire/nosy.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)

The patch looks correct to me, but it seems the same one has
just been merged into mainline Linux on Saturday (the patch
was posted back in October).

commit 2ae4b6b20e2004dccf80d804ae52b073377c2f5b
Author: Amitoj Kaur Chawla <amitoj1606@gmail.com>
Date:   Thu Oct 22 04:05:00 2015 +0530

    firewire: nosy: Replace timeval with timespec64
    
    32 bit systems using 'struct timeval' will break in the year 2038, so
    we replace the code appropriately. However, this driver is not broken
    in 2038 since we are using only the microseconds portion of the
    current time.
    
    This patch replaces timeval with timespec64.
    
    Signed-off-by: Amitoj Kaur Chawla <amitoj1606@gmail.com>
    Reviewed-by: Arnd Bergmann <arnd@arndb.de>
    Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

diff --git a/drivers/firewire/nosy.c b/drivers/firewire/nosy.c
index 76b2d390f6ec..8a46077129ac 100644
--- a/drivers/firewire/nosy.c
+++ b/drivers/firewire/nosy.c
@@ -33,6 +33,7 @@
 #include <linux/sched.h> /* required for linux/wait.h */
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/time64.h>
 #include <linux/timex.h>
 #include <linux/uaccess.h>
 #include <linux/wait.h>
@@ -413,17 +414,18 @@ static void
 packet_irq_handler(struct pcilynx *lynx)
 {
 	struct client *client;
-	u32 tcode_mask, tcode;
+	u32 tcode_mask, tcode, timestamp;
 	size_t length;
-	struct timeval tv;
+	struct timespec64 ts64;
 
 	/* FIXME: Also report rcv_speed. */
 
 	length = __le32_to_cpu(lynx->rcv_pcl->pcl_status) & 0x00001fff;
 	tcode  = __le32_to_cpu(lynx->rcv_buffer[1]) >> 4 & 0xf;
 
-	do_gettimeofday(&tv);
-	lynx->rcv_buffer[0] = (__force __le32)tv.tv_usec;
+	ktime_get_real_ts64(&ts64);
+	timestamp = ts64.tv_nsec / NSEC_PER_USEC;
+	lynx->rcv_buffer[0] = (__force __le32)timestamp;
 
 	if (length == PHY_PACKET_SIZE)
 		tcode_mask = 1 << TCODE_PHY_PACKET;

[toc] | [prev] | [next] | [standalone]

#1362133

On Mar 21 Arnd Bergmann wrote:
> On Sunday 20 March 2016 22:59:11 Tina Ruchandani wrote:
> > 'struct timeval' uses a 32 bit field for its 'seconds' value which
> > will overflow in year 2038 and beyond. This patch replaces the use
> > of timeval in nosy.c with timespec64 which doesn't suffer from y2038
> > issue. The code is correct as is - since it is only using the
> > microseconds portion of timeval. However, this patch does the
> > replacement as part of a larger effort to remove all instances of
> > 'struct timeval' from the kernel (that would help identify cases
> > where the code is actually broken).
> > 
> > Signed-off-by: Tina Ruchandani <ruchandani.tina@gmail.com>
> > ---
> >  drivers/firewire/nosy.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)  
> 
> The patch looks correct to me, but it seems the same one has
> just been merged into mainline Linux on Saturday (the patch
> was posted back in October).
> 
> commit 2ae4b6b20e2004dccf80d804ae52b073377c2f5b
[...]

No, Amitoj's patch from October changed nosy.c::packet_irq_hander, whereas
Tina' patch changes nosy.c::bus_reset_irq_handler.  IOW the new patch
completes what the former patch (and us reviewers) missed.
-- 
Stefan Richter
-======----- --== =-=-=
http://arcgraph.de/sr/

[toc] | [prev] | [next] | [standalone]

#1362165

On Monday 21 March 2016 21:19:17 Stefan Richter wrote:
> On Mar 21 Arnd Bergmann wrote:
> > On Sunday 20 March 2016 22:59:11 Tina Ruchandani wrote:
> > > 'struct timeval' uses a 32 bit field for its 'seconds' value which
> > > will overflow in year 2038 and beyond. This patch replaces the use
> > > of timeval in nosy.c with timespec64 which doesn't suffer from y2038
> > > issue. The code is correct as is - since it is only using the
> > > microseconds portion of timeval. However, this patch does the
> > > replacement as part of a larger effort to remove all instances of
> > > 'struct timeval' from the kernel (that would help identify cases
> > > where the code is actually broken).
> > > 
> > > Signed-off-by: Tina Ruchandani <ruchandani.tina@gmail.com>
> > > ---
> > >  drivers/firewire/nosy.c | 8 +++++---
> > >  1 file changed, 5 insertions(+), 3 deletions(-)  
> > 
> > The patch looks correct to me, but it seems the same one has
> > just been merged into mainline Linux on Saturday (the patch
> > was posted back in October).
> > 
> > commit 2ae4b6b20e2004dccf80d804ae52b073377c2f5b
> [...]
> 
> No, Amitoj's patch from October changed nosy.c::packet_irq_hander, whereas
> Tina' patch changes nosy.c::bus_reset_irq_handler.  IOW the new patch
> completes what the former patch (and us reviewers) missed.
> 

Ah, I see. Sorry for my confusion.

Reviewed-by: Arnd Bergmann <arnd@arndb.de>

[toc] | [prev] | [next] | [standalone]

#1362769

On Mar 21 Arnd Bergmann wrote:
>>> On Sunday 20 March 2016 22:59:11 Tina Ruchandani wrote:  
>>>> 'struct timeval' uses a 32 bit field for its 'seconds' value which
>>>> will overflow in year 2038 and beyond. This patch replaces the use
>>>> of timeval in nosy.c with timespec64 which doesn't suffer from y2038
>>>> issue. The code is correct as is - since it is only using the
>>>> microseconds portion of timeval. However, this patch does the
>>>> replacement as part of a larger effort to remove all instances of
>>>> 'struct timeval' from the kernel (that would help identify cases
>>>> where the code is actually broken).
>>>> 
>>>> Signed-off-by: Tina Ruchandani <ruchandani.tina@gmail.com>
[...]
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>

Committed to linux1394.git.
I will try to get this pulled upstream in the next few days.
-- 
Stefan Richter
-======----- --== =-==-
http://arcgraph.de/sr/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web