[PATCH v2] firewire: Replace timeval with timespec64

[PATCH v2] firewire: Replace timeval with timespec64

[Amitoj Kaur Chawla]

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>
---
Changes in v2:
        -Replaced timespec with timspec64
        -Modified commit message
        -Used ktime_get_real_ts64() instead of getnstimeofday64()

 drivers/firewire/nosy.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/firewire/nosy.c b/drivers/firewire/nosy.c
index 76b2d39..8a46077 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;
-- 
1.9.1

Re: [Outreachy kernel] [PATCH v2] firewire: Replace timeval with timespec64

[Arnd Bergmann]

On Thursday 22 October 2015 04:05:00 Amitoj Kaur Chawla wrote:
> 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>

(adding the y2038 mailing list as well)

> Changes in v2:
>         -Replaced timespec with timspec64
>         -Modified commit message
>         -Used ktime_get_real_ts64() instead of getnstimeofday64()
> 
>  drivers/firewire/nosy.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/firewire/nosy.c b/drivers/firewire/nosy.c
> index 76b2d39..8a46077 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;
> 

Re: [PATCH v2] firewire: Replace timeval with timespec64

[Stefan Richter]

On Oct 22 Arnd Bergmann wrote:
> On Thursday 22 October 2015 04:05:00 Amitoj Kaur Chawla wrote:
[...]
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> 
> (adding the y2038 mailing list as well)
> 
> > Changes in v2:
> >         -Replaced timespec with timspec64
> >         -Modified commit message
> >         -Used ktime_get_real_ts64() instead of getnstimeofday64()
[...]
> > --- a/drivers/firewire/nosy.c
> > +++ b/drivers/firewire/nosy.c
[...]
> > @@ -413,17 +414,18 @@ static void
> >  packet_irq_handler(struct pcilynx *lynx)
[...]
> > -	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;

Looks fine to me, but I have a question.  It was possibly already
discussed at patch v1, though that was apparently not posted to an open
list.

include/linux/timekeeping.h says:
#define ktime_get_real_ts64(ts) getnstimeofday64(ts)

kernel/time/timekeeping.c says:
/**
  * do_gettimeofday - Returns the time of day in a timeval
  * @tv:         pointer to the timeval to be set
  *
  * NOTE: Users should be converted to using getnstimeofday()
  */

So what is the reason for calling ktime_get_real_ts64() instead of
getnstimeofday[64]()?

PS, note to self:
Independently of this patch, I need to check whether CLOCK_REALTIME was
really the right clock here, in contrast to CLOCK_MONOTONIC.
-- 
Stefan Richter
-=====-===== =-=- =-==-
http://arcgraph.de/sr/

Re: [PATCH v2] firewire: Replace timeval with timespec64

[Arnd Bergmann]

On Thursday 22 October 2015 15:07:50 Stefan Richter wrote:
> Looks fine to me, but I have a question.  It was possibly already
> discussed at patch v1, though that was apparently not posted to an open
> list.
> 
> include/linux/timekeeping.h says:
> #define ktime_get_real_ts64(ts) getnstimeofday64(ts)
> 
> kernel/time/timekeeping.c says:
> /**
>   * do_gettimeofday - Returns the time of day in a timeval
>   * @tv:         pointer to the timeval to be set
>   *
>   * NOTE: Users should be converted to using getnstimeofday()
>   */
> 
> So what is the reason for calling ktime_get_real_ts64() instead of
> getnstimeofday[64]()?

They are identical in behavior, I don't know exactly why we have two
but I'm advocating the move to ktime_* functions for consistency
with ktime_get_seconds(), ktime_get_ns() and ktime_get() that don't
have another alias. Once all users of the old getnstimeofday()
and do_gettimeofday() have been converted, I might change over all
users of getnstimeofday64() to ktime_get_real_ts64() and remove
all of the get*timeofday*() family.
 
> PS, note to self:
> Independently of this patch, I need to check whether CLOCK_REALTIME was
> really the right clock here, in contrast to CLOCK_MONOTONIC.

Yes, good idea. I usually recommend changing to montonic time
(ktime_get_ts64()) in the same patch, but in this particular case that
would have been a user-visible change that we should not mix in
to a single commit.

	Arnd

Re: [Outreachy kernel] [PATCH v2] firewire: Replace timeval with timespec64

[Stefan Richter]

[-- Attachment #1: Type: text/plain, Size: 2335 bytes --]

On Oct 22 Arnd Bergmann wrote:
> On Thursday 22 October 2015 04:05:00 Amitoj Kaur Chawla wrote:
> > 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>
> 
> (adding the y2038 mailing list as well)

Committed to linux1394.git.
Arnd, do you have special y2038 plans that make it desirable to have this
merged before v4.4?  Otherwise I would submit it for v4.5-rc1.

> > Changes in v2:
> >         -Replaced timespec with timspec64
> >         -Modified commit message
> >         -Used ktime_get_real_ts64() instead of getnstimeofday64()
> > 
> >  drivers/firewire/nosy.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/firewire/nosy.c b/drivers/firewire/nosy.c
> > index 76b2d39..8a46077 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;
> > 
> 

-- 
Stefan Richter
-=====-===== =-== --=-=
http://arcgraph.de/sr/

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Outreachy kernel] [PATCH v2] firewire: Replace timeval with timespec64

[Arnd Bergmann]

On Thursday 05 November 2015 14:34:48 Stefan Richter wrote:
> On Oct 22 Arnd Bergmann wrote:
> > On Thursday 22 October 2015 04:05:00 Amitoj Kaur Chawla wrote:
> > > 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>
> > 
> > (adding the y2038 mailing list as well)
> 
> Committed to linux1394.git.
> Arnd, do you have special y2038 plans that make it desirable to have this
> merged before v4.4?  Otherwise I would submit it for v4.5-rc1.

4.5-rc1 is fine.

	Arnd