[patch 0/5] hrtimer: Cleanup usage trainwrecks treewide

[patch 0/5] hrtimer: Cleanup usage trainwrecks treewide

[Thomas Gleixner]

While doing a tree wide audit of hrtimer users I found several bugs
which can result in data corruption.

Fix the bugs and update the documentation in the hope that people actually read it.

Thanks,

	tglx

[patch 1/5] perf: Fixup hrtimer forward wreckage

[Thomas Gleixner]

[-- Attachment #1: perf-remove-hrtimer-forward-wreckage.patch --]
[-- Type: text/plain, Size: 3209 bytes --]

perf_event_mux_interval_ms_store() tries to apply an updated
hrtimer_interval to all possible cpus in a completely unsafe way. The
changelog of the offending commit says:

 "In the 5th version, we handle the reprogramming of the hrtimer using
  hrtimer_forward_now(). That way, we sync up to new timer value
  quickly (suggested by Jiri Olsa)."

The hrtimer reprogramming is completely unrelated to
hrtimer_forward_now(). hrtimer_forward_now() merily forwards the
expiry time of the hrtimer past now and that's where things get really
bad in this code:

The forward function is called on enqueued timers. That means the
update of the expiry time corrupts the ordering of the hrtimer rbtree.

The proper way to update hrtimers on remote cpus is to use a smp
function call on all online cpus and perform the update locally by
canceling the timer, forwarding and restarting it.

Fixes: 62b856397927 "perf: Add sysfs entry to adjust multiplexing interval per PMU"
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Stephane Eranian <eranian@google.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: stable@vger.kernel.org
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
---
 kernel/events/core.c |   36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

Index: linux/kernel/events/core.c
===================================================================
--- linux.orig/kernel/events/core.c
+++ linux/kernel/events/core.c
@@ -6827,13 +6827,27 @@ perf_event_mux_interval_ms_show(struct d
 	return snprintf(page, PAGE_SIZE-1, "%d\n", pmu->hrtimer_interval_ms);
 }
 
+static void perf_event_mux_update_interval(void *info)
+{
+	struct pmu *pmu = info;
+	struct perf_cpu_context *cpuctx = this_cpu_ptr(pmu->pmu_cpu_context);
+	int was_armed = hrtimer_cancel(&cpuctx->hrtimer);
+
+	/* Update the interval and restart the timer with the new interval */
+	cpuctx->hrtimer_interval = ms_to_ktime(pmu->hrtimer_interval_ms);
+	if (was_armed) {
+		hrtimer_forward_now(&cpuctx->hrtimer, cpuctx->hrtimer_interval);
+		hrtimer_start_expires(&cpuctx->hrtimer, HRTIMER_MODE_ABS);
+	}
+}
+
 static ssize_t
 perf_event_mux_interval_ms_store(struct device *dev,
 				 struct device_attribute *attr,
 				 const char *buf, size_t count)
 {
 	struct pmu *pmu = dev_get_drvdata(dev);
-	int timer, cpu, ret;
+	int timer, ret;
 
 	ret = kstrtoint(buf, 0, &timer);
 	if (ret)
@@ -6842,22 +6856,12 @@ perf_event_mux_interval_ms_store(struct
 	if (timer < 1)
 		return -EINVAL;
 
-	/* same value, noting to do */
-	if (timer == pmu->hrtimer_interval_ms)
-		return count;
-
-	pmu->hrtimer_interval_ms = timer;
-
-	/* update all cpuctx for this PMU */
-	for_each_possible_cpu(cpu) {
-		struct perf_cpu_context *cpuctx;
-		cpuctx = per_cpu_ptr(pmu->pmu_cpu_context, cpu);
-		cpuctx->hrtimer_interval = ns_to_ktime(NSEC_PER_MSEC * timer);
-
-		if (hrtimer_active(&cpuctx->hrtimer))
-			hrtimer_forward_now(&cpuctx->hrtimer, cpuctx->hrtimer_interval);
+	if (timer != pmu->hrtimer_interval_ms) {
+		get_online_cpus();
+		pmu->hrtimer_interval_ms = timer;
+		on_each_cpu(perf_event_mux_update_interval, pmu, 1);
+		put_online_cpus();
 	}
-
 	return count;
 }
 static DEVICE_ATTR_RW(perf_event_mux_interval_ms);

Re: [patch 1/5] perf: Fixup hrtimer forward wreckage

[Stephane Eranian]

On Mon, Apr 13, 2015 at 2:02 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>
> perf_event_mux_interval_ms_store() tries to apply an updated
> hrtimer_interval to all possible cpus in a completely unsafe way. The
> changelog of the offending commit says:
>
>  "In the 5th version, we handle the reprogramming of the hrtimer using
>   hrtimer_forward_now(). That way, we sync up to new timer value
>   quickly (suggested by Jiri Olsa)."
>
> The hrtimer reprogramming is completely unrelated to
> hrtimer_forward_now(). hrtimer_forward_now() merily forwards the
> expiry time of the hrtimer past now and that's where things get really
> bad in this code:
>
> The forward function is called on enqueued timers. That means the
> update of the expiry time corrupts the ordering of the hrtimer rbtree.
>
> The proper way to update hrtimers on remote cpus is to use a smp
> function call on all online cpus and perform the update locally by
> canceling the timer, forwarding and restarting it.
>
> Fixes: 62b856397927 "perf: Add sysfs entry to adjust multiplexing interval per PMU"
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Stephane Eranian <eranian@google.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: stable@vger.kernel.org
> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>


Thanks for fixing this Thomas!

>
> ---
>  kernel/events/core.c |   36 ++++++++++++++++++++----------------
>  1 file changed, 20 insertions(+), 16 deletions(-)
>
> Index: linux/kernel/events/core.c
> ===================================================================
> --- linux.orig/kernel/events/core.c
> +++ linux/kernel/events/core.c
> @@ -6827,13 +6827,27 @@ perf_event_mux_interval_ms_show(struct d
>         return snprintf(page, PAGE_SIZE-1, "%d\n", pmu->hrtimer_interval_ms);
>  }
>
> +static void perf_event_mux_update_interval(void *info)
> +{
> +       struct pmu *pmu = info;
> +       struct perf_cpu_context *cpuctx = this_cpu_ptr(pmu->pmu_cpu_context);
> +       int was_armed = hrtimer_cancel(&cpuctx->hrtimer);
> +
> +       /* Update the interval and restart the timer with the new interval */
> +       cpuctx->hrtimer_interval = ms_to_ktime(pmu->hrtimer_interval_ms);
> +       if (was_armed) {
> +               hrtimer_forward_now(&cpuctx->hrtimer, cpuctx->hrtimer_interval);
> +               hrtimer_start_expires(&cpuctx->hrtimer, HRTIMER_MODE_ABS);
> +       }
> +}
> +
>  static ssize_t
>  perf_event_mux_interval_ms_store(struct device *dev,
>                                  struct device_attribute *attr,
>                                  const char *buf, size_t count)
>  {
>         struct pmu *pmu = dev_get_drvdata(dev);
> -       int timer, cpu, ret;
> +       int timer, ret;
>
>         ret = kstrtoint(buf, 0, &timer);
>         if (ret)
> @@ -6842,22 +6856,12 @@ perf_event_mux_interval_ms_store(struct
>         if (timer < 1)
>                 return -EINVAL;
>
> -       /* same value, noting to do */
> -       if (timer == pmu->hrtimer_interval_ms)
> -               return count;
> -
> -       pmu->hrtimer_interval_ms = timer;
> -
> -       /* update all cpuctx for this PMU */
> -       for_each_possible_cpu(cpu) {
> -               struct perf_cpu_context *cpuctx;
> -               cpuctx = per_cpu_ptr(pmu->pmu_cpu_context, cpu);
> -               cpuctx->hrtimer_interval = ns_to_ktime(NSEC_PER_MSEC * timer);
> -
> -               if (hrtimer_active(&cpuctx->hrtimer))
> -                       hrtimer_forward_now(&cpuctx->hrtimer, cpuctx->hrtimer_interval);
> +       if (timer != pmu->hrtimer_interval_ms) {
> +               get_online_cpus();
> +               pmu->hrtimer_interval_ms = timer;
> +               on_each_cpu(perf_event_mux_update_interval, pmu, 1);
> +               put_online_cpus();
>         }
> -
>         return count;
>  }
>  static DEVICE_ATTR_RW(perf_event_mux_interval_ms);
>
>

[patch 2/5] s390: crypto: Protect poll timeout update

[Thomas Gleixner]

[-- Attachment #1: s390-crypto-protect-hrtimer-update.patch --]
[-- Type: text/plain, Size: 2719 bytes --]

The poll timeout update via sysfs runs completely unprotected against
other usage sites of the timer. Take the proper lock before fiddling
with the timer.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Tuchscherer <ingo.tuchscherer@de.ibm.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: linux-s390@vger.kernel.org
---

P.S: I have serious doubts about the following snippets in that code:

@@ -1174,11 +1174,13 @@ static ssize_t poll_timeout_store(struct
       poll_timeout = time;
       hr_time = ktime_set(0, poll_timeout);

       spin_lock_bh(&ap_poll_timer_lock);
       if (!hrtimer_is_queued(&ap_poll_timer) ||
-->        !hrtimer_forward(&ap_poll_timer, hrtimer_get_expires(&ap_poll_timer), hr_time)) {

Why does it check, whether the forwarding resulted in an overrun?

If the timer is not queued, then it is either expired or has never
been started. So if the poll timeout gets written, then the timer is
started when

	timer->expires + hr_time < now

This results in random behaviour at best.

                hrtimer_set_expires(&ap_poll_timer, hr_time);
                hrtimer_start_expires(&ap_poll_timer, HRTIMER_MODE_ABS);
        }
@@ -1552,11 +1553,16 @@ static inline void __ap_schedule_poll_timeout
       spin_lock_bh(&ap_poll_timer_lock);
       if (hrtimer_is_queued(&ap_poll_timer) || ap_suspend_flag)
                goto out;
--->   if (ktime_to_ns(hrtimer_expires_remaining(&ap_poll_timer)) <= 0) {

The check above does not make any sense either. Again, if the timer is
not queued then it is either expired or was never started. As the
timer is never canceled except on module unload the condition is
always true.

               hr_time = ktime_set(0, poll_timeout);
               hrtimer_forward_now(&ap_poll_timer, hr_time);
               hrtimer_restart(&ap_poll_timer);
       }

But that's not scope of that patch and I leave this to the s390
wizards to digest.

---

 drivers/s390/crypto/ap_bus.c |    2 ++
 1 file changed, 2 insertions(+)

Index: linux/drivers/s390/crypto/ap_bus.c
===================================================================
--- linux.orig/drivers/s390/crypto/ap_bus.c
+++ linux/drivers/s390/crypto/ap_bus.c
@@ -1174,11 +1174,13 @@ static ssize_t poll_timeout_store(struct
 	poll_timeout = time;
 	hr_time = ktime_set(0, poll_timeout);
 
+	spin_lock_bh(&ap_poll_timer_lock);
 	if (!hrtimer_is_queued(&ap_poll_timer) ||
 	    !hrtimer_forward(&ap_poll_timer, hrtimer_get_expires(&ap_poll_timer), hr_time)) {
 		hrtimer_set_expires(&ap_poll_timer, hr_time);
 		hrtimer_start_expires(&ap_poll_timer, HRTIMER_MODE_ABS);
 	}
+	spin_unlock_bh(&ap_poll_timer_lock);
 	return count;
 }
 

Re: [patch 2/5] s390: crypto: Protect poll timeout update

[Ingo Tuchscherer]

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


You're right. Thanks for pointing this out.
I'll fix these issues and bring them upstream.

Mit freundlichen Grüßen / Kind regards

Ingo Tuchscherer

Software Development - Linux on z Systems
IBM Systems &Technology Group
                                                                       
                                                                       
                                                                       
                                                                       
                                                                       
 Phone:            +49-7031-16-1986                 IBM Deutschland                          (Embedded
                                                                                           image moved
                                                                                              to file:
                                                                                         pic14137.gif)
                                                                       
 E-Mail:           ingo.tuchscherer@de.ibm.com      Schoenaicher Str. 220
                                                                       
                                                    71032 Boeblingen   
                                                                       
                                                    Germany            
                                                                       
                                                                       
                                                                       
                                                                       
                                                                       
 IBM Deutschland                                                       
 Research &                                                            
 Development                                                           
 GmbH /                                                                
 Vorsitzender des                                                      
 Aufsichtsrats:                                                        
 Martina Koederitz                                                     
                                                                       
 Geschäftsführung:                                                 
 Dirk Wittkopp                                                         
 Sitz der                                                              
 Gesellschaft:                                                         
 Böblingen /                                                         
 Registergericht:                                                      
 Amtsgericht                                                           
 Stuttgart, HRB                                                        
 243294                                                                
                                                                       





From:	Thomas Gleixner <tglx@linutronix.de>
To:	LKML <linux-kernel@vger.kernel.org>
Cc:	Peter Zijlstra <peterz@infradead.org>, Ingo Molnar
            <mingo@kernel.org>, Ingo Tuchscherer/Germany/IBM@IBMDE,
            mschwid2@linux.vnet.ibm.com, heicars2@linux.vnet.ibm.com,
            linux-s390@vger.kernel.org
Date:	04/13/2015 11:02 PM
Subject:	[patch 2/5] s390: crypto: Protect poll timeout update



The poll timeout update via sysfs runs completely unprotected against
other usage sites of the timer. Take the proper lock before fiddling
with the timer.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Tuchscherer <ingo.tuchscherer@de.ibm.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: linux-s390@vger.kernel.org
---

P.S: I have serious doubts about the following snippets in that code:

@@ -1174,11 +1174,13 @@ static ssize_t poll_timeout_store(struct
       poll_timeout = time;
       hr_time = ktime_set(0, poll_timeout);

       spin_lock_bh(&ap_poll_timer_lock);
       if (!hrtimer_is_queued(&ap_poll_timer) ||
-->        !hrtimer_forward(&ap_poll_timer, hrtimer_get_expires
(&ap_poll_timer), hr_time)) {

Why does it check, whether the forwarding resulted in an overrun?

If the timer is not queued, then it is either expired or has never
been started. So if the poll timeout gets written, then the timer is
started when

		 timer->expires + hr_time < now

This results in random behaviour at best.

                hrtimer_set_expires(&ap_poll_timer, hr_time);
                hrtimer_start_expires(&ap_poll_timer, HRTIMER_MODE_ABS);
        }
@@ -1552,11 +1553,16 @@ static inline void __ap_schedule_poll_timeout
       spin_lock_bh(&ap_poll_timer_lock);
       if (hrtimer_is_queued(&ap_poll_timer) || ap_suspend_flag)
                goto out;
--->   if (ktime_to_ns(hrtimer_expires_remaining(&ap_poll_timer)) <= 0) {

The check above does not make any sense either. Again, if the timer is
not queued then it is either expired or was never started. As the
timer is never canceled except on module unload the condition is
always true.

               hr_time = ktime_set(0, poll_timeout);
               hrtimer_forward_now(&ap_poll_timer, hr_time);
               hrtimer_restart(&ap_poll_timer);
       }

But that's not scope of that patch and I leave this to the s390
wizards to digest.

---

 drivers/s390/crypto/ap_bus.c |    2 ++
 1 file changed, 2 insertions(+)

Index: linux/drivers/s390/crypto/ap_bus.c
===================================================================
--- linux.orig/drivers/s390/crypto/ap_bus.c
+++ linux/drivers/s390/crypto/ap_bus.c
@@ -1174,11 +1174,13 @@ static ssize_t poll_timeout_store(struct
 		 poll_timeout = time;
 		 hr_time = ktime_set(0, poll_timeout);

+		 spin_lock_bh(&ap_poll_timer_lock);
 		 if (!hrtimer_is_queued(&ap_poll_timer) ||
 		     !hrtimer_forward(&ap_poll_timer, hrtimer_get_expires
(&ap_poll_timer), hr_time)) {
 		 		 hrtimer_set_expires(&ap_poll_timer, hr_time);
 		 		 hrtimer_start_expires(&ap_poll_timer,
HRTIMER_MODE_ABS);
 		 }
+		 spin_unlock_bh(&ap_poll_timer_lock);
 		 return count;
 }




[-- Attachment #2: pic14137.gif --]
[-- Type: image/gif, Size: 1851 bytes --]

[patch 3/5] hrtimer: Document hrtimer_forward[_now]() proper

[Thomas Gleixner]

[-- Attachment #1: hrtimer-document-hrtimer-forward-now.patch --]
[-- Type: text/plain, Size: 2062 bytes --]

Document the calling context conditions.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/hrtimer.h |   17 ++++++++++++++++-
 kernel/time/hrtimer.c   |    8 ++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

Index: linux/include/linux/hrtimer.h
===================================================================
--- linux.orig/include/linux/hrtimer.h
+++ linux/include/linux/hrtimer.h
@@ -418,7 +418,22 @@ static inline int hrtimer_callback_runni
 extern u64
 hrtimer_forward(struct hrtimer *timer, ktime_t now, ktime_t interval);
 
-/* Forward a hrtimer so it expires after the hrtimer's current now */
+/**
+ * hrtimer_forward_now - forward the timer expiry so it expires after now
+ * @timer:	hrtimer to forward
+ * @interval:	the interval to forward
+ *
+ * Forward the timer expiry so it will expire after the current time
+ * of the hrtimer clock base. Returns the number of overruns.
+ *
+ * Can be safely called from the callback function of @timer. If
+ * called from other contexts @timer must neither be enqueued nor
+ * running the callback and the caller needs to take care of
+ * serialization.
+ *
+ * Note: This only updates the timer expiry value and does not requeue
+ * the timer.
+ */
 static inline u64 hrtimer_forward_now(struct hrtimer *timer,
 				      ktime_t interval)
 {
Index: linux/kernel/time/hrtimer.c
===================================================================
--- linux.orig/kernel/time/hrtimer.c
+++ linux/kernel/time/hrtimer.c
@@ -801,6 +801,14 @@ void unlock_hrtimer_base(const struct hr
  *
  * Forward the timer expiry so it will expire in the future.
  * Returns the number of overruns.
+ *
+ * Can be safely called from the callback function of @timer. If
+ * called from other contexts @timer must neither be enqueued nor
+ * running the callback and the caller needs to take care of
+ * serialization.
+ *
+ * Note: This only updates the timer expiry value and does not requeue
+ * the timer.
  */
 u64 hrtimer_forward(struct hrtimer *timer, ktime_t now, ktime_t interval)
 {

[tip:timers/core] hrtimer: Document hrtimer_forward[_now]() proper

[tip-bot for Thomas Gleixner]

Commit-ID:  91e5a2170e795989da9f90c18ba18984f23acc5b
Gitweb:     http://git.kernel.org/tip/91e5a2170e795989da9f90c18ba18984f23acc5b
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Mon, 13 Apr 2015 21:02:22 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 22 Apr 2015 17:06:48 +0200

hrtimer: Document hrtimer_forward[_now]() proper

Document the calling context conditions.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150413210035.178751779@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/hrtimer.h | 17 ++++++++++++++++-
 kernel/time/hrtimer.c   |  8 ++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index 05f6df1..7770676 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -418,7 +418,22 @@ static inline int hrtimer_callback_running(struct hrtimer *timer)
 extern u64
 hrtimer_forward(struct hrtimer *timer, ktime_t now, ktime_t interval);
 
-/* Forward a hrtimer so it expires after the hrtimer's current now */
+/**
+ * hrtimer_forward_now - forward the timer expiry so it expires after now
+ * @timer:	hrtimer to forward
+ * @interval:	the interval to forward
+ *
+ * Forward the timer expiry so it will expire after the current time
+ * of the hrtimer clock base. Returns the number of overruns.
+ *
+ * Can be safely called from the callback function of @timer. If
+ * called from other contexts @timer must neither be enqueued nor
+ * running the callback and the caller needs to take care of
+ * serialization.
+ *
+ * Note: This only updates the timer expiry value and does not requeue
+ * the timer.
+ */
 static inline u64 hrtimer_forward_now(struct hrtimer *timer,
 				      ktime_t interval)
 {
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 76d4bd9..b1a74ee 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -801,6 +801,14 @@ void unlock_hrtimer_base(const struct hrtimer *timer, unsigned long *flags)
  *
  * Forward the timer expiry so it will expire in the future.
  * Returns the number of overruns.
+ *
+ * Can be safely called from the callback function of @timer. If
+ * called from other contexts @timer must neither be enqueued nor
+ * running the callback and the caller needs to take care of
+ * serialization.
+ *
+ * Note: This only updates the timer expiry value and does not requeue
+ * the timer.
  */
 u64 hrtimer_forward(struct hrtimer *timer, ktime_t now, ktime_t interval)
 {

[patch 4/5] net: hip04: Make tx coalesce timer actually work

[Thomas Gleixner]

[-- Attachment #1: net-hip04-fix-hrtimer-wreckage.patch --]
[-- Type: text/plain, Size: 2871 bytes --]

The code sets the expiry value of the timer to a relative value and
starts it with hrtimer_start_expires. That's fine, but that only works
once. The timer is started in relative mode, so the expiry value gets
overwritten with the absolut expiry time (now + expiry).

So once the timer expired, a new call to hrtimer_start_expires results
in an immidiately expired timer, because the expiry value is
already in the past.

Use the proper mechanisms to (re)start the timer in the intended way.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: dingtianhong <dingtianhong@huawei.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Cc: netdev@vger.kernel.org
---
 drivers/net/ethernet/hisilicon/hip04_eth.c |   18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

Index: linux/drivers/net/ethernet/hisilicon/hip04_eth.c
===================================================================
--- linux.orig/drivers/net/ethernet/hisilicon/hip04_eth.c
+++ linux/drivers/net/ethernet/hisilicon/hip04_eth.c
@@ -413,6 +413,15 @@ out:
 	return count;
 }
 
+static void hip04_start_tx_timer(struct hip04_priv *priv)
+{
+	ktime_t t;
+
+	/* allow timer to fire after half the time at the earliest */
+	t = ktime_set(0, priv->tx_coalesce_usecs * NSEC_PER_USEC / 2);
+	hrtimer_start(&priv->tx_coalesce_timer, t, HRTIMER_MODE_REL);
+}
+
 static int hip04_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 {
 	struct hip04_priv *priv = netdev_priv(ndev);
@@ -466,8 +475,7 @@ static int hip04_mac_start_xmit(struct s
 		}
 	} else if (!hrtimer_is_queued(&priv->tx_coalesce_timer)) {
 		/* cleanup not pending yet, start a new timer */
-		hrtimer_start_expires(&priv->tx_coalesce_timer,
-				      HRTIMER_MODE_REL);
+		hip04_start_tx_timer(priv);
 	}
 
 	return NETDEV_TX_OK;
@@ -549,7 +557,7 @@ done:
 	/* clean up tx descriptors and start a new timer if necessary */
 	tx_remaining = hip04_tx_reclaim(ndev, false);
 	if (rx < budget && tx_remaining)
-		hrtimer_start_expires(&priv->tx_coalesce_timer, HRTIMER_MODE_REL);
+		hip04_start_tx_timer(priv);
 
 	return rx;
 }
@@ -809,7 +817,6 @@ static int hip04_mac_probe(struct platfo
 	struct hip04_priv *priv;
 	struct resource *res;
 	unsigned int irq;
-	ktime_t txtime;
 	int ret;
 
 	ndev = alloc_etherdev(sizeof(struct hip04_priv));
@@ -846,9 +853,6 @@ static int hip04_mac_probe(struct platfo
 	 */
 	priv->tx_coalesce_frames = TX_DESC_NUM * 3 / 4;
 	priv->tx_coalesce_usecs = 200;
-	/* allow timer to fire after half the time at the earliest */
-	txtime = ktime_set(0, priv->tx_coalesce_usecs * NSEC_PER_USEC / 2);
-	hrtimer_set_expires_range(&priv->tx_coalesce_timer, txtime, txtime);
 	priv->tx_coalesce_timer.function = tx_done;
 
 	priv->map = syscon_node_to_regmap(arg.np);

Re: [patch 4/5] net: hip04: Make tx coalesce timer actually work

[Arnd Bergmann]

On Monday 13 April 2015 21:02:23 Thomas Gleixner wrote:
> The code sets the expiry value of the timer to a relative value and
> starts it with hrtimer_start_expires. That's fine, but that only works
> once. The timer is started in relative mode, so the expiry value gets
> overwritten with the absolut expiry time (now + expiry).
> 
> So once the timer expired, a new call to hrtimer_start_expires results
> in an immidiately expired timer, because the expiry value is
> already in the past.
> 
> Use the proper mechanisms to (re)start the timer in the intended way.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: dingtianhong <dingtianhong@huawei.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
> Cc: Dan Carpenter <dan.carpenter@oracle.com>
> Cc: netdev@vger.kernel.org

Thanks a lot for the fix. The mistake was clearly mine, as I had sent
a patch to introduce the tx coalesce timer without access to hardware
or a way to test that what I did was correct.

There are other known problems in the version of the driver that got
merged, and I believe that someone is now looking at them.

What I think we really want here is a way for user space to configure
both the minimum and maximum coalesce timer separately rather than
assuming half the time is what we want.

	Arnd

> @@ -413,6 +413,15 @@ out:
>  	return count;
>  }
>  
> +static void hip04_start_tx_timer(struct hip04_priv *priv)
> +{
> +	ktime_t t;
> +
> +	/* allow timer to fire after half the time at the earliest */
> +	t = ktime_set(0, priv->tx_coalesce_usecs * NSEC_PER_USEC / 2);
> +	hrtimer_start(&priv->tx_coalesce_timer, t, HRTIMER_MODE_REL);
> +}

Question: this looks to me like it sets both the minimum and maximum
time to priv->tx_coalesce_usecs/2, when the intention was to set
the minimum to priv->tx_coalesce_usecs/2 and the maximum to
priv->tx_coalesce_usecs. Am I missing something subtle here, or did
you just misread my original intention from the botched code?

	Arnd

Re: [patch 4/5] net: hip04: Make tx coalesce timer actually work

[Thomas Gleixner]

On Mon, 13 Apr 2015, Arnd Bergmann wrote:

> On Monday 13 April 2015 21:02:23 Thomas Gleixner wrote:
> > The code sets the expiry value of the timer to a relative value and
> > starts it with hrtimer_start_expires. That's fine, but that only works
> > once. The timer is started in relative mode, so the expiry value gets
> > overwritten with the absolut expiry time (now + expiry).
> > 
> > So once the timer expired, a new call to hrtimer_start_expires results
> > in an immidiately expired timer, because the expiry value is
> > already in the past.
> > 
> > Use the proper mechanisms to (re)start the timer in the intended way.
> > 
> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: dingtianhong <dingtianhong@huawei.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
> > Cc: Dan Carpenter <dan.carpenter@oracle.com>
> > Cc: netdev@vger.kernel.org
> 
> Thanks a lot for the fix. The mistake was clearly mine, as I had sent
> a patch to introduce the tx coalesce timer without access to hardware
> or a way to test that what I did was correct.
> 
> There are other known problems in the version of the driver that got
> merged, and I believe that someone is now looking at them.
> 
> What I think we really want here is a way for user space to configure
> both the minimum and maximum coalesce timer separately rather than
> assuming half the time is what we want.
> 
> 	Arnd
> 
> > @@ -413,6 +413,15 @@ out:
> >  	return count;
> >  }
> >  
> > +static void hip04_start_tx_timer(struct hip04_priv *priv)
> > +{
> > +	ktime_t t;
> > +
> > +	/* allow timer to fire after half the time at the earliest */
> > +	t = ktime_set(0, priv->tx_coalesce_usecs * NSEC_PER_USEC / 2);
> > +	hrtimer_start(&priv->tx_coalesce_timer, t, HRTIMER_MODE_REL);
> > +}
> 
> Question: this looks to me like it sets both the minimum and maximum
> time to priv->tx_coalesce_usecs/2, when the intention was to set
> the minimum to priv->tx_coalesce_usecs/2 and the maximum to
> priv->tx_coalesce_usecs. Am I missing something subtle here, or did
> you just misread my original intention from the botched code?

Yes, I missed that. Simple fix for this is:

  unsigned long t_ns = priv->tx_coalesce_usecs * NSEC_PER_USEC / 2;
  
  hrtimer_start_range_ns(&priv->tx_coalesce_timer, ns_to_ktime(t_ns),
			 t_ns, HRTIMER_MODE_REL);

Thanks,

	tglx

Re: [patch 4/5] net: hip04: Make tx coalesce timer actually work

[Arnd Bergmann]

On Monday 13 April 2015 23:42:03 Thomas Gleixner wrote:
> > 
> > Question: this looks to me like it sets both the minimum and maximum
> > time to priv->tx_coalesce_usecs/2, when the intention was to set
> > the minimum to priv->tx_coalesce_usecs/2 and the maximum to
> > priv->tx_coalesce_usecs. Am I missing something subtle here, or did
> > you just misread my original intention from the botched code?
> 
> Yes, I missed that. Simple fix for this is:
> 
>   unsigned long t_ns = priv->tx_coalesce_usecs * NSEC_PER_USEC / 2;
>   
>   hrtimer_start_range_ns(&priv->tx_coalesce_timer, ns_to_ktime(t_ns),
>                          t_ns, HRTIMER_MODE_REL);

Ah, good. I have to admit that I'd probably make the same mistake
again if I was to do this for another driver and you hadn't sent
the fix. The hrtimer_set_expires_range() function just looked like
it had been designed for the use case I was interested in ;-).

Any idea how to prevent the next person from making the same mistake?

	Arnd

Re: [patch 4/5] net: hip04: Make tx coalesce timer actually work

[Thomas Gleixner]

On Tue, 14 Apr 2015, Arnd Bergmann wrote:
> On Monday 13 April 2015 23:42:03 Thomas Gleixner wrote:
> > > 
> > > Question: this looks to me like it sets both the minimum and maximum
> > > time to priv->tx_coalesce_usecs/2, when the intention was to set
> > > the minimum to priv->tx_coalesce_usecs/2 and the maximum to
> > > priv->tx_coalesce_usecs. Am I missing something subtle here, or did
> > > you just misread my original intention from the botched code?
> > 
> > Yes, I missed that. Simple fix for this is:
> > 
> >   unsigned long t_ns = priv->tx_coalesce_usecs * NSEC_PER_USEC / 2;
> >   
> >   hrtimer_start_range_ns(&priv->tx_coalesce_timer, ns_to_ktime(t_ns),
> >                          t_ns, HRTIMER_MODE_REL);
> 
> Ah, good. I have to admit that I'd probably make the same mistake
> again if I was to do this for another driver and you hadn't sent
> the fix. The hrtimer_set_expires_range() function just looked like
> it had been designed for the use case I was interested in ;-).
> 
> Any idea how to prevent the next person from making the same mistake?

Yes. Documentation :)

Thanks,

	tglx

Re: [patch 4/5] net: hip04: Make tx coalesce timer actually work

[Ding Tianhong]

On 2015/4/14 6:08, Thomas Gleixner wrote:
> On Tue, 14 Apr 2015, Arnd Bergmann wrote:
>> On Monday 13 April 2015 23:42:03 Thomas Gleixner wrote:
>>>>
>>>> Question: this looks to me like it sets both the minimum and maximum
>>>> time to priv->tx_coalesce_usecs/2, when the intention was to set
>>>> the minimum to priv->tx_coalesce_usecs/2 and the maximum to
>>>> priv->tx_coalesce_usecs. Am I missing something subtle here, or did
>>>> you just misread my original intention from the botched code?
>>>
>>> Yes, I missed that. Simple fix for this is:
>>>
>>>   unsigned long t_ns = priv->tx_coalesce_usecs * NSEC_PER_USEC / 2;
>>>   
>>>   hrtimer_start_range_ns(&priv->tx_coalesce_timer, ns_to_ktime(t_ns),
>>>                          t_ns, HRTIMER_MODE_REL);
>>
>> Ah, good. I have to admit that I'd probably make the same mistake
>> again if I was to do this for another driver and you hadn't sent
>> the fix. The hrtimer_set_expires_range() function just looked like
>> it had been designed for the use case I was interested in ;-).
>>
>> Any idea how to prevent the next person from making the same mistake?
> 
> Yes. Documentation :)
> 
Looks good to me, thanks everyone.

Ding

> Thanks,
> 
> 	tglx
> 
> .
> 

Re: [patch 4/5] net: hip04: Make tx coalesce timer actually work

[David Miller]

From: Thomas Gleixner <tglx@linutronix.de>
Date: Tue, 14 Apr 2015 00:08:23 +0200 (CEST)

> On Tue, 14 Apr 2015, Arnd Bergmann wrote:
>> On Monday 13 April 2015 23:42:03 Thomas Gleixner wrote:
>> > > 
>> > > Question: this looks to me like it sets both the minimum and maximum
>> > > time to priv->tx_coalesce_usecs/2, when the intention was to set
>> > > the minimum to priv->tx_coalesce_usecs/2 and the maximum to
>> > > priv->tx_coalesce_usecs. Am I missing something subtle here, or did
>> > > you just misread my original intention from the botched code?
>> > 
>> > Yes, I missed that. Simple fix for this is:
>> > 
>> >   unsigned long t_ns = priv->tx_coalesce_usecs * NSEC_PER_USEC / 2;
>> >   
>> >   hrtimer_start_range_ns(&priv->tx_coalesce_timer, ns_to_ktime(t_ns),
>> >                          t_ns, HRTIMER_MODE_REL);
>> 
>> Ah, good. I have to admit that I'd probably make the same mistake
>> again if I was to do this for another driver and you hadn't sent
>> the fix. The hrtimer_set_expires_range() function just looked like
>> it had been designed for the use case I was interested in ;-).
>> 
>> Any idea how to prevent the next person from making the same mistake?
> 
> Yes. Documentation :)

Can I get a respin of this patch with the above?

[patch v2] net: hip04: Make tx coalesce timer actually work

[Thomas Gleixner]

The code sets the expiry value of the timer to a relative value and
starts it with hrtimer_start_expires. That's fine, but that only works
once. The timer is started in relative mode, so the expiry value gets
overwritten with the absolut expiry time (now + expiry).

So once the timer expired, a new call to hrtimer_start_expires results
in an immidiately expired timer, because the expiry value is
already in the past.

Use the proper mechanisms to (re)start the timer in the intended way.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: dingtianhong <dingtianhong@huawei.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Cc: netdev@vger.kernel.org
---
 drivers/net/ethernet/hisilicon/hip04_eth.c |   18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

Index: linux/drivers/net/ethernet/hisilicon/hip04_eth.c
===================================================================
--- linux.orig/drivers/net/ethernet/hisilicon/hip04_eth.c
+++ linux/drivers/net/ethernet/hisilicon/hip04_eth.c
@@ -413,6 +413,15 @@ out:
 	return count;
 }
 
+static void hip04_start_tx_timer(struct hip04_priv *priv)
+{
+	unsigned long ns = priv->tx_coalesce_usecs * NSEC_PER_USEC / 2;
+
+	/* allow timer to fire after half the time at the earliest */
+	hrtimer_start_range_ns(&priv->tx_coalesce_timer, ns_to_ktime(ns),
+			       ns, HRTIMER_MODE_REL);
+}
+
 static int hip04_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
 {
 	struct hip04_priv *priv = netdev_priv(ndev);
@@ -466,8 +475,7 @@ static int hip04_mac_start_xmit(struct s
 		}
 	} else if (!hrtimer_is_queued(&priv->tx_coalesce_timer)) {
 		/* cleanup not pending yet, start a new timer */
-		hrtimer_start_expires(&priv->tx_coalesce_timer,
-				      HRTIMER_MODE_REL);
+		hip04_start_tx_timer(priv);
 	}
 
 	return NETDEV_TX_OK;
@@ -549,7 +557,7 @@ done:
 	/* clean up tx descriptors and start a new timer if necessary */
 	tx_remaining = hip04_tx_reclaim(ndev, false);
 	if (rx < budget && tx_remaining)
-		hrtimer_start_expires(&priv->tx_coalesce_timer, HRTIMER_MODE_REL);
+		hip04_start_tx_timer(priv);
 
 	return rx;
 }
@@ -809,7 +817,6 @@ static int hip04_mac_probe(struct platfo
 	struct hip04_priv *priv;
 	struct resource *res;
 	unsigned int irq;
-	ktime_t txtime;
 	int ret;
 
 	ndev = alloc_etherdev(sizeof(struct hip04_priv));
@@ -846,9 +853,6 @@ static int hip04_mac_probe(struct platfo
 	 */
 	priv->tx_coalesce_frames = TX_DESC_NUM * 3 / 4;
 	priv->tx_coalesce_usecs = 200;
-	/* allow timer to fire after half the time at the earliest */
-	txtime = ktime_set(0, priv->tx_coalesce_usecs * NSEC_PER_USEC / 2);
-	hrtimer_set_expires_range(&priv->tx_coalesce_timer, txtime, txtime);
 	priv->tx_coalesce_timer.function = tx_done;
 
 	priv->map = syscon_node_to_regmap(arg.np);

Re: [patch v2] net: hip04: Make tx coalesce timer actually work

[Ding Tianhong]

On 2015/4/15 3:42, Thomas Gleixner wrote:
> The code sets the expiry value of the timer to a relative value and
> starts it with hrtimer_start_expires. That's fine, but that only works
> once. The timer is started in relative mode, so the expiry value gets
> overwritten with the absolut expiry time (now + expiry).
> 
> So once the timer expired, a new call to hrtimer_start_expires results
> in an immidiately expired timer, because the expiry value is
> already in the past.
> 
> Use the proper mechanisms to (re)start the timer in the intended way.
> 

Acked-by: Ding Tianhong <dingtianhong@huawei.com>

> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: dingtianhong <dingtianhong@huawei.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
> Cc: Dan Carpenter <dan.carpenter@oracle.com>
> Cc: netdev@vger.kernel.org
> ---
>  drivers/net/ethernet/hisilicon/hip04_eth.c |   18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> Index: linux/drivers/net/ethernet/hisilicon/hip04_eth.c
> ===================================================================
> --- linux.orig/drivers/net/ethernet/hisilicon/hip04_eth.c
> +++ linux/drivers/net/ethernet/hisilicon/hip04_eth.c
> @@ -413,6 +413,15 @@ out:
>  	return count;
>  }
>  
> +static void hip04_start_tx_timer(struct hip04_priv *priv)
> +{
> +	unsigned long ns = priv->tx_coalesce_usecs * NSEC_PER_USEC / 2;
> +
> +	/* allow timer to fire after half the time at the earliest */
> +	hrtimer_start_range_ns(&priv->tx_coalesce_timer, ns_to_ktime(ns),
> +			       ns, HRTIMER_MODE_REL);
> +}
> +
>  static int hip04_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>  {
>  	struct hip04_priv *priv = netdev_priv(ndev);
> @@ -466,8 +475,7 @@ static int hip04_mac_start_xmit(struct s
>  		}
>  	} else if (!hrtimer_is_queued(&priv->tx_coalesce_timer)) {
>  		/* cleanup not pending yet, start a new timer */
> -		hrtimer_start_expires(&priv->tx_coalesce_timer,
> -				      HRTIMER_MODE_REL);
> +		hip04_start_tx_timer(priv);
>  	}
>  
>  	return NETDEV_TX_OK;
> @@ -549,7 +557,7 @@ done:
>  	/* clean up tx descriptors and start a new timer if necessary */
>  	tx_remaining = hip04_tx_reclaim(ndev, false);
>  	if (rx < budget && tx_remaining)
> -		hrtimer_start_expires(&priv->tx_coalesce_timer, HRTIMER_MODE_REL);
> +		hip04_start_tx_timer(priv);
>  
>  	return rx;
>  }
> @@ -809,7 +817,6 @@ static int hip04_mac_probe(struct platfo
>  	struct hip04_priv *priv;
>  	struct resource *res;
>  	unsigned int irq;
> -	ktime_t txtime;
>  	int ret;
>  
>  	ndev = alloc_etherdev(sizeof(struct hip04_priv));
> @@ -846,9 +853,6 @@ static int hip04_mac_probe(struct platfo
>  	 */
>  	priv->tx_coalesce_frames = TX_DESC_NUM * 3 / 4;
>  	priv->tx_coalesce_usecs = 200;
> -	/* allow timer to fire after half the time at the earliest */
> -	txtime = ktime_set(0, priv->tx_coalesce_usecs * NSEC_PER_USEC / 2);
> -	hrtimer_set_expires_range(&priv->tx_coalesce_timer, txtime, txtime);
>  	priv->tx_coalesce_timer.function = tx_done;
>  
>  	priv->map = syscon_node_to_regmap(arg.np);
> 
> .
> 

Re: [patch v2] net: hip04: Make tx coalesce timer actually work

[Arnd Bergmann]

On Tuesday 14 April 2015 21:42:42 Thomas Gleixner wrote:
> The code sets the expiry value of the timer to a relative value and
> starts it with hrtimer_start_expires. That's fine, but that only works
> once. The timer is started in relative mode, so the expiry value gets
> overwritten with the absolut expiry time (now + expiry).
> 
> So once the timer expired, a new call to hrtimer_start_expires results
> in an immidiately expired timer, because the expiry value is
> already in the past.
> 
> Use the proper mechanisms to (re)start the timer in the intended way.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: dingtianhong <dingtianhong@huawei.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Zhangfei Gao <zhangfei.gao@linaro.org>
> Cc: Dan Carpenter <dan.carpenter@oracle.com>
> Cc: netdev@vger.kernel.org
> 

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

Re: [patch v2] net: hip04: Make tx coalesce timer actually work

[David Miller]

From: Thomas Gleixner <tglx@linutronix.de>
Date: Tue, 14 Apr 2015 21:42:42 +0200 (CEST)

> The code sets the expiry value of the timer to a relative value and
> starts it with hrtimer_start_expires. That's fine, but that only works
> once. The timer is started in relative mode, so the expiry value gets
> overwritten with the absolut expiry time (now + expiry).
> 
> So once the timer expired, a new call to hrtimer_start_expires results
> in an immidiately expired timer, because the expiry value is
> already in the past.
> 
> Use the proper mechanisms to (re)start the timer in the intended way.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Applied, thanks Thomas.

[patch 5/5] staging: ozwpan: Fix hrtimer wreckage

[Thomas Gleixner]

[-- Attachment #1: stagin-ozwpan-fix-hrtimer-wreckage.patch --]
[-- Type: text/plain, Size: 1712 bytes --]

oz_timer_add() modifies the expiry value of an active timer, which
results in data corruption.

Use hrtimer_start() and remove the silly conditional.

While at it use the proper helper function to convert milliseconds to
ktime.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Shigekatsu Tateno <shigekatsu.tateno@atmel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: devel@driverdev.osuosl.org
---
 drivers/staging/ozwpan/ozproto.c |   17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

Index: linux/drivers/staging/ozwpan/ozproto.c
===================================================================
--- linux.orig/drivers/staging/ozwpan/ozproto.c
+++ linux/drivers/staging/ozwpan/ozproto.c
@@ -566,23 +566,14 @@ void oz_timer_add(struct oz_pd *pd, int
 	switch (type) {
 	case OZ_TIMER_TOUT:
 	case OZ_TIMER_STOP:
-		if (hrtimer_active(&pd->timeout)) {
-			hrtimer_set_expires(&pd->timeout, ktime_set(due_time /
-			MSEC_PER_SEC, (due_time % MSEC_PER_SEC) *
-							NSEC_PER_MSEC));
-			hrtimer_start_expires(&pd->timeout, HRTIMER_MODE_REL);
-		} else {
-			hrtimer_start(&pd->timeout, ktime_set(due_time /
-			MSEC_PER_SEC, (due_time % MSEC_PER_SEC) *
-					NSEC_PER_MSEC), HRTIMER_MODE_REL);
-		}
+		hrtimer_start(&pd->timeout, ms_to_ktime(due_time),
+			      HRTIMER_MODE_REL);
 		pd->timeout_type = type;
 		break;
 	case OZ_TIMER_HEARTBEAT:
 		if (!hrtimer_active(&pd->heartbeat))
-			hrtimer_start(&pd->heartbeat, ktime_set(due_time /
-			MSEC_PER_SEC, (due_time % MSEC_PER_SEC) *
-					NSEC_PER_MSEC), HRTIMER_MODE_REL);
+			hrtimer_start(&pd->heartbeat, ms_to_ktime(due_time),
+				      HRTIMER_MODE_REL);
 		break;
 	}
 	spin_unlock_bh(&g_polling_lock);

Re: [patch 5/5] staging: ozwpan: Fix hrtimer wreckage

[Greg Kroah-Hartman]

On Mon, Apr 13, 2015 at 09:02:25PM -0000, Thomas Gleixner wrote:
> oz_timer_add() modifies the expiry value of an active timer, which
> results in data corruption.
> 
> Use hrtimer_start() and remove the silly conditional.
> 
> While at it use the proper helper function to convert milliseconds to
> ktime.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: Shigekatsu Tateno <shigekatsu.tateno@atmel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: devel@driverdev.osuosl.org
> ---
>  drivers/staging/ozwpan/ozproto.c |   17 ++++-------------
>  1 file changed, 4 insertions(+), 13 deletions(-)

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[tip:timers/core] staging: ozwpan: Fix hrtimer wreckage

[tip-bot for Thomas Gleixner]

Commit-ID:  5a447f09a8dffc0dd7569aec614ad3613a050098
Gitweb:     http://git.kernel.org/tip/5a447f09a8dffc0dd7569aec614ad3613a050098
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Mon, 13 Apr 2015 21:02:25 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Mon, 20 Jul 2015 11:37:46 +0200

staging: ozwpan: Fix hrtimer wreckage

oz_timer_add() modifies the expiry value of an active timer, which
results in data corruption.

Use hrtimer_start() and remove the silly conditional.

While at it use the proper helper function to convert milliseconds to
ktime.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Shigekatsu Tateno <shigekatsu.tateno@atmel.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: devel@driverdev.osuosl.org
Link: http://lkml.kernel.org/r/20150413210035.357448657@linutronix.de
---
 drivers/staging/ozwpan/ozproto.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/ozwpan/ozproto.c b/drivers/staging/ozwpan/ozproto.c
index 1ba24a2..23aa817 100644
--- a/drivers/staging/ozwpan/ozproto.c
+++ b/drivers/staging/ozwpan/ozproto.c
@@ -566,23 +566,14 @@ void oz_timer_add(struct oz_pd *pd, int type, unsigned long due_time)
 	switch (type) {
 	case OZ_TIMER_TOUT:
 	case OZ_TIMER_STOP:
-		if (hrtimer_active(&pd->timeout)) {
-			hrtimer_set_expires(&pd->timeout, ktime_set(due_time /
-			MSEC_PER_SEC, (due_time % MSEC_PER_SEC) *
-							NSEC_PER_MSEC));
-			hrtimer_start_expires(&pd->timeout, HRTIMER_MODE_REL);
-		} else {
-			hrtimer_start(&pd->timeout, ktime_set(due_time /
-			MSEC_PER_SEC, (due_time % MSEC_PER_SEC) *
-					NSEC_PER_MSEC), HRTIMER_MODE_REL);
-		}
+		hrtimer_start(&pd->timeout, ms_to_ktime(due_time),
+			      HRTIMER_MODE_REL);
 		pd->timeout_type = type;
 		break;
 	case OZ_TIMER_HEARTBEAT:
 		if (!hrtimer_active(&pd->heartbeat))
-			hrtimer_start(&pd->heartbeat, ktime_set(due_time /
-			MSEC_PER_SEC, (due_time % MSEC_PER_SEC) *
-					NSEC_PER_MSEC), HRTIMER_MODE_REL);
+			hrtimer_start(&pd->heartbeat, ms_to_ktime(due_time),
+				      HRTIMER_MODE_REL);
 		break;
 	}
 	spin_unlock_bh(&g_polling_lock);