divamnt stores a start_time at init and uses it to calculate elapsed time. These operations are all internal to divamnt.
Address struct timeval overflow on 32-bit systems by replacing: struct timeval with struct timespec64; do_gettimeofday() with getnstimeofday64(); the calculations that yield a safe and normalized elapsed time with timespec64_sub() which provides same.
Signed-off-by: Alison Schofield amsfield22@gmail.com --- drivers/isdn/hardware/eicon/divamnt.c | 35 +++++++++++------------------------ 1 file changed, 11 insertions(+), 24 deletions(-)
diff --git a/drivers/isdn/hardware/eicon/divamnt.c b/drivers/isdn/hardware/eicon/divamnt.c index 48db08d..7ac057e 100644 --- a/drivers/isdn/hardware/eicon/divamnt.c +++ b/drivers/isdn/hardware/eicon/divamnt.c @@ -45,7 +45,7 @@ char *DRIVERRELEASE_MNT = "2.0";
static wait_queue_head_t msgwaitq; static unsigned long opened; -static struct timeval start_time; +static struct timespec64 start_time;
extern int mntfunc_init(int *, void **, unsigned long); extern void mntfunc_finit(void); @@ -88,28 +88,15 @@ int diva_os_copy_from_user(void *os_handle, void *dst, const void __user *src, */ void diva_os_get_time(dword *sec, dword *usec) { - struct timeval tv; - - do_gettimeofday(&tv); - - if (tv.tv_sec > start_time.tv_sec) { - if (start_time.tv_usec > tv.tv_usec) { - tv.tv_sec--; - tv.tv_usec += 1000000; - } - *sec = (dword) (tv.tv_sec - start_time.tv_sec); - *usec = (dword) (tv.tv_usec - start_time.tv_usec); - } else if (tv.tv_sec == start_time.tv_sec) { - *sec = 0; - if (start_time.tv_usec < tv.tv_usec) { - *usec = (dword) (tv.tv_usec - start_time.tv_usec); - } else { - *usec = 0; - } - } else { - *sec = (dword) tv.tv_sec; - *usec = (dword) tv.tv_usec; - } + struct timespec64 curr_time; + struct timespec64 delta; + + getnstimeofday64(&curr_time); + + delta = timespec64_sub(curr_time, start_time); + + *sec = (dword) delta.tv_sec; + *usec = (dword) (delta.tv_nsec / NSEC_PER_USEC); }
/* @@ -213,7 +200,7 @@ static int __init maint_init(void) int ret = 0; void *buffer = NULL;
- do_gettimeofday(&start_time); + getnstimeofday64(&start_time); init_waitqueue_head(&msgwaitq);
printk(KERN_INFO "%s\n", DRIVERNAME);
resending for review...
On Fri, Nov 20, 2015 at 10:23:17AM -0800, Alison Schofield wrote:
divamnt stores a start_time at init and uses it to calculate elapsed time. These operations are all internal to divamnt.
Address struct timeval overflow on 32-bit systems by replacing: struct timeval with struct timespec64; do_gettimeofday() with getnstimeofday64(); the calculations that yield a safe and normalized elapsed time with timespec64_sub() which provides same.
Signed-off-by: Alison Schofield amsfield22@gmail.com
drivers/isdn/hardware/eicon/divamnt.c | 35 +++++++++++------------------------ 1 file changed, 11 insertions(+), 24 deletions(-)
diff --git a/drivers/isdn/hardware/eicon/divamnt.c b/drivers/isdn/hardware/eicon/divamnt.c index 48db08d..7ac057e 100644 --- a/drivers/isdn/hardware/eicon/divamnt.c +++ b/drivers/isdn/hardware/eicon/divamnt.c @@ -45,7 +45,7 @@ char *DRIVERRELEASE_MNT = "2.0"; static wait_queue_head_t msgwaitq; static unsigned long opened; -static struct timeval start_time; +static struct timespec64 start_time; extern int mntfunc_init(int *, void **, unsigned long); extern void mntfunc_finit(void); @@ -88,28 +88,15 @@ int diva_os_copy_from_user(void *os_handle, void *dst, const void __user *src, */ void diva_os_get_time(dword *sec, dword *usec) {
- struct timeval tv;
- do_gettimeofday(&tv);
- if (tv.tv_sec > start_time.tv_sec) {
if (start_time.tv_usec > tv.tv_usec) {tv.tv_sec--;tv.tv_usec += 1000000;}*sec = (dword) (tv.tv_sec - start_time.tv_sec);*usec = (dword) (tv.tv_usec - start_time.tv_usec);- } else if (tv.tv_sec == start_time.tv_sec) {
*sec = 0;if (start_time.tv_usec < tv.tv_usec) {*usec = (dword) (tv.tv_usec - start_time.tv_usec);} else {*usec = 0;}- } else {
*sec = (dword) tv.tv_sec;*usec = (dword) tv.tv_usec;- }
- struct timespec64 curr_time;
- struct timespec64 delta;
- getnstimeofday64(&curr_time);
- delta = timespec64_sub(curr_time, start_time);
- *sec = (dword) delta.tv_sec;
- *usec = (dword) (delta.tv_nsec / NSEC_PER_USEC);
} /* @@ -213,7 +200,7 @@ static int __init maint_init(void) int ret = 0; void *buffer = NULL;
- do_gettimeofday(&start_time);
- getnstimeofday64(&start_time); init_waitqueue_head(&msgwaitq);
printk(KERN_INFO "%s\n", DRIVERNAME); -- 2.1.4
On Mon, Jan 4, 2016 at 11:27 AM, Alison Schofield amsfield22@gmail.com wrote:
resending for review...
On Fri, Nov 20, 2015 at 10:23:17AM -0800, Alison Schofield wrote:
divamnt stores a start_time at init and uses it to calculate elapsed time. These operations are all internal to divamnt.
Address struct timeval overflow on 32-bit systems by replacing: struct timeval with struct timespec64; do_gettimeofday() with getnstimeofday64(); the calculations that yield a safe and normalized elapsed time with timespec64_sub() which provides same.
Signed-off-by: Alison Schofield amsfield22@gmail.com
drivers/isdn/hardware/eicon/divamnt.c | 35
+++++++++++------------------------
1 file changed, 11 insertions(+), 24 deletions(-)
diff --git a/drivers/isdn/hardware/eicon/divamnt.c
b/drivers/isdn/hardware/eicon/divamnt.c
index 48db08d..7ac057e 100644 --- a/drivers/isdn/hardware/eicon/divamnt.c +++ b/drivers/isdn/hardware/eicon/divamnt.c @@ -45,7 +45,7 @@ char *DRIVERRELEASE_MNT = "2.0";
static wait_queue_head_t msgwaitq; static unsigned long opened; -static struct timeval start_time; +static struct timespec64 start_time;
extern int mntfunc_init(int *, void **, unsigned long); extern void mntfunc_finit(void); @@ -88,28 +88,15 @@ int diva_os_copy_from_user(void *os_handle, void
*dst, const void __user *src,
*/ void diva_os_get_time(dword *sec, dword *usec) {
struct timeval tv;do_gettimeofday(&tv);if (tv.tv_sec > start_time.tv_sec) {if (start_time.tv_usec > tv.tv_usec) {tv.tv_sec--;tv.tv_usec += 1000000;}*sec = (dword) (tv.tv_sec - start_time.tv_sec);*usec = (dword) (tv.tv_usec - start_time.tv_usec);} else if (tv.tv_sec == start_time.tv_sec) {*sec = 0;if (start_time.tv_usec < tv.tv_usec) {*usec = (dword) (tv.tv_usec - start_time.tv_usec);} else {*usec = 0;}} else {*sec = (dword) tv.tv_sec;*usec = (dword) tv.tv_usec;}
timespec64_sub(lhs, rhs) assumes lhs > rhs always.
But, the original code doesn't seem to think this is always the case because of the else condition. The first 2 cases where start_time < curr_time, start_time == curr_time are handled in the change. But, think you have missed the case when start_time > curr_time?
struct timespec64 curr_time;struct timespec64 delta;getnstimeofday64(&curr_time);delta = timespec64_sub(curr_time, start_time);*sec = (dword) delta.tv_sec;*usec = (dword) (delta.tv_nsec / NSEC_PER_USEC);}
Here dword is defined as u32. Since this is only a difference in times, this might be warranted. But, the else part above might not fit in u32 because now you use 64 bit time. Maybe commit text should mention why this is still valid?
Since we are only concerned about difference monotonic times also should be okay.
On Tue, Jan 05, 2016 at 08:27:09AM -0800, Deepa Dinamani wrote:
On Mon, Jan 4, 2016 at 11:27 AM, Alison Schofield amsfield22@gmail.com wrote:
resending for review...
On Fri, Nov 20, 2015 at 10:23:17AM -0800, Alison Schofield wrote:
divamnt stores a start_time at init and uses it to calculate elapsed time. These operations are all internal to divamnt.
Address struct timeval overflow on 32-bit systems by replacing: struct timeval with struct timespec64; do_gettimeofday() with getnstimeofday64(); the calculations that yield a safe and normalized elapsed time with timespec64_sub() which provides same.
Signed-off-by: Alison Schofield amsfield22@gmail.com
drivers/isdn/hardware/eicon/divamnt.c | 35
+++++++++++------------------------
1 file changed, 11 insertions(+), 24 deletions(-)
diff --git a/drivers/isdn/hardware/eicon/divamnt.c
b/drivers/isdn/hardware/eicon/divamnt.c
index 48db08d..7ac057e 100644 --- a/drivers/isdn/hardware/eicon/divamnt.c +++ b/drivers/isdn/hardware/eicon/divamnt.c @@ -45,7 +45,7 @@ char *DRIVERRELEASE_MNT = "2.0";
static wait_queue_head_t msgwaitq; static unsigned long opened; -static struct timeval start_time; +static struct timespec64 start_time;
extern int mntfunc_init(int *, void **, unsigned long); extern void mntfunc_finit(void); @@ -88,28 +88,15 @@ int diva_os_copy_from_user(void *os_handle, void
*dst, const void __user *src,
*/ void diva_os_get_time(dword *sec, dword *usec) {
struct timeval tv;do_gettimeofday(&tv);if (tv.tv_sec > start_time.tv_sec) {if (start_time.tv_usec > tv.tv_usec) {tv.tv_sec--;tv.tv_usec += 1000000;}*sec = (dword) (tv.tv_sec - start_time.tv_sec);*usec = (dword) (tv.tv_usec - start_time.tv_usec);} else if (tv.tv_sec == start_time.tv_sec) {*sec = 0;if (start_time.tv_usec < tv.tv_usec) {*usec = (dword) (tv.tv_usec - start_time.tv_usec);} else {*usec = 0;}} else {*sec = (dword) tv.tv_sec;*usec = (dword) tv.tv_usec;}timespec64_sub(lhs, rhs) assumes lhs > rhs always.
But, the original code doesn't seem to think this is always the case because of the else condition. The first 2 cases where start_time < curr_time, start_time == curr_time are handled in the change. But, think you have missed the case when start_time > curr_time?
struct timespec64 curr_time;struct timespec64 delta;getnstimeofday64(&curr_time);delta = timespec64_sub(curr_time, start_time);*sec = (dword) delta.tv_sec;*usec = (dword) (delta.tv_nsec / NSEC_PER_USEC);}
Here dword is defined as u32. Since this is only a difference in times, this might be warranted. But, the else part above might not fit in u32 because now you use 64 bit time. Maybe commit text should mention why this is still valid?
Since we are only concerned about difference monotonic times also should be okay.
Deepa, Thanks for the review. Your feedback led me to...
use monotonic time: The driver wants separate secs and usecs fields AND is only looking at delta so use ktime_get_ts64 to get that timespec64 struct. This removes the risk of start time being greater than current time.
uncover a small code cleanup: In further walkthrough of callers of diva_os_get_time() found redundant secs and usec constants declared and initialized using diva_os_get_time. I removed that code.
See what you think in version2. alison
divamnt stores a start_time at module init and uses it to calculate elapsed time. That elapsed time, stored in secs and usecs, is part of the trace data the driver maintains for the DIVA Server ISDN cards. No change to the format of that time data is required.
To avoid overflow on 32-bit systems, change how time data is obtained: - replace struct timeval with struct timespec64 - replace do_gettimeofday() with ktime_get_ts64() - use timespec64_sub() to calculate elapsed time
This is a change from real to monotonic time. Since the driver only stores elapsed time, monotonic time is sufficient and more robust against real time clock changes.
These time deltas are stored in unsigned 32 bits. This is safe as we are always storing time deltas.
Remove declaration and init of unused time constants.
Signed-off-by: Alison Schofield amsfield22@gmail.com --- Changes in v2: - switched to monotonic time - removed the unused time constants - changelog updated
drivers/isdn/hardware/eicon/debug.c | 4 ---- drivers/isdn/hardware/eicon/divamnt.c | 35 +++++++++++------------------------ 2 files changed, 11 insertions(+), 28 deletions(-)
diff --git a/drivers/isdn/hardware/eicon/debug.c b/drivers/isdn/hardware/eicon/debug.c index b5226af..576b7b4 100644 --- a/drivers/isdn/hardware/eicon/debug.c +++ b/drivers/isdn/hardware/eicon/debug.c @@ -192,8 +192,6 @@ static diva_os_spin_lock_t dbg_q_lock; static diva_os_spin_lock_t dbg_adapter_lock; static int dbg_q_busy; static volatile dword dbg_sequence; -static dword start_sec; -static dword start_usec;
/* INTERFACE: @@ -215,8 +213,6 @@ int diva_maint_init(byte *base, unsigned long length, int do_init) {
dbg_base = base;
- diva_os_get_time(&start_sec, &start_usec); - *(dword *)base = (dword)DBG_MAGIC; /* Store Magic */ base += sizeof(dword); length -= sizeof(dword); diff --git a/drivers/isdn/hardware/eicon/divamnt.c b/drivers/isdn/hardware/eicon/divamnt.c index 48db08d..1bb1b26 100644 --- a/drivers/isdn/hardware/eicon/divamnt.c +++ b/drivers/isdn/hardware/eicon/divamnt.c @@ -45,7 +45,7 @@ char *DRIVERRELEASE_MNT = "2.0";
static wait_queue_head_t msgwaitq; static unsigned long opened; -static struct timeval start_time; +static struct timespec64 start_time;
extern int mntfunc_init(int *, void **, unsigned long); extern void mntfunc_finit(void); @@ -88,28 +88,15 @@ int diva_os_copy_from_user(void *os_handle, void *dst, const void __user *src, */ void diva_os_get_time(dword *sec, dword *usec) { - struct timeval tv; - - do_gettimeofday(&tv); - - if (tv.tv_sec > start_time.tv_sec) { - if (start_time.tv_usec > tv.tv_usec) { - tv.tv_sec--; - tv.tv_usec += 1000000; - } - *sec = (dword) (tv.tv_sec - start_time.tv_sec); - *usec = (dword) (tv.tv_usec - start_time.tv_usec); - } else if (tv.tv_sec == start_time.tv_sec) { - *sec = 0; - if (start_time.tv_usec < tv.tv_usec) { - *usec = (dword) (tv.tv_usec - start_time.tv_usec); - } else { - *usec = 0; - } - } else { - *sec = (dword) tv.tv_sec; - *usec = (dword) tv.tv_usec; - } + struct timespec64 curr_time; + struct timespec64 delta; + + ktime_get_ts64(&curr_time); + + delta = timespec64_sub(curr_time, start_time); + + *sec = (dword) delta.tv_sec; + *usec = (dword) (delta.tv_nsec / NSEC_PER_USEC); }
/* @@ -213,7 +200,7 @@ static int __init maint_init(void) int ret = 0; void *buffer = NULL;
- do_gettimeofday(&start_time); + ktime_get_ts64(&start_time); init_waitqueue_head(&msgwaitq);
printk(KERN_INFO "%s\n", DRIVERNAME);
On Friday 08 January 2016 09:47:39 Alison Schofield wrote:
divamnt stores a start_time at module init and uses it to calculate elapsed time. That elapsed time, stored in secs and usecs, is part of the trace data the driver maintains for the DIVA Server ISDN cards. No change to the format of that time data is required.
To avoid overflow on 32-bit systems, change how time data is obtained: - replace struct timeval with struct timespec64 - replace do_gettimeofday() with ktime_get_ts64() - use timespec64_sub() to calculate elapsed time
This is a change from real to monotonic time. Since the driver only stores elapsed time, monotonic time is sufficient and more robust against real time clock changes.
These time deltas are stored in unsigned 32 bits. This is safe as we are always storing time deltas.
Remove declaration and init of unused time constants.
Signed-off-by: Alison Schofield amsfield22@gmail.com
Hi Alison,
I've finally gotten around to look at the patch, and it all looks correct to me in this version, so please post that to the appropriate maintainer lists now.
Reviewed-by: Arnd Bergmann arnd@arndb.de
I think there would be an alternative approach, by removing the 'start_time' completely and directly returning the time from ktime_get_ts64() here. This would make the returned time values slightly different, as they start at system boot time rather than driver load time, but it's only used for debugging, and the normal monotonic times are more useful for that (you can easily compare them to other timestamps).
If you want to do that instead, that is also fine with me.
Arnd
On Tue, Jan 12, 2016 at 11:53:29AM +0100, Arnd Bergmann wrote:
On Friday 08 January 2016 09:47:39 Alison Schofield wrote:
divamnt stores a start_time at module init and uses it to calculate elapsed time. That elapsed time, stored in secs and usecs, is part of the trace data the driver maintains for the DIVA Server ISDN cards. No change to the format of that time data is required.
To avoid overflow on 32-bit systems, change how time data is obtained: - replace struct timeval with struct timespec64 - replace do_gettimeofday() with ktime_get_ts64() - use timespec64_sub() to calculate elapsed time
This is a change from real to monotonic time. Since the driver only stores elapsed time, monotonic time is sufficient and more robust against real time clock changes.
These time deltas are stored in unsigned 32 bits. This is safe as we are always storing time deltas.
Remove declaration and init of unused time constants.
Signed-off-by: Alison Schofield amsfield22@gmail.com
Hi Alison,
I've finally gotten around to look at the patch, and it all looks correct to me in this version, so please post that to the appropriate maintainer lists now.
Reviewed-by: Arnd Bergmann arnd@arndb.de
I think there would be an alternative approach, by removing the 'start_time' completely and directly returning the time from ktime_get_ts64() here. This would make the returned time values slightly different, as they start at system boot time rather than driver load time, but it's only used for debugging, and the normal monotonic times are more useful for that (you can easily compare them to other timestamps).
If you want to do that instead, that is also fine with me.
Arnd
Arnd, The alternative is simplest/cleanest. Gotta do it. Please review v3. Thanks!
divamnt stores a start_time at module init and uses it to calculate elapsed time. The elapsed time, stored in secs and usecs, is part of the trace data the driver maintains for the DIVA Server ISDN cards. No change to the format of that time data is required.
To avoid overflow on 32-bit systems use ktime_get_ts64() to return the elapsed monotonic time since system boot.
This is a change from real to monotonic time. Since the driver only stores elapsed time, monotonic time is sufficient and more robust against real time clock changes. These new monotonic values can be more useful for debugging because they can be easily compared to other monotonic timestamps.
Note elaspsed time values will now start at system boot time rather than module load time, so they will differ slightly from previously reported values.
Remove declaration and init of previously unused time constants: start_sec, start_usec.
Signed-off-by: Alison Schofield amsfield22@gmail.com --- Changes in v3: - use elapsed time since system boot in place of elapsed time since module load - commit message updated - changelog updated
Changes in v2: - switched to monotonic time - removed the unused time constants - changelog updated
drivers/isdn/hardware/eicon/debug.c | 4 ---- drivers/isdn/hardware/eicon/divamnt.c | 30 ++++++------------------------ 2 files changed, 6 insertions(+), 28 deletions(-)
diff --git a/drivers/isdn/hardware/eicon/debug.c b/drivers/isdn/hardware/eicon/debug.c index b5226af..576b7b4 100644 --- a/drivers/isdn/hardware/eicon/debug.c +++ b/drivers/isdn/hardware/eicon/debug.c @@ -192,8 +192,6 @@ static diva_os_spin_lock_t dbg_q_lock; static diva_os_spin_lock_t dbg_adapter_lock; static int dbg_q_busy; static volatile dword dbg_sequence; -static dword start_sec; -static dword start_usec;
/* INTERFACE: @@ -215,8 +213,6 @@ int diva_maint_init(byte *base, unsigned long length, int do_init) {
dbg_base = base;
- diva_os_get_time(&start_sec, &start_usec); - *(dword *)base = (dword)DBG_MAGIC; /* Store Magic */ base += sizeof(dword); length -= sizeof(dword); diff --git a/drivers/isdn/hardware/eicon/divamnt.c b/drivers/isdn/hardware/eicon/divamnt.c index 48db08d..0de29b7b 100644 --- a/drivers/isdn/hardware/eicon/divamnt.c +++ b/drivers/isdn/hardware/eicon/divamnt.c @@ -45,7 +45,6 @@ char *DRIVERRELEASE_MNT = "2.0";
static wait_queue_head_t msgwaitq; static unsigned long opened; -static struct timeval start_time;
extern int mntfunc_init(int *, void **, unsigned long); extern void mntfunc_finit(void); @@ -88,28 +87,12 @@ int diva_os_copy_from_user(void *os_handle, void *dst, const void __user *src, */ void diva_os_get_time(dword *sec, dword *usec) { - struct timeval tv; - - do_gettimeofday(&tv); - - if (tv.tv_sec > start_time.tv_sec) { - if (start_time.tv_usec > tv.tv_usec) { - tv.tv_sec--; - tv.tv_usec += 1000000; - } - *sec = (dword) (tv.tv_sec - start_time.tv_sec); - *usec = (dword) (tv.tv_usec - start_time.tv_usec); - } else if (tv.tv_sec == start_time.tv_sec) { - *sec = 0; - if (start_time.tv_usec < tv.tv_usec) { - *usec = (dword) (tv.tv_usec - start_time.tv_usec); - } else { - *usec = 0; - } - } else { - *sec = (dword) tv.tv_sec; - *usec = (dword) tv.tv_usec; - } + struct timespec64 time; + + ktime_get_ts64(&time); + + *sec = (dword) time.tv_sec; + *usec = (dword) (time.tv_nsec / NSEC_PER_USEC); }
/* @@ -213,7 +196,6 @@ static int __init maint_init(void) int ret = 0; void *buffer = NULL;
- do_gettimeofday(&start_time); init_waitqueue_head(&msgwaitq);
printk(KERN_INFO "%s\n", DRIVERNAME);
On Thursday 14 January 2016 21:52:15 Alison Schofield wrote:
divamnt stores a start_time at module init and uses it to calculate elapsed time. The elapsed time, stored in secs and usecs, is part of the trace data the driver maintains for the DIVA Server ISDN cards. No change to the format of that time data is required.
To avoid overflow on 32-bit systems use ktime_get_ts64() to return the elapsed monotonic time since system boot.
This is a change from real to monotonic time. Since the driver only stores elapsed time, monotonic time is sufficient and more robust against real time clock changes. These new monotonic values can be more useful for debugging because they can be easily compared to other monotonic timestamps.
Note elaspsed time values will now start at system boot time rather than module load time, so they will differ slightly from previously reported values.
Remove declaration and init of previously unused time constants: start_sec, start_usec.
Signed-off-by: Alison Schofield amsfield22@gmail.com
Reviewed-by: Arnd Bergmann arnd@arndb.de
divamnt stores a start_time at module init and uses it to calculate elapsed time. The elapsed time, stored in secs and usecs, is part of the trace data the driver maintains for the DIVA Server ISDN cards. No change to the format of that time data is required.
To avoid overflow on 32-bit systems use ktime_get_ts64() to return the elapsed monotonic time since system boot.
This is a change from real to monotonic time. Since the driver only stores elapsed time, monotonic time is sufficient and more robust against real time clock changes. These new monotonic values can be more useful for debugging because they can be easily compared to other monotonic timestamps.
Note elaspsed time values will now start at system boot time rather than module load time, so they will differ slightly from previously reported values.
Remove declaration and init of previously unused time constants: start_sec, start_usec.
Signed-off-by: Alison Schofield amsfield22@gmail.com Reviewed-by: Arnd Bergmann arnd@arndb.de --- Changes in v3: - use elapsed time since system boot in place of elapsed time since module load - commit message updated - changelog updated
Changes in v2: - switched to monotonic time - removed the unused time constants - changelog updated
drivers/isdn/hardware/eicon/debug.c | 4 ---- drivers/isdn/hardware/eicon/divamnt.c | 30 ++++++------------------------ 2 files changed, 6 insertions(+), 28 deletions(-)
diff --git a/drivers/isdn/hardware/eicon/debug.c b/drivers/isdn/hardware/eicon/debug.c index b5226af..576b7b4 100644 --- a/drivers/isdn/hardware/eicon/debug.c +++ b/drivers/isdn/hardware/eicon/debug.c @@ -192,8 +192,6 @@ static diva_os_spin_lock_t dbg_q_lock; static diva_os_spin_lock_t dbg_adapter_lock; static int dbg_q_busy; static volatile dword dbg_sequence; -static dword start_sec; -static dword start_usec;
/* INTERFACE: @@ -215,8 +213,6 @@ int diva_maint_init(byte *base, unsigned long length, int do_init) {
dbg_base = base;
- diva_os_get_time(&start_sec, &start_usec); - *(dword *)base = (dword)DBG_MAGIC; /* Store Magic */ base += sizeof(dword); length -= sizeof(dword); diff --git a/drivers/isdn/hardware/eicon/divamnt.c b/drivers/isdn/hardware/eicon/divamnt.c index 48db08d..0de29b7b 100644 --- a/drivers/isdn/hardware/eicon/divamnt.c +++ b/drivers/isdn/hardware/eicon/divamnt.c @@ -45,7 +45,6 @@ char *DRIVERRELEASE_MNT = "2.0";
static wait_queue_head_t msgwaitq; static unsigned long opened; -static struct timeval start_time;
extern int mntfunc_init(int *, void **, unsigned long); extern void mntfunc_finit(void); @@ -88,28 +87,12 @@ int diva_os_copy_from_user(void *os_handle, void *dst, const void __user *src, */ void diva_os_get_time(dword *sec, dword *usec) { - struct timeval tv; - - do_gettimeofday(&tv); - - if (tv.tv_sec > start_time.tv_sec) { - if (start_time.tv_usec > tv.tv_usec) { - tv.tv_sec--; - tv.tv_usec += 1000000; - } - *sec = (dword) (tv.tv_sec - start_time.tv_sec); - *usec = (dword) (tv.tv_usec - start_time.tv_usec); - } else if (tv.tv_sec == start_time.tv_sec) { - *sec = 0; - if (start_time.tv_usec < tv.tv_usec) { - *usec = (dword) (tv.tv_usec - start_time.tv_usec); - } else { - *usec = 0; - } - } else { - *sec = (dword) tv.tv_sec; - *usec = (dword) tv.tv_usec; - } + struct timespec64 time; + + ktime_get_ts64(&time); + + *sec = (dword) time.tv_sec; + *usec = (dword) (time.tv_nsec / NSEC_PER_USEC); }
/* @@ -213,7 +196,6 @@ static int __init maint_init(void) int ret = 0; void *buffer = NULL;
- do_gettimeofday(&start_time); init_waitqueue_head(&msgwaitq);
printk(KERN_INFO "%s\n", DRIVERNAME);
From: Alison Schofield amsfield22@gmail.com Date: Fri, 15 Jan 2016 08:51:25 -0800
divamnt stores a start_time at module init and uses it to calculate elapsed time. The elapsed time, stored in secs and usecs, is part of the trace data the driver maintains for the DIVA Server ISDN cards. No change to the format of that time data is required.
To avoid overflow on 32-bit systems use ktime_get_ts64() to return the elapsed monotonic time since system boot.
This is a change from real to monotonic time. Since the driver only stores elapsed time, monotonic time is sufficient and more robust against real time clock changes. These new monotonic values can be more useful for debugging because they can be easily compared to other monotonic timestamps.
Note elaspsed time values will now start at system boot time rather than module load time, so they will differ slightly from previously reported values.
Remove declaration and init of previously unused time constants: start_sec, start_usec.
Signed-off-by: Alison Schofield amsfield22@gmail.com Reviewed-by: Arnd Bergmann arnd@arndb.de
Please resubmit this when the net-next tree is open again.
Thank you.
divamnt stores a start_time at module init and uses it to calculate elapsed time. The elapsed time, stored in secs and usecs, is part of the trace data the driver maintains for the DIVA Server ISDN cards. No change to the format of that time data is required.
To avoid overflow on 32-bit systems use ktime_get_ts64() to return the elapsed monotonic time since system boot.
This is a change from real to monotonic time. Since the driver only stores elapsed time, monotonic time is sufficient and more robust against real time clock changes. These new monotonic values can be more useful for debugging because they can be easily compared to other monotonic timestamps.
Note elaspsed time values will now start at system boot time rather than module load time, so they will differ slightly from previously reported values.
Remove declaration and init of previously unused time constants: start_sec, start_usec.
Signed-off-by: Alison Schofield amsfield22@gmail.com Reviewed-by: Arnd Bergmann arnd@arndb.de --- Changes in v3: - use elapsed time since system boot in place of elapsed time since module load - commit message updated - changelog updated
Changes in v2: - switched to monotonic time - removed the unused time constants - changelog updated
drivers/isdn/hardware/eicon/debug.c | 4 ---- drivers/isdn/hardware/eicon/divamnt.c | 30 ++++++------------------------ 2 files changed, 6 insertions(+), 28 deletions(-)
diff --git a/drivers/isdn/hardware/eicon/debug.c b/drivers/isdn/hardware/eicon/debug.c index b5226af..576b7b4 100644 --- a/drivers/isdn/hardware/eicon/debug.c +++ b/drivers/isdn/hardware/eicon/debug.c @@ -192,8 +192,6 @@ static diva_os_spin_lock_t dbg_q_lock; static diva_os_spin_lock_t dbg_adapter_lock; static int dbg_q_busy; static volatile dword dbg_sequence; -static dword start_sec; -static dword start_usec;
/* INTERFACE: @@ -215,8 +213,6 @@ int diva_maint_init(byte *base, unsigned long length, int do_init) {
dbg_base = base;
- diva_os_get_time(&start_sec, &start_usec); - *(dword *)base = (dword)DBG_MAGIC; /* Store Magic */ base += sizeof(dword); length -= sizeof(dword); diff --git a/drivers/isdn/hardware/eicon/divamnt.c b/drivers/isdn/hardware/eicon/divamnt.c index 48db08d..0de29b7b 100644 --- a/drivers/isdn/hardware/eicon/divamnt.c +++ b/drivers/isdn/hardware/eicon/divamnt.c @@ -45,7 +45,6 @@ char *DRIVERRELEASE_MNT = "2.0";
static wait_queue_head_t msgwaitq; static unsigned long opened; -static struct timeval start_time;
extern int mntfunc_init(int *, void **, unsigned long); extern void mntfunc_finit(void); @@ -88,28 +87,12 @@ int diva_os_copy_from_user(void *os_handle, void *dst, const void __user *src, */ void diva_os_get_time(dword *sec, dword *usec) { - struct timeval tv; - - do_gettimeofday(&tv); - - if (tv.tv_sec > start_time.tv_sec) { - if (start_time.tv_usec > tv.tv_usec) { - tv.tv_sec--; - tv.tv_usec += 1000000; - } - *sec = (dword) (tv.tv_sec - start_time.tv_sec); - *usec = (dword) (tv.tv_usec - start_time.tv_usec); - } else if (tv.tv_sec == start_time.tv_sec) { - *sec = 0; - if (start_time.tv_usec < tv.tv_usec) { - *usec = (dword) (tv.tv_usec - start_time.tv_usec); - } else { - *usec = 0; - } - } else { - *sec = (dword) tv.tv_sec; - *usec = (dword) tv.tv_usec; - } + struct timespec64 time; + + ktime_get_ts64(&time); + + *sec = (dword) time.tv_sec; + *usec = (dword) (time.tv_nsec / NSEC_PER_USEC); }
/* @@ -213,7 +196,6 @@ static int __init maint_init(void) int ret = 0; void *buffer = NULL;
- do_gettimeofday(&start_time); init_waitqueue_head(&msgwaitq);
printk(KERN_INFO "%s\n", DRIVERNAME);
From: Alison Schofield amsfield22@gmail.com Date: Wed, 17 Feb 2016 22:35:11 -0800
divamnt stores a start_time at module init and uses it to calculate elapsed time. The elapsed time, stored in secs and usecs, is part of the trace data the driver maintains for the DIVA Server ISDN cards. No change to the format of that time data is required.
To avoid overflow on 32-bit systems use ktime_get_ts64() to return the elapsed monotonic time since system boot.
This is a change from real to monotonic time. Since the driver only stores elapsed time, monotonic time is sufficient and more robust against real time clock changes. These new monotonic values can be more useful for debugging because they can be easily compared to other monotonic timestamps.
Note elaspsed time values will now start at system boot time rather than module load time, so they will differ slightly from previously reported values.
Remove declaration and init of previously unused time constants: start_sec, start_usec.
Signed-off-by: Alison Schofield amsfield22@gmail.com Reviewed-by: Arnd Bergmann arnd@arndb.de
Applied to net-next, thanks.
-
Alison Schofield -
Arnd Bergmann -
David Miller -
Deepa Dinamani