[patch 00/13] time: Tree wide cleanup of interfaces and crap

[patch 00/13] time: Tree wide cleanup of interfaces and crap

[Thomas Gleixner]

This is part 1 of a larger cleanup series I have on store. This just
deals with simple independent cleanups, except for the removal of
do_posix_clock_monotonic_gettime() which depends on all the related
cleanup patches being applied. I'm happy to take the whole series
trough my tree if nobody objects.

1) Finally get rid of do_posix_clock_monotonic_gettime() which is just
   a wrapper around ktime_get_ts()

2) Kill a few braindamaged time conversion sites

Thanks,

	tglx
----
 drivers/firewire/core-cdev.c            |    2 +-
 drivers/net/wireless/mwifiex/cfg80211.c |    4 +---
 drivers/net/wireless/mwifiex/main.c     |    4 +---
 drivers/net/wireless/mwifiex/tdls.c     |    6 ++----
 drivers/net/wireless/mwifiex/uap_txrx.c |    4 +---
 drivers/net/wireless/mwifiex/wmm.c      |    9 +--------
 include/linux/time.h                    |    1 -
 include/sound/pcm.h                     |    2 +-
 kernel/acct.c                           |    2 +-
 kernel/debug/kdb/kdb_main.c             |    2 +-
 kernel/delayacct.c                      |   16 +++-------------
 kernel/fork.c                           |    2 +-
 kernel/tsacct.c                         |    2 +-
 net/mac80211/cfg.c                      |    2 +-
 net/mac80211/debugfs_sta.c              |    2 +-
 net/mac80211/sta_info.c                 |    2 +-
 net/mac80211/status.c                   |    7 ++-----
 net/mac80211/tx.c                       |    5 +----
 security/tomoyo/audit.c                 |    8 +++-----
 security/tomoyo/common.c                |    4 +---
 sound/core/timer.c                      |    4 ++--
 sound/pci/intel8x0.c                    |   10 ++++------
 22 files changed, 31 insertions(+), 69 deletions(-)

[patch 01/13] acct: Use ktime_get_ts()

[Thomas Gleixner]

