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

Groups > linux.kernel > #1253317 > unrolled thread

[PATCH v2] firewire: Replace timeval with timespec64

Back to article view | Back to linux.kernel

Contents

  [PATCH v2] firewire: Replace timeval with timespec64 Amitoj Kaur Chawla <amitoj1606@gmail.com> - 2015-10-22 00:40 +0200
    Re: [Outreachy kernel] [PATCH v2] firewire: Replace timeval with timespec64 Arnd Bergmann <arnd@arndb.de> - 2015-10-22 01:00 +0200
      Re: [PATCH v2] firewire: Replace timeval with timespec64 Arnd Bergmann <arnd@arndb.de> - 2015-10-22 15:20 +0200
      Re: [PATCH v2] firewire: Replace timeval with timespec64 Stefan Richter <stefanr@s5r6.in-berlin.de> - 2015-10-22 15:20 +0200

#1253317 — [PATCH v2] firewire: 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>
---
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

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

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;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1253796

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1253798

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/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web