[-- Attachment #1: acct-use-ktime-get.patch --]
[-- Type: text/plain, Size: 820 bytes --]

do_posix_clock_monotonic_gettime() is a leftover from the initial
posix timer implementation which maps to ktime_get_ts()

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/acct.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/kernel/acct.c
===================================================================
--- linux.orig/kernel/acct.c
+++ linux/kernel/acct.c
@@ -484,7 +484,7 @@ static void do_acct_process(struct bsd_a
 	strlcpy(ac.ac_comm, current->comm, sizeof(ac.ac_comm));
 
 	/* calculate run_time in nsec*/
-	do_posix_clock_monotonic_gettime(&uptime);
+	ktime_get_ts(&uptime);
 	run_time = (u64)uptime.tv_sec*NSEC_PER_SEC + uptime.tv_nsec;
 	run_time -= (u64)current->group_leader->start_time.tv_sec * NSEC_PER_SEC
 		       + current->group_leader->start_time.tv_nsec;

[tip:timers/core] acct: Use ktime_get_ts()

[tip-bot for Thomas Gleixner]

Commit-ID:  22001821d9cb6ddb83ee4e1f81e6b905de623165
Gitweb:     http://git.kernel.org/tip/22001821d9cb6ddb83ee4e1f81e6b905de623165
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Wed, 11 Jun 2014 23:59:12 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 12 Jun 2014 16:18:44 +0200

acct: Use ktime_get_ts()

do_posix_clock_monotonic_gettime() is a leftover from the initial
posix timer implementation which maps to ktime_get_ts()

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20140611234606.764810535@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/acct.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/acct.c b/kernel/acct.c
index 808a86f..1be013c 100644
--- a/kernel/acct.c
+++ b/kernel/acct.c
@@ -484,7 +484,7 @@ static void do_acct_process(struct bsd_acct_struct *acct,
 	strlcpy(ac.ac_comm, current->comm, sizeof(ac.ac_comm));
 
 	/* calculate run_time in nsec*/
-	do_posix_clock_monotonic_gettime(&uptime);
+	ktime_get_ts(&uptime);
 	run_time = (u64)uptime.tv_sec*NSEC_PER_SEC + uptime.tv_nsec;
 	run_time -= (u64)current->group_leader->start_time.tv_sec * NSEC_PER_SEC
 		       + current->group_leader->start_time.tv_nsec;

[patch 02/13] tsacct: Use ktime_get_ts()

[Thomas Gleixner]

[-- Attachment #1: tsacct-use-ktime-get.patch --]
[-- Type: text/plain, Size: 777 bytes --]

do_posix_clock_monotonic_gettime() is a leftover from the initial
posix timer implementation which maps to ktime_get_ts()

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/tsacct.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/kernel/tsacct.c
===================================================================
--- linux.orig/kernel/tsacct.c
+++ linux/kernel/tsacct.c
@@ -38,7 +38,7 @@ void bacct_add_tsk(struct user_namespace
 	BUILD_BUG_ON(TS_COMM_LEN < TASK_COMM_LEN);
 
 	/* calculate task elapsed time in timespec */
-	do_posix_clock_monotonic_gettime(&uptime);
+	ktime_get_ts(&uptime);
 	ts = timespec_sub(uptime, tsk->start_time);
 	/* rebase elapsed time to usec (should never be negative) */
 	ac_etime = timespec_to_ns(&ts);

[tip:timers/core] tsacct: Use ktime_get_ts()

[tip-bot for Thomas Gleixner]

Commit-ID:  4e8c5847d1c55efed896508fb769f78ab07b968a
Gitweb:     http://git.kernel.org/tip/4e8c5847d1c55efed896508fb769f78ab07b968a
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Wed, 11 Jun 2014 23:59:13 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 12 Jun 2014 16:18:45 +0200

tsacct: Use ktime_get_ts()

do_posix_clock_monotonic_gettime() is a leftover from the initial
posix timer implementation which maps to ktime_get_ts()

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20140611234606.840900621@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/tsacct.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/tsacct.c b/kernel/tsacct.c
index a1dd9a1..ea6d170 100644
--- a/kernel/tsacct.c
+++ b/kernel/tsacct.c
@@ -38,7 +38,7 @@ void bacct_add_tsk(struct user_namespace *user_ns,
 	BUILD_BUG_ON(TS_COMM_LEN < TASK_COMM_LEN);
 
 	/* calculate task elapsed time in timespec */
-	do_posix_clock_monotonic_gettime(&uptime);
+	ktime_get_ts(&uptime);
 	ts = timespec_sub(uptime, tsk->start_time);
 	/* rebase elapsed time to usec (should never be negative) */
 	ac_etime = timespec_to_ns(&ts);

[patch 03/13] delayacct: Use ktime_get_ts()

[Thomas Gleixner]

[-- Attachment #1: kernel-delayacct-use-ktime-get.patch --]
[-- Type: text/plain, Size: 1599 bytes --]

do_posix_clock_monotonic_gettime() is a leftover from the initial
posix timer implementation which maps to ktime_get_ts(). Remove the
silly wrapper while at it.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/delayacct.c |   16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

Index: linux/kernel/delayacct.c
===================================================================
--- linux.orig/kernel/delayacct.c
+++ linux/kernel/delayacct.c
@@ -46,16 +46,6 @@ void __delayacct_tsk_init(struct task_st
 }
 
 /*
- * Start accounting for a delay statistic using
- * its starting timestamp (@start)
- */
-
-static inline void delayacct_start(struct timespec *start)
-{
-	do_posix_clock_monotonic_gettime(start);
-}
-
-/*
  * Finish delay accounting for a statistic using
  * its timestamps (@start, @end), accumalator (@total) and @count
  */
@@ -67,7 +57,7 @@ static void delayacct_end(struct timespe
 	s64 ns;
 	unsigned long flags;
 
-	do_posix_clock_monotonic_gettime(end);
+	ktime_get_ts(end);
 	ts = timespec_sub(*end, *start);
 	ns = timespec_to_ns(&ts);
 	if (ns < 0)
@@ -81,7 +71,7 @@ static void delayacct_end(struct timespe
 
 void __delayacct_blkio_start(void)
 {
-	delayacct_start(&current->delays->blkio_start);
+	ktime_get_ts(&current->delays->blkio_start);
 }
 
 void __delayacct_blkio_end(void)
@@ -169,7 +159,7 @@ __u64 __delayacct_blkio_ticks(struct tas
 
 void __delayacct_freepages_start(void)
 {
-	delayacct_start(&current->delays->freepages_start);
+	ktime_get_ts(&current->delays->freepages_start);
 }
 
 void __delayacct_freepages_end(void)

[tip:timers/core] delayacct: Use ktime_get_ts()

[tip-bot for Thomas Gleixner]

Commit-ID:  b5d7682533941edb121f7495bdb2a17abac03ff3
Gitweb:     http://git.kernel.org/tip/b5d7682533941edb121f7495bdb2a17abac03ff3
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Wed, 11 Jun 2014 23:59:13 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 12 Jun 2014 16:18:45 +0200

delayacct: Use ktime_get_ts()

do_posix_clock_monotonic_gettime() is a leftover from the initial
posix timer implementation which maps to ktime_get_ts(). Remove the
silly wrapper while at it.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20140611234606.931409215@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/delayacct.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/kernel/delayacct.c b/kernel/delayacct.c
index 54996b7..de699f4 100644
--- a/kernel/delayacct.c
+++ b/kernel/delayacct.c
@@ -46,16 +46,6 @@ void __delayacct_tsk_init(struct task_struct *tsk)
 }
 
 /*
- * Start accounting for a delay statistic using
- * its starting timestamp (@start)
- */
-
-static inline void delayacct_start(struct timespec *start)
-{
-	do_posix_clock_monotonic_gettime(start);
-}
-
-/*
  * Finish delay accounting for a statistic using
  * its timestamps (@start, @end), accumalator (@total) and @count
  */
@@ -67,7 +57,7 @@ static void delayacct_end(struct timespec *start, struct timespec *end,
 	s64 ns;
 	unsigned long flags;
 
-	do_posix_clock_monotonic_gettime(end);
+	ktime_get_ts(end);
 	ts = timespec_sub(*end, *start);
 	ns = timespec_to_ns(&ts);
 	if (ns < 0)
@@ -81,7 +71,7 @@ static void delayacct_end(struct timespec *start, struct timespec *end,
 
 void __delayacct_blkio_start(void)
 {
-	delayacct_start(&current->delays->blkio_start);
+	ktime_get_ts(&current->delays->blkio_start);
 }
 
 void __delayacct_blkio_end(void)
@@ -169,7 +159,7 @@ __u64 __delayacct_blkio_ticks(struct task_struct *tsk)
 
 void __delayacct_freepages_start(void)
 {
-	delayacct_start(&current->delays->freepages_start);
+	ktime_get_ts(&current->delays->freepages_start);
 }
 
 void __delayacct_freepages_end(void)

[patch 05/13] sound: Use ktime_get_ts()

[Thomas Gleixner]

[-- Attachment #1: sound-use-ktime-get.patch --]
[-- Type: text/plain, Size: 1494 bytes --]

do_posix_clock_monotonic_gettime() is a leftover from the initial
posix timer implementation which maps to ktime_get_ts().

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.de>
---
 include/sound/pcm.h |    2 +-
 sound/core/timer.c  |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

Index: linux/include/sound/pcm.h
===================================================================
--- linux.orig/include/sound/pcm.h
+++ linux/include/sound/pcm.h
@@ -932,7 +932,7 @@ static inline void snd_pcm_gettime(struc
 				   struct timespec *tv)
 {
 	if (runtime->tstamp_type == SNDRV_PCM_TSTAMP_TYPE_MONOTONIC)
-		do_posix_clock_monotonic_gettime(tv);
+		ktime_get_ts(tv);
 	else
 		getnstimeofday(tv);
 }
Index: linux/sound/core/timer.c
===================================================================
--- linux.orig/sound/core/timer.c
+++ linux/sound/core/timer.c
@@ -390,7 +390,7 @@ static void snd_timer_notify1(struct snd
 	struct timespec tstamp;
 
 	if (timer_tstamp_monotonic)
-		do_posix_clock_monotonic_gettime(&tstamp);
+		ktime_get_ts(&tstamp);
 	else
 		getnstimeofday(&tstamp);
 	if (snd_BUG_ON(event < SNDRV_TIMER_EVENT_START ||
@@ -1203,7 +1203,7 @@ static void snd_timer_user_tinterrupt(st
 	}
 	if (tu->last_resolution != resolution || ticks > 0) {
 		if (timer_tstamp_monotonic)
-			do_posix_clock_monotonic_gettime(&tstamp);
+			ktime_get_ts(&tstamp);
 		else
 			getnstimeofday(&tstamp);
 	}

Re: [patch 05/13] sound: Use ktime_get_ts()

[Takashi Iwai]

At Wed, 11 Jun 2014 23:59:14 -0000,
Thomas Gleixner wrote:
> 
> do_posix_clock_monotonic_gettime() is a leftover from the initial
> posix timer implementation which maps to ktime_get_ts().
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jaroslav Kysela <perex@perex.cz>
> Cc: Takashi Iwai <tiwai@suse.de>

Shall I apply these patches 5 and 6 to sound git tree, or would you
like to apply all in tip tree instead?  In the latter case, feel free
to take my ack for both patches:

  Reviewed-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi


> ---
>  include/sound/pcm.h |    2 +-
>  sound/core/timer.c  |    4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> Index: linux/include/sound/pcm.h
> ===================================================================
> --- linux.orig/include/sound/pcm.h
> +++ linux/include/sound/pcm.h
> @@ -932,7 +932,7 @@ static inline void snd_pcm_gettime(struc
>  				   struct timespec *tv)
>  {
>  	if (runtime->tstamp_type == SNDRV_PCM_TSTAMP_TYPE_MONOTONIC)
> -		do_posix_clock_monotonic_gettime(tv);
> +		ktime_get_ts(tv);
>  	else
>  		getnstimeofday(tv);
>  }
> Index: linux/sound/core/timer.c
> ===================================================================
> --- linux.orig/sound/core/timer.c
> +++ linux/sound/core/timer.c
> @@ -390,7 +390,7 @@ static void snd_timer_notify1(struct snd
>  	struct timespec tstamp;
>  
>  	if (timer_tstamp_monotonic)
> -		do_posix_clock_monotonic_gettime(&tstamp);
> +		ktime_get_ts(&tstamp);
>  	else
>  		getnstimeofday(&tstamp);
>  	if (snd_BUG_ON(event < SNDRV_TIMER_EVENT_START ||
> @@ -1203,7 +1203,7 @@ static void snd_timer_user_tinterrupt(st
>  	}
>  	if (tu->last_resolution != resolution || ticks > 0) {
>  		if (timer_tstamp_monotonic)
> -			do_posix_clock_monotonic_gettime(&tstamp);
> +			ktime_get_ts(&tstamp);
>  		else
>  			getnstimeofday(&tstamp);
>  	}
> 
> 

Re: [patch 05/13] sound: Use ktime_get_ts()

[Thomas Gleixner]

On Thu, 12 Jun 2014, Takashi Iwai wrote:

> At Wed, 11 Jun 2014 23:59:14 -0000,
> Thomas Gleixner wrote:
> > 
> > do_posix_clock_monotonic_gettime() is a leftover from the initial
> > posix timer implementation which maps to ktime_get_ts().
> > 
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Jaroslav Kysela <perex@perex.cz>
> > Cc: Takashi Iwai <tiwai@suse.de>
> 
> Shall I apply these patches 5 and 6 to sound git tree, or would you
> like to apply all in tip tree instead?  In the latter case, feel free
> to take my ack for both patches:
> 
>   Reviewed-by: Takashi Iwai <tiwai@suse.de>

Please take them via sound.

Thanks,

	tglx

Re: [patch 05/13] sound: Use ktime_get_ts()

[Takashi Iwai]

At Thu, 12 Jun 2014 12:51:07 +0200 (CEST),
Thomas Gleixner wrote:
> 
> On Thu, 12 Jun 2014, Takashi Iwai wrote:
> 
> > At Wed, 11 Jun 2014 23:59:14 -0000,
> > Thomas Gleixner wrote:
> > > 
> > > do_posix_clock_monotonic_gettime() is a leftover from the initial
> > > posix timer implementation which maps to ktime_get_ts().
> > > 
> > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > > Cc: Jaroslav Kysela <perex@perex.cz>
> > > Cc: Takashi Iwai <tiwai@suse.de>
> > 
> > Shall I apply these patches 5 and 6 to sound git tree, or would you
> > like to apply all in tip tree instead?  In the latter case, feel free
> > to take my ack for both patches:
> > 
> >   Reviewed-by: Takashi Iwai <tiwai@suse.de>
> 
> Please take them via sound.

OK, applied both now.  Thanks.


Takashi

[patch 04/13] net: mac80211: Use ktime_get_ts()

[Thomas Gleixner]

[-- Attachment #1: net-mac80211-use-ktime-get.patch --]
[-- Type: text/plain, Size: 1964 bytes --]

do_posix_clock_monotonic_gettime() is a leftover from the initial
posix timer implementation which maps to ktime_get_ts().

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: "John W. Linville" <linville@tuxdriver.com>
Cc: linux-wireless@vger.kernel.org
---
 net/mac80211/cfg.c         |    2 +-
 net/mac80211/debugfs_sta.c |    2 +-
 net/mac80211/sta_info.c    |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

Index: linux/net/mac80211/cfg.c
===================================================================
--- linux.orig/net/mac80211/cfg.c
+++ linux/net/mac80211/cfg.c
@@ -484,7 +484,7 @@ static void sta_set_sinfo(struct sta_inf
 			STATION_INFO_STA_FLAGS |
 			STATION_INFO_BEACON_LOSS_COUNT;
 
-	do_posix_clock_monotonic_gettime(&uptime);
+	ktime_get_ts(&uptime);
 	sinfo->connected_time = uptime.tv_sec - sta->last_connected;
 
 	sinfo->inactive_time = jiffies_to_msecs(jiffies - sta->last_rx);
Index: linux/net/mac80211/debugfs_sta.c
===================================================================
--- linux.orig/net/mac80211/debugfs_sta.c
+++ linux/net/mac80211/debugfs_sta.c
@@ -124,7 +124,7 @@ static ssize_t sta_connected_time_read(s
 	long connected_time_secs;
 	char buf[100];
 	int res;
-	do_posix_clock_monotonic_gettime(&uptime);
+	ktime_get_ts(&uptime);
 	connected_time_secs = uptime.tv_sec - sta->last_connected;
 	time_to_tm(connected_time_secs, 0, &result);
 	result.tm_year -= 70;
Index: linux/net/mac80211/sta_info.c
===================================================================
--- linux.orig/net/mac80211/sta_info.c
+++ linux/net/mac80211/sta_info.c
@@ -357,7 +357,7 @@ struct sta_info *sta_info_alloc(struct i
 
 	sta->sta_state = IEEE80211_STA_NONE;
 
-	do_posix_clock_monotonic_gettime(&uptime);
+	ktime_get_ts(&uptime);
 	sta->last_connected = uptime.tv_sec;
 	ewma_init(&sta->avg_signal, 1024, 8);
 	for (i = 0; i < ARRAY_SIZE(sta->chain_signal_avg); i++)

Re: [patch 04/13] net: mac80211: Use ktime_get_ts()

[Johannes Berg]

On Wed, 2014-06-11 at 23:59 +0000, Thomas Gleixner wrote:

> do_posix_clock_monotonic_gettime() is a leftover from the initial
> posix timer implementation which maps to ktime_get_ts().

I didn't even know we *had* such code, heh.

I've applied it to my tree - note that some code moved into sta_info.c
recently there.

johannes

Re: [patch 04/13] net: mac80211: Use ktime_get_ts()

[Luis R. Rodriguez]

On Wed, Jun 11, 2014 at 11:53 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Wed, 2014-06-11 at 23:59 +0000, Thomas Gleixner wrote:
>
>> do_posix_clock_monotonic_gettime() is a leftover from the initial
>> posix timer implementation which maps to ktime_get_ts().
>
> I didn't even know we *had* such code, heh.

If we want want maintainers to police this stuff we can add an SmPL
rule for this upstream and have maintainer use 'make coccicheck
M=path/'. The way of the future. The way of the future...

  Luis

Re: [Cocci] [patch 04/13] net: mac80211: Use ktime_get_ts()

[Julia.Lawall]

"Luis R. Rodriguez" <mcgrof@do-not-panic.com> a écrit :

> On Wed, Jun 11, 2014 at 11:53 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
>> On Wed, 2014-06-11 at 23:59 +0000, Thomas Gleixner wrote:
>>
>>> do_posix_clock_monotonic_gettime() is a leftover from the initial
>>> posix timer implementation which maps to ktime_get_ts().
>>
>> I didn't even know we *had* such code, heh.
>
> If we want want maintainers to police this stuff we can add an SmPL
> rule for this upstream and have maintainer use 'make coccicheck
> M=path/'. The way of the future. The way of the future...

If I could have a little more context, I can do something.

julia

>
>   Luis
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

Re: [Cocci] [patch 04/13] net: mac80211: Use ktime_get_ts()

[Thomas Gleixner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 883 bytes --]

On Thu, 12 Jun 2014, Julia.Lawall@lip6.fr wrote:

> "Luis R. Rodriguez" <mcgrof@do-not-panic.com> a écrit :
> 
> > On Wed, Jun 11, 2014 at 11:53 PM, Johannes Berg
> > <johannes@sipsolutions.net> wrote:
> > > On Wed, 2014-06-11 at 23:59 +0000, Thomas Gleixner wrote:
> > > 
> > > > do_posix_clock_monotonic_gettime() is a leftover from the initial
> > > > posix timer implementation which maps to ktime_get_ts().
> > > 
> > > I didn't even know we *had* such code, heh.
> > 
> > If we want want maintainers to police this stuff we can add an SmPL
> > rule for this upstream and have maintainer use 'make coccicheck
> > M=path/'. The way of the future. The way of the future...
> 
> If I could have a little more context, I can do something.

I already killed all instances of do_posix_clock_monotonic_gettime().
The define will be gone soon, so nothing to worry about.

Thanks,

	tglx

[patch 06/13] sound: intel8x0: Use ktime and ktime_get()

[Thomas Gleixner]

[-- Attachment #1: sound-intel8x0-use-ktime.patch --]
[-- Type: text/plain, Size: 2015 bytes --]

do_posix_clock_monotonic_gettime() is a leftover from the initial
posix timer implementation which maps to ktime_get_ts() and returns
the monotonic time in a timespec.

Use ktime based ktime_get() and use the ktime_delta_us() function to
calculate the delta instead of open coding the timespec math.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Jaroslav Kysela <perex@perex.cz>
Cc: Takashi Iwai <tiwai@suse.de>
---
 sound/pci/intel8x0.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Index: linux/sound/pci/intel8x0.c
===================================================================
--- linux.orig/sound/pci/intel8x0.c
+++ linux/sound/pci/intel8x0.c
@@ -2779,7 +2779,7 @@ static void intel8x0_measure_ac97_clock(
 	unsigned long port;
 	unsigned long pos, pos1, t;
 	int civ, timeout = 1000, attempt = 1;
-	struct timespec start_time, stop_time;
+	ktime_t start_time, stop_time;
 
 	if (chip->ac97_bus->clock != 48000)
 		return; /* specified in module option */
@@ -2813,7 +2813,7 @@ static void intel8x0_measure_ac97_clock(
 		iputbyte(chip, port + ICH_REG_OFF_CR, ICH_IOCE);
 		iputdword(chip, ICHREG(ALI_DMACR), 1 << ichdev->ali_slot);
 	}
-	do_posix_clock_monotonic_gettime(&start_time);
+	start_time = ktime_get();
 	spin_unlock_irq(&chip->reg_lock);
 	msleep(50);
 	spin_lock_irq(&chip->reg_lock);
@@ -2837,7 +2837,7 @@ static void intel8x0_measure_ac97_clock(
 		pos += ichdev->position;
 	}
 	chip->in_measurement = 0;
-	do_posix_clock_monotonic_gettime(&stop_time);
+	stop_time = ktime_get();
 	/* stop */
 	if (chip->device_type == DEVICE_ALI) {
 		iputdword(chip, ICHREG(ALI_DMACR), 1 << (ichdev->ali_slot + 16));
@@ -2865,9 +2865,7 @@ static void intel8x0_measure_ac97_clock(
 	}
 
 	pos /= 4;
-	t = stop_time.tv_sec - start_time.tv_sec;
-	t *= 1000000;
-	t += (stop_time.tv_nsec - start_time.tv_nsec) / 1000;
+	t = ktime_us_delta(stop_time, start_time);
 	dev_info(chip->card->dev,
 		 "%s: measured %lu usecs (%lu samples)\n", __func__, t, pos);
 	if (t == 0) {

[patch 08/13] firewire: Use ktime_get_ts()

[Thomas Gleixner]

[-- Attachment #1: firewire-use-ktime-get-ts.patch --]
[-- Type: text/plain, Size: 902 bytes --]

do_posix_clock_monotonic_gettime() is a leftover from the initial
posix timer implementation which maps to ktime_get_ts()

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/firewire/core-cdev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/drivers/firewire/core-cdev.c
===================================================================
--- linux.orig/drivers/firewire/core-cdev.c
+++ linux/drivers/firewire/core-cdev.c
@@ -1215,7 +1215,7 @@ static int ioctl_get_cycle_timer2(struct
 
 	switch (a->clk_id) {
 	case CLOCK_REALTIME:      getnstimeofday(&ts);                   break;
-	case CLOCK_MONOTONIC:     do_posix_clock_monotonic_gettime(&ts); break;
+	case CLOCK_MONOTONIC:     ktime_get_ts(&ts); break;
 	case CLOCK_MONOTONIC_RAW: getrawmonotonic(&ts);                  break;
 	default:
 		ret = -EINVAL;

Re: [patch 08/13] firewire: Use ktime_get_ts()

[Stefan Richter]

On Jun 11 Thomas Gleixner wrote:
> do_posix_clock_monotonic_gettime() is a leftover from the initial
> posix timer implementation which maps to ktime_get_ts()
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Stefan Richter <stefanr@s5r6.in-berlin.de>

Acked-by: Stefan Richter <stefanr@s5r6.in-berlin.de>

(Though I am noticing increasing entropy WRT whitespace.)

> ---
>  drivers/firewire/core-cdev.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: linux/drivers/firewire/core-cdev.c
> ===================================================================
> --- linux.orig/drivers/firewire/core-cdev.c
> +++ linux/drivers/firewire/core-cdev.c
> @@ -1215,7 +1215,7 @@ static int ioctl_get_cycle_timer2(struct
>  
>  	switch (a->clk_id) {
>  	case CLOCK_REALTIME:      getnstimeofday(&ts);                   break;
> -	case CLOCK_MONOTONIC:     do_posix_clock_monotonic_gettime(&ts); break;
> +	case CLOCK_MONOTONIC:     ktime_get_ts(&ts); break;
>  	case CLOCK_MONOTONIC_RAW: getrawmonotonic(&ts);                  break;
>  	default:
>  		ret = -EINVAL;
> 
> 

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

Re: [patch 08/13] firewire: Use ktime_get_ts()

[Thomas Gleixner]

On Thu, 12 Jun 2014, Stefan Richter wrote:

> On Jun 11 Thomas Gleixner wrote:
> > do_posix_clock_monotonic_gettime() is a leftover from the initial
> > posix timer implementation which maps to ktime_get_ts()
> > 
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Stefan Richter <stefanr@s5r6.in-berlin.de>
> 
> Acked-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
> 
> (Though I am noticing increasing entropy WRT whitespace.)

Fixed it up.

[tip:timers/core] firewire: Use ktime_get_ts()

[tip-bot for Thomas Gleixner]

Commit-ID:  889235ce2b26cdd231c7a1d6005928ec42f11637
Gitweb:     http://git.kernel.org/tip/889235ce2b26cdd231c7a1d6005928ec42f11637
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Wed, 11 Jun 2014 23:59:16 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 12 Jun 2014 16:18:45 +0200

firewire: Use ktime_get_ts()

do_posix_clock_monotonic_gettime() is a leftover from the initial
posix timer implementation which maps to ktime_get_ts()

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Acked-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
Link: http://lkml.kernel.org/r/20140611234607.351283464@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/firewire/core-cdev.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index d7d5c8a..5d997a3 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1214,9 +1214,9 @@ static int ioctl_get_cycle_timer2(struct client *client, union ioctl_arg *arg)
 	cycle_time = card->driver->read_csr(card, CSR_CYCLE_TIME);
 
 	switch (a->clk_id) {
-	case CLOCK_REALTIME:      getnstimeofday(&ts);                   break;
-	case CLOCK_MONOTONIC:     do_posix_clock_monotonic_gettime(&ts); break;
-	case CLOCK_MONOTONIC_RAW: getrawmonotonic(&ts);                  break;
+	case CLOCK_REALTIME:      getnstimeofday(&ts);	break;
+	case CLOCK_MONOTONIC:     ktime_get_ts(&ts);	break;
+	case CLOCK_MONOTONIC_RAW: getrawmonotonic(&ts);	break;
 	default:
 		ret = -EINVAL;
 	}

[patch 07/13] kdb: Use ktime_get_ts()

[Thomas Gleixner]

[-- Attachment #1: kdb-use-ktime-get.patch --]
[-- Type: text/plain, Size: 806 bytes --]

do_posix_clock_monotonic_gettime() is a leftover from the initial
posix timer implementation which maps to ktime_get_ts().

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Jason Wessel <jason.wessel@windriver.com>
---
 kernel/debug/kdb/kdb_main.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/kernel/debug/kdb/kdb_main.c
===================================================================
--- linux.orig/kernel/debug/kdb/kdb_main.c
+++ linux/kernel/debug/kdb/kdb_main.c
@@ -2472,7 +2472,7 @@ static void kdb_gmtime(struct timespec *
 static void kdb_sysinfo(struct sysinfo *val)
 {
 	struct timespec uptime;
-	do_posix_clock_monotonic_gettime(&uptime);
+	ktime_get_ts(&uptime);
 	memset(val, 0, sizeof(*val));
 	val->uptime = uptime.tv_sec;
 	val->loads[0] = avenrun[0];

[tip:timers/core] kdb: Use ktime_get_ts()

[tip-bot for Thomas Gleixner]

Commit-ID:  a9821c741c960a77a7f08491883f9cc4bffd2279
Gitweb:     http://git.kernel.org/tip/a9821c741c960a77a7f08491883f9cc4bffd2279
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Wed, 11 Jun 2014 23:59:16 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 12 Jun 2014 16:18:45 +0200

kdb: Use ktime_get_ts()

do_posix_clock_monotonic_gettime() is a leftover from the initial
posix timer implementation which maps to ktime_get_ts().

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jason Wessel <jason.wessel@windriver.com>
Link: http://lkml.kernel.org/r/20140611234607.261629142@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/debug/kdb/kdb_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 2f7c760..379650b 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2472,7 +2472,7 @@ static void kdb_gmtime(struct timespec *tv, struct kdb_tm *tm)
 static void kdb_sysinfo(struct sysinfo *val)
 {
 	struct timespec uptime;
-	do_posix_clock_monotonic_gettime(&uptime);
+	ktime_get_ts(&uptime);
 	memset(val, 0, sizeof(*val));
 	val->uptime = uptime.tv_sec;
 	val->loads[0] = avenrun[0];

[patch 10/13] time: Remove do_posix_clock_monotonic_gettime()

[Thomas Gleixner]

[-- Attachment #1: time-remove-cruft.patch --]
[-- Type: text/plain, Size: 903 bytes --]

do_posix_clock_monotonic_gettime() is a leftover from the initial
posix timer implementation which maps to ktime_get_ts().

No more users. Remove the cruft.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/time.h |    1 -
 1 file changed, 1 deletion(-)

Index: linux/include/linux/time.h
===================================================================
--- linux.orig/include/linux/time.h
+++ linux/include/linux/time.h
@@ -157,7 +157,6 @@ extern void do_gettimeofday(struct timev
 extern int do_settimeofday(const struct timespec *tv);
 extern int do_sys_settimeofday(const struct timespec *tv,
 			       const struct timezone *tz);
-#define do_posix_clock_monotonic_gettime(ts) ktime_get_ts(ts)
 extern long do_utimes(int dfd, const char __user *filename, struct timespec *times, int flags);
 struct itimerval;
 extern int do_setitimer(int which, struct itimerval *value,

[patch 09/13] fork: Use ktime_get_ts()

[Thomas Gleixner]

[-- Attachment #1: fork-use-ktime-get-ts.patch --]
[-- Type: text/plain, Size: 727 bytes --]

do_posix_clock_monotonic_gettime() is a leftover from the initial
posix timer implementation which maps to ktime_get_ts().

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Oleg Nesterov <oleg@redhat.com>
---
 kernel/fork.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/kernel/fork.c
===================================================================
--- linux.orig/kernel/fork.c
+++ linux/kernel/fork.c
@@ -1262,7 +1262,7 @@ static struct task_struct *copy_process(
 
 	posix_cpu_timers_init(p);
 
-	do_posix_clock_monotonic_gettime(&p->start_time);
+	ktime_get_ts(&p->start_time);
 	p->real_start_time = p->start_time;
 	monotonic_to_bootbased(&p->real_start_time);
 	p->io_context = NULL;

[tip:timers/core] fork: Use ktime_get_ts()

[tip-bot for Thomas Gleixner]

Commit-ID:  f037c1171db79be2a047b1a5aafa2fd1f05051cb
Gitweb:     http://git.kernel.org/tip/f037c1171db79be2a047b1a5aafa2fd1f05051cb
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Wed, 11 Jun 2014 23:59:17 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 12 Jun 2014 16:18:45 +0200

fork: Use ktime_get_ts()

do_posix_clock_monotonic_gettime() is a leftover from the initial
posix timer implementation which maps to ktime_get_ts().

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Link: http://lkml.kernel.org/r/20140611234607.427408044@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Oleg Nesterov <oleg@redhat.com>
---
 kernel/fork.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index d2799d1..ea0dd70 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1262,7 +1262,7 @@ static struct task_struct *copy_process(unsigned long clone_flags,
 
 	posix_cpu_timers_init(p);
 
-	do_posix_clock_monotonic_gettime(&p->start_time);
+	ktime_get_ts(&p->start_time);
 	p->real_start_time = p->start_time;
 	monotonic_to_bootbased(&p->real_start_time);
 	p->io_context = NULL;

[patch 11/13] wireless: mwifiex: Use the proper interfaces

[Thomas Gleixner]

[-- Attachment #1: wireless-use-ktime-get-wtf.patch --]
[-- Type: text/plain, Size: 4478 bytes --]

Why is converting time formats so desired if there are proper
interfaces for this?

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Bing Zhao <bzhao@marvell.com>
Cc: "John W. Linville" <linville@tuxdriver.com>
Cc: linux-wireless@vger.kernel.org
---
 drivers/net/wireless/mwifiex/cfg80211.c |    4 +---
 drivers/net/wireless/mwifiex/main.c     |    4 +---
 drivers/net/wireless/mwifiex/tdls.c     |    6 ++----
 drivers/net/wireless/mwifiex/uap_txrx.c |    4 +---
 drivers/net/wireless/mwifiex/wmm.c      |    9 +--------
 5 files changed, 6 insertions(+), 21 deletions(-)

Index: linux/drivers/net/wireless/mwifiex/cfg80211.c
===================================================================
--- linux.orig/drivers/net/wireless/mwifiex/cfg80211.c
+++ linux/drivers/net/wireless/mwifiex/cfg80211.c
@@ -151,7 +151,6 @@ mwifiex_form_mgmt_frame(struct sk_buff *
 	u8 addr[ETH_ALEN] = {0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
 	u16 pkt_len;
 	u32 tx_control = 0, pkt_type = PKT_TYPE_MGMT;
-	struct timeval tv;
 
 	pkt_len = len + ETH_ALEN;
 
@@ -173,8 +172,7 @@ mwifiex_form_mgmt_frame(struct sk_buff *
 	       len - sizeof(struct ieee80211_hdr_3addr));
 
 	skb->priority = LOW_PRIO_TID;
-	do_gettimeofday(&tv);
-	skb->tstamp = timeval_to_ktime(tv);
+	__net_timestamp(skb);
 
 	return 0;
 }
Index: linux/drivers/net/wireless/mwifiex/main.c
===================================================================
--- linux.orig/drivers/net/wireless/mwifiex/main.c
+++ linux/drivers/net/wireless/mwifiex/main.c
@@ -611,7 +611,6 @@ mwifiex_hard_start_xmit(struct sk_buff *
 	struct mwifiex_private *priv = mwifiex_netdev_get_priv(dev);
 	struct sk_buff *new_skb;
 	struct mwifiex_txinfo *tx_info;
-	struct timeval tv;
 
 	dev_dbg(priv->adapter->dev, "data: %lu BSS(%d-%d): Data <= kernel\n",
 		jiffies, priv->bss_type, priv->bss_num);
@@ -658,8 +657,7 @@ mwifiex_hard_start_xmit(struct sk_buff *
 	 * firmware for aggregate delay calculation for stats and
 	 * MSDU lifetime expiry.
 	 */
-	do_gettimeofday(&tv);
-	skb->tstamp = timeval_to_ktime(tv);
+	__net_timestamp(skb);
 
 	mwifiex_queue_tx_pkt(priv, skb);
 
Index: linux/drivers/net/wireless/mwifiex/tdls.c
===================================================================
--- linux.orig/drivers/net/wireless/mwifiex/tdls.c
+++ linux/drivers/net/wireless/mwifiex/tdls.c
@@ -552,8 +552,7 @@ int mwifiex_send_tdls_data_frame(struct
 	tx_info->bss_num = priv->bss_num;
 	tx_info->bss_type = priv->bss_type;
 
-	do_gettimeofday(&tv);
-	skb->tstamp = timeval_to_ktime(tv);
+	__net_timestamp(skb);
 	mwifiex_queue_tx_pkt(priv, skb);
 
 	return 0;
@@ -710,8 +709,7 @@ int mwifiex_send_tdls_action_frame(struc
 	pkt_len = skb->len - MWIFIEX_MGMT_FRAME_HEADER_SIZE - sizeof(pkt_len);
 	memcpy(skb->data + MWIFIEX_MGMT_FRAME_HEADER_SIZE, &pkt_len,
 	       sizeof(pkt_len));
-	do_gettimeofday(&tv);
-	skb->tstamp = timeval_to_ktime(tv);
+	__net_timestamp(skb);
 	mwifiex_queue_tx_pkt(priv, skb);
 
 	return 0;
Index: linux/drivers/net/wireless/mwifiex/uap_txrx.c
===================================================================
--- linux.orig/drivers/net/wireless/mwifiex/uap_txrx.c
+++ linux/drivers/net/wireless/mwifiex/uap_txrx.c
@@ -96,7 +96,6 @@ static void mwifiex_uap_queue_bridged_pk
 	struct sk_buff *new_skb;
 	struct mwifiex_txinfo *tx_info;
 	int hdr_chop;
-	struct timeval tv;
 	struct ethhdr *p_ethhdr;
 
 	uap_rx_pd = (struct uap_rxpd *)(skb->data);
@@ -192,8 +191,7 @@ static void mwifiex_uap_queue_bridged_pk
 		tx_info->pkt_len = skb->len;
 	}
 
-	do_gettimeofday(&tv);
-	skb->tstamp = timeval_to_ktime(tv);
+	__net_timestamp(skb);
 	mwifiex_wmm_add_buf_txqueue(priv, skb);
 	atomic_inc(&adapter->tx_pending);
 	atomic_inc(&adapter->pending_bridged_pkts);
Index: linux/drivers/net/wireless/mwifiex/wmm.c
===================================================================
--- linux.orig/drivers/net/wireless/mwifiex/wmm.c
+++ linux/drivers/net/wireless/mwifiex/wmm.c
@@ -886,15 +886,8 @@ u8
 mwifiex_wmm_compute_drv_pkt_delay(struct mwifiex_private *priv,
 				  const struct sk_buff *skb)
 {
+	u32 queue_delay = ktime_to_ms(net_timedelta(skb->tstamp));
 	u8 ret_val;
-	struct timeval out_tstamp, in_tstamp;
-	u32 queue_delay;
-
-	do_gettimeofday(&out_tstamp);
-	in_tstamp = ktime_to_timeval(skb->tstamp);
-
-	queue_delay = (out_tstamp.tv_sec - in_tstamp.tv_sec) * 1000;
-	queue_delay += (out_tstamp.tv_usec - in_tstamp.tv_usec) / 1000;
 
 	/*
 	 * Queue delay is passed as a uint8 in units of 2ms (ms shifted

RE: [patch 11/13] wireless: mwifiex: Use the proper interfaces

[Bing Zhao]

Hi Thomas,

Thanks for your patch.

> Why is converting time formats so desired if there are proper
> interfaces for this?
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Bing Zhao <bzhao@marvell.com>
> Cc: "John W. Linville" <linville@tuxdriver.com>
> Cc: linux-wireless@vger.kernel.org

[...]

> Index: linux/drivers/net/wireless/mwifiex/main.c
> ===================================================================
> --- linux.orig/drivers/net/wireless/mwifiex/main.c
> +++ linux/drivers/net/wireless/mwifiex/main.c
> @@ -611,7 +611,6 @@ mwifiex_hard_start_xmit(struct sk_buff *
>  	struct mwifiex_private *priv = mwifiex_netdev_get_priv(dev);
>  	struct sk_buff *new_skb;
>  	struct mwifiex_txinfo *tx_info;
> -	struct timeval tv;
> 
>  	dev_dbg(priv->adapter->dev, "data: %lu BSS(%d-%d): Data <= kernel\n",
>  		jiffies, priv->bss_type, priv->bss_num);
> @@ -658,8 +657,7 @@ mwifiex_hard_start_xmit(struct sk_buff *
>  	 * firmware for aggregate delay calculation for stats and
>  	 * MSDU lifetime expiry.
>  	 */
> -	do_gettimeofday(&tv);
> -	skb->tstamp = timeval_to_ktime(tv);
> +	__net_timestamp(skb);
> 
>  	mwifiex_queue_tx_pkt(priv, skb);
> 
> Index: linux/drivers/net/wireless/mwifiex/tdls.c
> ===================================================================
> --- linux.orig/drivers/net/wireless/mwifiex/tdls.c
> +++ linux/drivers/net/wireless/mwifiex/tdls.c
> @@ -552,8 +552,7 @@ int mwifiex_send_tdls_data_frame(struct
>  	tx_info->bss_num = priv->bss_num;
>  	tx_info->bss_type = priv->bss_type;
> 
> -	do_gettimeofday(&tv);
> -	skb->tstamp = timeval_to_ktime(tv);
> +	__net_timestamp(skb);

I guess we need to remove "struct timeval tv" local variable too.

>  	mwifiex_queue_tx_pkt(priv, skb);
> 
>  	return 0;
> @@ -710,8 +709,7 @@ int mwifiex_send_tdls_action_frame(struc
>  	pkt_len = skb->len - MWIFIEX_MGMT_FRAME_HEADER_SIZE - sizeof(pkt_len);
>  	memcpy(skb->data + MWIFIEX_MGMT_FRAME_HEADER_SIZE, &pkt_len,
>  	       sizeof(pkt_len));
> -	do_gettimeofday(&tv);
> -	skb->tstamp = timeval_to_ktime(tv);
> +	__net_timestamp(skb);

And here too.

Could you please remove these two "struct timeval tv" and send v2 with my ACK?

Acked-by: Bing Zhao <bzhao@marvell.com>

Thanks,
Bing

>  	mwifiex_queue_tx_pkt(priv, skb);
> 
>  	return 0;

[patch V2] wireless: mwifiex: Use the proper interfaces

[Thomas Gleixner]

Why is converting time formats so desired if there are proper
interfaces for this?

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Bing Zhao <bzhao@marvell.com>
Cc: "John W. Linville" <linville@tuxdriver.com>
Cc: linux-wireless@vger.kernel.org
---
 drivers/net/wireless/mwifiex/cfg80211.c |    4 +---
 drivers/net/wireless/mwifiex/main.c     |    4 +---
 drivers/net/wireless/mwifiex/tdls.c     |    8 ++------
 drivers/net/wireless/mwifiex/uap_txrx.c |    4 +---
 drivers/net/wireless/mwifiex/wmm.c      |    9 +--------
 5 files changed, 6 insertions(+), 23 deletions(-)

Index: linux/drivers/net/wireless/mwifiex/cfg80211.c
===================================================================
--- linux.orig/drivers/net/wireless/mwifiex/cfg80211.c
+++ linux/drivers/net/wireless/mwifiex/cfg80211.c
@@ -151,7 +151,6 @@ mwifiex_form_mgmt_frame(struct sk_buff *
 	u8 addr[ETH_ALEN] = {0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
 	u16 pkt_len;
 	u32 tx_control = 0, pkt_type = PKT_TYPE_MGMT;
-	struct timeval tv;
 
 	pkt_len = len + ETH_ALEN;
 
@@ -173,8 +172,7 @@ mwifiex_form_mgmt_frame(struct sk_buff *
 	       len - sizeof(struct ieee80211_hdr_3addr));
 
 	skb->priority = LOW_PRIO_TID;
-	do_gettimeofday(&tv);
-	skb->tstamp = timeval_to_ktime(tv);
+	__net_timestamp(skb);
 
 	return 0;
 }
Index: linux/drivers/net/wireless/mwifiex/main.c
===================================================================
--- linux.orig/drivers/net/wireless/mwifiex/main.c
+++ linux/drivers/net/wireless/mwifiex/main.c
@@ -611,7 +611,6 @@ mwifiex_hard_start_xmit(struct sk_buff *
 	struct mwifiex_private *priv = mwifiex_netdev_get_priv(dev);
 	struct sk_buff *new_skb;
 	struct mwifiex_txinfo *tx_info;
-	struct timeval tv;
 
 	dev_dbg(priv->adapter->dev, "data: %lu BSS(%d-%d): Data <= kernel\n",
 		jiffies, priv->bss_type, priv->bss_num);
@@ -658,8 +657,7 @@ mwifiex_hard_start_xmit(struct sk_buff *
 	 * firmware for aggregate delay calculation for stats and
 	 * MSDU lifetime expiry.
 	 */
-	do_gettimeofday(&tv);
-	skb->tstamp = timeval_to_ktime(tv);
+	__net_timestamp(skb);
 
 	mwifiex_queue_tx_pkt(priv, skb);
 
Index: linux/drivers/net/wireless/mwifiex/tdls.c
===================================================================
--- linux.orig/drivers/net/wireless/mwifiex/tdls.c
+++ linux/drivers/net/wireless/mwifiex/tdls.c
@@ -474,7 +474,6 @@ int mwifiex_send_tdls_data_frame(struct
 {
 	struct sk_buff *skb;
 	struct mwifiex_txinfo *tx_info;
-	struct timeval tv;
 	int ret;
 	u16 skb_len;
 
@@ -552,8 +551,7 @@ int mwifiex_send_tdls_data_frame(struct
 	tx_info->bss_num = priv->bss_num;
 	tx_info->bss_type = priv->bss_type;
 
-	do_gettimeofday(&tv);
-	skb->tstamp = timeval_to_ktime(tv);
+	__net_timestamp(skb);
 	mwifiex_queue_tx_pkt(priv, skb);
 
 	return 0;
@@ -645,7 +643,6 @@ int mwifiex_send_tdls_action_frame(struc
 {
 	struct sk_buff *skb;
 	struct mwifiex_txinfo *tx_info;
-	struct timeval tv;
 	u8 *pos;
 	u32 pkt_type, tx_control;
 	u16 pkt_len, skb_len;
@@ -710,8 +707,7 @@ int mwifiex_send_tdls_action_frame(struc
 	pkt_len = skb->len - MWIFIEX_MGMT_FRAME_HEADER_SIZE - sizeof(pkt_len);
 	memcpy(skb->data + MWIFIEX_MGMT_FRAME_HEADER_SIZE, &pkt_len,
 	       sizeof(pkt_len));
-	do_gettimeofday(&tv);
-	skb->tstamp = timeval_to_ktime(tv);
+	__net_timestamp(skb);
 	mwifiex_queue_tx_pkt(priv, skb);
 
 	return 0;
Index: linux/drivers/net/wireless/mwifiex/uap_txrx.c
===================================================================
--- linux.orig/drivers/net/wireless/mwifiex/uap_txrx.c
+++ linux/drivers/net/wireless/mwifiex/uap_txrx.c
@@ -96,7 +96,6 @@ static void mwifiex_uap_queue_bridged_pk
 	struct sk_buff *new_skb;
 	struct mwifiex_txinfo *tx_info;
 	int hdr_chop;
-	struct timeval tv;
 	struct ethhdr *p_ethhdr;
 
 	uap_rx_pd = (struct uap_rxpd *)(skb->data);
@@ -192,8 +191,7 @@ static void mwifiex_uap_queue_bridged_pk
 		tx_info->pkt_len = skb->len;
 	}
 
-	do_gettimeofday(&tv);
-	skb->tstamp = timeval_to_ktime(tv);
+	__net_timestamp(skb);
 	mwifiex_wmm_add_buf_txqueue(priv, skb);
 	atomic_inc(&adapter->tx_pending);
 	atomic_inc(&adapter->pending_bridged_pkts);
Index: linux/drivers/net/wireless/mwifiex/wmm.c
===================================================================
--- linux.orig/drivers/net/wireless/mwifiex/wmm.c
+++ linux/drivers/net/wireless/mwifiex/wmm.c
@@ -886,15 +886,8 @@ u8
 mwifiex_wmm_compute_drv_pkt_delay(struct mwifiex_private *priv,
 				  const struct sk_buff *skb)
 {
+	u32 queue_delay = ktime_to_ms(net_timedelta(skb->tstamp));
 	u8 ret_val;
-	struct timeval out_tstamp, in_tstamp;
-	u32 queue_delay;
-
-	do_gettimeofday(&out_tstamp);
-	in_tstamp = ktime_to_timeval(skb->tstamp);
-
-	queue_delay = (out_tstamp.tv_sec - in_tstamp.tv_sec) * 1000;
-	queue_delay += (out_tstamp.tv_usec - in_tstamp.tv_usec) / 1000;
 
 	/*
 	 * Queue delay is passed as a uint8 in units of 2ms (ms shifted

Re: [patch V2] wireless: mwifiex: Use the proper interfaces

[Johannes Berg]

On Thu, 2014-06-12 at 10:31 +0200, Thomas Gleixner wrote:

> +	u32 queue_delay = ktime_to_ms(net_timedelta(skb->tstamp));

FWIW, I think the same as patch 12 applies here. net_timedelta() doesn't
really seem to be a good way to calculate time deltas.

And yes - I've seen situations where this matters, e.g. when running
ntpdate after system startup.

johannes

RE: [patch V2] wireless: mwifiex: Use the proper interfaces

[Bing Zhao]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 636 bytes --]


> > +	u32 queue_delay = ktime_to_ms(net_timedelta(skb->tstamp));
> 
> FWIW, I think the same as patch 12 applies here. net_timedelta() doesn't
> really seem to be a good way to calculate time deltas.

Shall we change net_timedelta to ktime_sub() using ktime_get()?

+	u32 queue_delay = ktime_to_ms(ktime_sub(ktime_get(), skb->tstamp));

Thanks,
Bing

> 
> And yes - I've seen situations where this matters, e.g. when running
> ntpdate after system startup.
> 
> johannes
> 

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

[patch 12/13] net: mac80211: Remove silly timespec dance

[Thomas Gleixner]

[-- Attachment #1: net-max80211-remove-silly-timespec-dance.patch --]
[-- Type: text/plain, Size: 1833 bytes --]

Converting time from one format to another seems to give coders a warm
and fuzzy feeling.

Use the proper interfaces.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: "John W. Linville" <linville@tuxdriver.com>
Cc: linux-wireless@vger.kernel.org
---
 net/mac80211/status.c |    7 ++-----
 net/mac80211/tx.c     |    5 +----
 2 files changed, 3 insertions(+), 9 deletions(-)

Index: linux/net/mac80211/status.c
===================================================================
--- linux.orig/net/mac80211/status.c
+++ linux/net/mac80211/status.c
@@ -473,8 +473,6 @@ static void ieee80211_tx_latency_end_msr
 					    struct sta_info *sta,
 					    struct ieee80211_hdr *hdr)
 {
-	ktime_t skb_dprt;
-	struct timespec dprt_time;
 	u32 msrmnt;
 	u16 tid;
 	u8 *qc;
@@ -506,9 +504,8 @@ static void ieee80211_tx_latency_end_msr
 
 	tx_lat = &sta->tx_lat[tid];
 
-	ktime_get_ts(&dprt_time); /* time stamp completion time */
-	skb_dprt = ktime_set(dprt_time.tv_sec, dprt_time.tv_nsec);
-	msrmnt = ktime_to_ms(ktime_sub(skb_dprt, skb_arv));
+	/* Calculate the latency */
+	msrmnt = ktime_to_ms(net_timedelta(skb_arv));
 
 	if (tx_lat->max < msrmnt) /* update stats */
 		tx_lat->max = msrmnt;
Index: linux/net/mac80211/tx.c
===================================================================
--- linux.orig/net/mac80211/tx.c
+++ linux/net/mac80211/tx.c
@@ -1767,15 +1767,12 @@ fail:
 static void ieee80211_tx_latency_start_msrmnt(struct ieee80211_local *local,
 					      struct sk_buff *skb)
 {
-	struct timespec skb_arv;
 	struct ieee80211_tx_latency_bin_ranges *tx_latency;
 
 	tx_latency = rcu_dereference(local->tx_latency);
 	if (!tx_latency)
 		return;
-
-	ktime_get_ts(&skb_arv);
-	skb->tstamp = ktime_set(skb_arv.tv_sec, skb_arv.tv_nsec);
+	__net_timestamp(skb);
 }
 
 /**

Re: [patch 12/13] net: mac80211: Remove silly timespec dance

[Johannes Berg]

On Wed, 2014-06-11 at 23:59 +0000, Thomas Gleixner wrote:

> +	msrmnt = ktime_to_ms(net_timedelta(skb_arv));

This is probably more of a question about net_timedelta(), but is
ktime_get_real() really appropriate for duration measurements? Isn't
that non-monotonic?

johannes

Re: [patch 12/13] net: mac80211: Remove silly timespec dance

[Thomas Gleixner]

On Thu, 12 Jun 2014, Johannes Berg wrote:

> On Wed, 2014-06-11 at 23:59 +0000, Thomas Gleixner wrote:
> 
> > +	msrmnt = ktime_to_ms(net_timedelta(skb_arv));
> 
> This is probably more of a question about net_timedelta(), but is
> ktime_get_real() really appropriate for duration measurements? Isn't
> that non-monotonic?

Well, it's monotonic, but might be affected by settimeofday().

Thanks,

	tglx

net_timedelta() affected by settimeofday() (was: [patch 12/13] net: mac80211: Remove silly timespec dance)

[Johannes Berg]

+netdev, Stephen

On Thu, 2014-06-12 at 10:19 +0200, Thomas Gleixner wrote:
> On Thu, 12 Jun 2014, Johannes Berg wrote:
> 
> > On Wed, 2014-06-11 at 23:59 +0000, Thomas Gleixner wrote:
> > 
> > > +	msrmnt = ktime_to_ms(net_timedelta(skb_arv));
> > 
> > This is probably more of a question about net_timedelta(), but is
> > ktime_get_real() really appropriate for duration measurements? Isn't
> > that non-monotonic?
> 
> Well, it's monotonic, but might be affected by settimeofday().

Right, but isn't that odd? Suddenly your delay measurement here might be
minutes, hours, or years if you settimeofday() between timestamping and
calculating the delta. That seems very strange to me, why would that be
the right behaviour in any way?

Now, it seems that there are only two current users of net_timedelta()
(in DCCP) so perhaps it's not too late to change some of this?

Maybe in general the skb timestamp should be based on a different clock
and only adjusted to real time when used in userspace?

johannes

Re: net_timedelta() affected by settimeofday() (was: [patch 12/13] net: mac80211: Remove silly timespec dance)

[Johannes Berg]

On Thu, 2014-06-12 at 10:35 +0200, Johannes Berg wrote:
> +netdev, Stephen

Well, stupid me. Fixing that netdev address.

> On Thu, 2014-06-12 at 10:19 +0200, Thomas Gleixner wrote:
> > On Thu, 12 Jun 2014, Johannes Berg wrote:
> > 
> > > On Wed, 2014-06-11 at 23:59 +0000, Thomas Gleixner wrote:
> > > 
> > > > +	msrmnt = ktime_to_ms(net_timedelta(skb_arv));
> > > 
> > > This is probably more of a question about net_timedelta(), but is
> > > ktime_get_real() really appropriate for duration measurements? Isn't
> > > that non-monotonic?
> > 
> > Well, it's monotonic, but might be affected by settimeofday().
> 
> Right, but isn't that odd? Suddenly your delay measurement here might be
> minutes, hours, or years if you settimeofday() between timestamping and
> calculating the delta. That seems very strange to me, why would that be
> the right behaviour in any way?
> 
> Now, it seems that there are only two current users of net_timedelta()
> (in DCCP) so perhaps it's not too late to change some of this?
> 
> Maybe in general the skb timestamp should be based on a different clock
> and only adjusted to real time when used in userspace?

Re: net_timedelta() affected by settimeofday() (was: [patch 12/13] net: mac80211: Remove silly timespec dance)

[Thomas Gleixner]

On Thu, 12 Jun 2014, Johannes Berg wrote:

> On Thu, 2014-06-12 at 10:35 +0200, Johannes Berg wrote:
> > +netdev, Stephen
> 
> Well, stupid me. Fixing that netdev address.
> 
> > On Thu, 2014-06-12 at 10:19 +0200, Thomas Gleixner wrote:
> > > On Thu, 12 Jun 2014, Johannes Berg wrote:
> > > 
> > > > On Wed, 2014-06-11 at 23:59 +0000, Thomas Gleixner wrote:
> > > > 
> > > > > +	msrmnt = ktime_to_ms(net_timedelta(skb_arv));
> > > > 
> > > > This is probably more of a question about net_timedelta(), but is
> > > > ktime_get_real() really appropriate for duration measurements? Isn't
> > > > that non-monotonic?
> > > 
> > > Well, it's monotonic, but might be affected by settimeofday().
> > 
> > Right, but isn't that odd? Suddenly your delay measurement here might be
> > minutes, hours, or years if you settimeofday() between timestamping and
> > calculating the delta. That seems very strange to me, why would that be
> > the right behaviour in any way?

Indeed. clock monotonic is the appropriate one for measurements.
 
> > Now, it seems that there are only two current users of net_timedelta()
> > (in DCCP) so perhaps it's not too late to change some of this?
> > 
> > Maybe in general the skb timestamp should be based on a different clock
> > and only adjusted to real time when used in userspace?

You have the same problem then, just at a different place:

    ts = ktime_get();

			settimeofday()
			   offset_mono_to_real = new value;

    userts = mono_to_real(ts);

But maybe that's not a real issue, as ktime_get_real() can race with
settimeofday() or NTP as well.

   ts = ktime_get_real();
				settimeofday();
   userts = ts;

So the user might see a weird timestamp for a packet, which cannot be
correlated with the user space gettimeofday().

Thanks,

	tglx

Re: net_timedelta() affected by settimeofday() (was: [patch 12/13] net: mac80211: Remove silly timespec dance)

[Johannes Berg]

On Thu, 2014-06-12 at 10:57 +0200, Thomas Gleixner wrote:

> > > Right, but isn't that odd? Suddenly your delay measurement here might be
> > > minutes, hours, or years if you settimeofday() between timestamping and
> > > calculating the delta. That seems very strange to me, why would that be
> > > the right behaviour in any way?
> 
> Indeed. clock monotonic is the appropriate one for measurements.

And that's what we had here with ktime_get_ts()? I thought so, just
making sure.
 
> > > Now, it seems that there are only two current users of net_timedelta()
> > > (in DCCP) so perhaps it's not too late to change some of this?
> > > 
> > > Maybe in general the skb timestamp should be based on a different clock
> > > and only adjusted to real time when used in userspace?
> 
> You have the same problem then, just at a different place:
> 
>     ts = ktime_get();
> 
> 			settimeofday()
> 			   offset_mono_to_real = new value;
> 
>     userts = mono_to_real(ts);

Right. I'm not really sure if that's an issue though.

> But maybe that's not a real issue, as ktime_get_real() can race with
> settimeofday() or NTP as well.
> 
>    ts = ktime_get_real();
> 				settimeofday();
>    userts = ts;
> 
> So the user might see a weird timestamp for a packet, which cannot be
> correlated with the user space gettimeofday().

Right, once settimeofday() is called the timestamps from before/during
it can't really be correlated any more.

This is part of the userspace API already, but might it have been better
to expose the monotonic clock, since userspace can also get at it? Not
sure.

Either way it's an issue I guess; however I'm thinking your patch is
making it worse for the measurement in this particular code (where the
userspace issue doesn't come in, it should never be accessible there)

johannes

Re: net_timedelta() affected by settimeofday() (was: [patch 12/13] net: mac80211: Remove silly timespec dance)

[Thomas Gleixner]

On Thu, 12 Jun 2014, Johannes Berg wrote:

> On Thu, 2014-06-12 at 10:57 +0200, Thomas Gleixner wrote:
> 
> > > > Right, but isn't that odd? Suddenly your delay measurement here might be
> > > > minutes, hours, or years if you settimeofday() between timestamping and
> > > > calculating the delta. That seems very strange to me, why would that be
> > > > the right behaviour in any way?
> > 
> > Indeed. clock monotonic is the appropriate one for measurements.
> 
> And that's what we had here with ktime_get_ts()? I thought so, just
> making sure.
>  
> > > > Now, it seems that there are only two current users of net_timedelta()
> > > > (in DCCP) so perhaps it's not too late to change some of this?
> > > > 
> > > > Maybe in general the skb timestamp should be based on a different clock
> > > > and only adjusted to real time when used in userspace?
> > 
> > You have the same problem then, just at a different place:
> > 
> >     ts = ktime_get();
> > 
> > 			settimeofday()
> > 			   offset_mono_to_real = new value;
> > 
> >     userts = mono_to_real(ts);
> 
> Right. I'm not really sure if that's an issue though.
> 
> > But maybe that's not a real issue, as ktime_get_real() can race with
> > settimeofday() or NTP as well.
> > 
> >    ts = ktime_get_real();
> > 				settimeofday();
> >    userts = ts;
> > 
> > So the user might see a weird timestamp for a packet, which cannot be
> > correlated with the user space gettimeofday().
> 
> Right, once settimeofday() is called the timestamps from before/during
> it can't really be correlated any more.
> 
> This is part of the userspace API already, but might it have been better
> to expose the monotonic clock, since userspace can also get at it? Not
> sure.
> 
> Either way it's an issue I guess; however I'm thinking your patch is
> making it worse for the measurement in this particular code (where the
> userspace issue doesn't come in, it should never be accessible there)

Fair enough. Still the timespec is silly. Here is an updated version.

Thanks,

	tglx

------------------>

Subject: net: Mac80211: Remove silly timespec dance
From: Thomas Gleixner <tglx@linutronix.de>
Date: Wed, 11 Jun 2014 23:59:18 -0000

Converting time from one format to another seems to give coders a warm
and fuzzy feeling.

Use the proper interfaces.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: John W. Linville <linville@tuxdriver.com>
---
 net/mac80211/status.c |    7 ++-----
 net/mac80211/tx.c     |    5 +----
 2 files changed, 3 insertions(+), 9 deletions(-)


Index: tip/net/mac80211/status.c
===================================================================
--- tip.orig/net/mac80211/status.c
+++ tip/net/mac80211/status.c
@@ -473,8 +473,6 @@ static void ieee80211_tx_latency_end_msr
 					    struct sta_info *sta,
 					    struct ieee80211_hdr *hdr)
 {
-	ktime_t skb_dprt;
-	struct timespec dprt_time;
 	u32 msrmnt;
 	u16 tid;
 	u8 *qc;
@@ -506,9 +504,8 @@ static void ieee80211_tx_latency_end_msr
 
 	tx_lat = &sta->tx_lat[tid];
 
-	ktime_get_ts(&dprt_time); /* time stamp completion time */
-	skb_dprt = ktime_set(dprt_time.tv_sec, dprt_time.tv_nsec);
-	msrmnt = ktime_to_ms(ktime_sub(skb_dprt, skb_arv));
+	/* Calculate the latency */
+	msrmnt = ktime_to_ms(ktime_sub(ktime_get(), skb_arv);
 
 	if (tx_lat->max < msrmnt) /* update stats */
 		tx_lat->max = msrmnt;
Index: tip/net/mac80211/tx.c
===================================================================
--- tip.orig/net/mac80211/tx.c
+++ tip/net/mac80211/tx.c
@@ -1767,15 +1767,12 @@ fail:
 static void ieee80211_tx_latency_start_msrmnt(struct ieee80211_local *local,
 					      struct sk_buff *skb)
 {
-	struct timespec skb_arv;
 	struct ieee80211_tx_latency_bin_ranges *tx_latency;
 
 	tx_latency = rcu_dereference(local->tx_latency);
 	if (!tx_latency)
 		return;
-
-	ktime_get_ts(&skb_arv);
-	skb->tstamp = ktime_set(skb_arv.tv_sec, skb_arv.tv_nsec);
+	skb->tstamp = ktime_get();
 }
 
 /**

Re: net_timedelta() affected by settimeofday() (was: [patch 12/13] net: mac80211: Remove silly timespec dance)

[Johannes Berg]

On Thu, 2014-06-12 at 16:09 +0200, Thomas Gleixner wrote:

> Fair enough. Still the timespec is silly. Here is an updated version.

Makes sense, thanks. I've applied this (mac80211-next)

johannes

[patch 13/13] tomoyo: Use sensible time interface

[Thomas Gleixner]

[-- Attachment #1: tomoyo-use-sensible-time-interface.patch --]
[-- Type: text/plain, Size: 1603 bytes --]

There is no point in calling gettimeofday if only the seconds part of
the timespec is used. Use get_seconds() instead. It's not only the
proper interface it's also faster.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Kentaro Takeda <takedakn@nttdata.co.jp>
Cc: linux-security-module@vger.kernel.org
---
 security/tomoyo/audit.c  |    8 +++-----
 security/tomoyo/common.c |    4 +---
 2 files changed, 4 insertions(+), 8 deletions(-)

Index: linux/security/tomoyo/audit.c
===================================================================
--- linux.orig/security/tomoyo/audit.c
+++ linux/security/tomoyo/audit.c
@@ -155,11 +155,9 @@ static char *tomoyo_print_header(struct
 	u8 i;
 	if (!buffer)
 		return NULL;
-	{
-		struct timeval tv;
-		do_gettimeofday(&tv);
-		tomoyo_convert_time(tv.tv_sec, &stamp);
-	}
+
+	tomoyo_convert_time(get_seconds(), &stamp);
+
 	pos = snprintf(buffer, tomoyo_buffer_len - 1,
 		       "#%04u/%02u/%02u %02u:%02u:%02u# profile=%u mode=%s "
 		       "granted=%s (global-pid=%u) task={ pid=%u ppid=%u "
Index: linux/security/tomoyo/common.c
===================================================================
--- linux.orig/security/tomoyo/common.c
+++ linux/security/tomoyo/common.c
@@ -2267,13 +2267,11 @@ static unsigned int tomoyo_stat_modified
  */
 void tomoyo_update_stat(const u8 index)
 {
-	struct timeval tv;
-	do_gettimeofday(&tv);
 	/*
 	 * I don't use atomic operations because race condition is not fatal.
 	 */
 	tomoyo_stat_updated[index]++;
-	tomoyo_stat_modified[index] = tv.tv_sec;
+	tomoyo_stat_modified[index] = get_seconds();
 }
 
 /**

Re: [patch 13/13] tomoyo: Use sensible time interface

[John Stultz]

On Wed, Jun 11, 2014 at 4:59 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> There is no point in calling gettimeofday if only the seconds part of
> the timespec is used. Use get_seconds() instead. It's not only the
> proper interface it's also faster.

My only caution here is you only get tick-granular time here. So if
the second rolled over after the last tick, you'd get the previous
second when you call get_seconds(). This can cause some surprising
effects if the get_seconds() return value is mixed with clocksource
granular gettimeofday() calls.

thanks
-john

Re: [patch 13/13] tomoyo: Use sensible time interface

[Thomas Gleixner]

On Wed, 11 Jun 2014, John Stultz wrote:

> On Wed, Jun 11, 2014 at 4:59 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > There is no point in calling gettimeofday if only the seconds part of
> > the timespec is used. Use get_seconds() instead. It's not only the
> > proper interface it's also faster.
> 
> My only caution here is you only get tick-granular time here. So if
> the second rolled over after the last tick, you'd get the previous
> second when you call get_seconds(). This can cause some surprising
> effects if the get_seconds() return value is mixed with clocksource
> granular gettimeofday() calls.

If the whole thing only cares about the seconds value, then where is
the problem?

Even if you call gettimeofday() then you still can observe this

gettimeofday(ts)
	ts.tv_sec = 99
	ts.tv_nsec = 999999999

So if you readout the related value ONE nanosecond later, then this
value will have
	ts.tv_sec = 100
	ts.tv_nsec = 0

So what's the point? The tomoyo code chose to take seconds granular
time stamps for whatever reasons. So it should be able to deal with
that, right?

Thanks,

	tglx

Re: [patch 13/13] tomoyo: Use sensible time interface

[John Stultz]

On Wed, Jun 11, 2014 at 5:22 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Wed, 11 Jun 2014, John Stultz wrote:
>
>> On Wed, Jun 11, 2014 at 4:59 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > There is no point in calling gettimeofday if only the seconds part of
>> > the timespec is used. Use get_seconds() instead. It's not only the
>> > proper interface it's also faster.
>>
>> My only caution here is you only get tick-granular time here. So if
>> the second rolled over after the last tick, you'd get the previous
>> second when you call get_seconds(). This can cause some surprising
>> effects if the get_seconds() return value is mixed with clocksource
>> granular gettimeofday() calls.
>
> If the whole thing only cares about the seconds value, then where is
> the problem?
>
> Even if you call gettimeofday() then you still can observe this
>
> gettimeofday(ts)
>         ts.tv_sec = 99
>         ts.tv_nsec = 999999999
>
> So if you readout the related value ONE nanosecond later, then this
> value will have
>         ts.tv_sec = 100
>         ts.tv_nsec = 0
>
> So what's the point? The tomoyo code chose to take seconds granular
> time stamps for whatever reasons. So it should be able to deal with
> that, right?

No, the problem I'm warning about is if they were using gettimeofday()
elsewhere in relation to those timestamps, they could see something
like:

do_gettimeofday()  { 99, 888....}
get_seconds()   { 99 }
do_gettimeofday()  { 99, 999....}
get_seconds()   { 99 }
do_gettimeofday()  { 100, 000....}
get_seconds()   { 99 }
do_gettimeofday()  { 100, 011....}
get_seconds()   { 100 }

This is the same problem people come across occasionally if they call
gettimeofday, then create a file and fret that the file's timestamp
seems to be before the gettimefoday call, and its all due to comparing
timestamps with different granularities.

I'm not saying its a problem in this case, but I'm just throwing up
some additional caution since the change you're making isn't
completely equivalent.

thanks
-john

Re: [patch 13/13] tomoyo: Use sensible time interface

[Thomas Gleixner]

On Wed, 11 Jun 2014, John Stultz wrote:
> On Wed, Jun 11, 2014 at 5:22 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Wed, 11 Jun 2014, John Stultz wrote:
> >
> >> On Wed, Jun 11, 2014 at 4:59 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >> > There is no point in calling gettimeofday if only the seconds part of
> >> > the timespec is used. Use get_seconds() instead. It's not only the
> >> > proper interface it's also faster.
> >>
> >> My only caution here is you only get tick-granular time here. So if
> >> the second rolled over after the last tick, you'd get the previous
> >> second when you call get_seconds(). This can cause some surprising
> >> effects if the get_seconds() return value is mixed with clocksource
> >> granular gettimeofday() calls.
> >
> > If the whole thing only cares about the seconds value, then where is
> > the problem?
> >
> > Even if you call gettimeofday() then you still can observe this
> >
> > gettimeofday(ts)
> >         ts.tv_sec = 99
> >         ts.tv_nsec = 999999999
> >
> > So if you readout the related value ONE nanosecond later, then this
> > value will have
> >         ts.tv_sec = 100
> >         ts.tv_nsec = 0
> >
> > So what's the point? The tomoyo code chose to take seconds granular
> > time stamps for whatever reasons. So it should be able to deal with
> > that, right?
> 
> No, the problem I'm warning about is if they were using gettimeofday()
> elsewhere in relation to those timestamps, they could see something
> like:
> 
> do_gettimeofday()  { 99, 888....}
> get_seconds()   { 99 }
> do_gettimeofday()  { 99, 999....}
> get_seconds()   { 99 }
> do_gettimeofday()  { 100, 000....}
> get_seconds()   { 99 }
> do_gettimeofday()  { 100, 011....}
> get_seconds()   { 100 }
> 
> This is the same problem people come across occasionally if they call
> gettimeofday, then create a file and fret that the file's timestamp
> seems to be before the gettimefoday call, and its all due to comparing
> timestamps with different granularities.
> 
I'm aware of that, but there are only two places in that code which
deal with time and both are calling do_gettimeofday and both just use
the tv_sec part of it. And both of them are statistics.

One part of says clearly:

        * I don't use atomic operations because race condition is not fatal.

Thanks,

	tglx

Re: [patch 13/13] tomoyo: Use sensible time interface

[Tetsuo Handa]

Thomas Gleixner wrote:
> There is no point in calling gettimeofday if only the seconds part of
> the timespec is used. Use get_seconds() instead. It's not only the
> proper interface it's also faster.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Kentaro Takeda <takedakn@nttdata.co.jp>
> Cc: linux-security-module@vger.kernel.org

Acked-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

John Stultz wrote:
> do_gettimeofday()  { 99, 888....}
> get_seconds()   { 99 }
> do_gettimeofday()  { 99, 999....}
> get_seconds()   { 99 }
> do_gettimeofday()  { 100, 000....}
> get_seconds()   { 99 }
> do_gettimeofday()  { 100, 011....}
> get_seconds()   { 100 }

That will be acceptable error for TOMOYO. Thank you.

[tip:timers/core] tomoyo: Use sensible time interface

[tip-bot for Thomas Gleixner]

Commit-ID:  77f4fa089c724adc3a87c10eb031bca91b144ac0
Gitweb:     http://git.kernel.org/tip/77f4fa089c724adc3a87c10eb031bca91b144ac0
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Wed, 11 Jun 2014 23:59:19 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Thu, 12 Jun 2014 16:18:45 +0200

tomoyo: Use sensible time interface

There is no point in calling gettimeofday if only the seconds part of
the timespec is used. Use get_seconds() instead. It's not only the
proper interface it's also faster.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Kentaro Takeda <takedakn@nttdata.co.jp>
Cc: linux-security-module@vger.kernel.org
Link: http://lkml.kernel.org/r/20140611234607.775273584@linutronix.de
---
 security/tomoyo/audit.c  | 8 +++-----
 security/tomoyo/common.c | 4 +---
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/security/tomoyo/audit.c b/security/tomoyo/audit.c
index c1b0037..3ffa4f5 100644
--- a/security/tomoyo/audit.c
+++ b/security/tomoyo/audit.c
@@ -155,11 +155,9 @@ static char *tomoyo_print_header(struct tomoyo_request_info *r)
 	u8 i;
 	if (!buffer)
 		return NULL;
-	{
-		struct timeval tv;
-		do_gettimeofday(&tv);
-		tomoyo_convert_time(tv.tv_sec, &stamp);
-	}
+
+	tomoyo_convert_time(get_seconds(), &stamp);
+
 	pos = snprintf(buffer, tomoyo_buffer_len - 1,
 		       "#%04u/%02u/%02u %02u:%02u:%02u# profile=%u mode=%s "
 		       "granted=%s (global-pid=%u) task={ pid=%u ppid=%u "
diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c
index 283862a..e0fb750 100644
--- a/security/tomoyo/common.c
+++ b/security/tomoyo/common.c
@@ -2267,13 +2267,11 @@ static unsigned int tomoyo_stat_modified[TOMOYO_MAX_POLICY_STAT];
  */
 void tomoyo_update_stat(const u8 index)
 {
-	struct timeval tv;
-	do_gettimeofday(&tv);
 	/*
 	 * I don't use atomic operations because race condition is not fatal.
 	 */
 	tomoyo_stat_updated[index]++;
-	tomoyo_stat_modified[index] = tv.tv_sec;
+	tomoyo_stat_modified[index] = get_seconds();
 }
 
 /**