| Message ID | bc8cabd00810c2f91cb45a64514f5c4ca12d584c.1404888801.git.viresh.kumar@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Wed, Jul 09, 2014 at 12:25:33PM +0530, Viresh Kumar wrote: > hrtimer_start*() family never fails to enqueue a hrtimer to a clock-base. The > only special case is when the hrtimer was in past. If it is getting enqueued to > local CPUs's clock-base, we raise a softirq and exit, else we handle that on > next interrupt on remote CPU. > > At several places in kernel we check if hrtimer is enqueued properly with > hrtimer_active(). This isn't required and can be dropped. > > Before fixing that, lets make sure hrtimer is always enqueued properly by adding > > WARN_ON_ONCE(!hrtimer_active(timer)); > > towards the end of __hrtimer_start_range_ns(). > > Suggested-by: Frederic Weisbecker <fweisbec@gmail.com> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > kernel/hrtimer.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c > index 3ab2899..cf40209 100644 > --- a/kernel/hrtimer.c > +++ b/kernel/hrtimer.c > @@ -1037,6 +1037,8 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim, > > unlock_hrtimer_base(timer, &flags); > > + /* Make sure timer is enqueued */ > + WARN_ON_ONCE(!hrtimer_active(timer)); Hmm, after reading Thomas reply, I think it's possible that the hrtimer expires right after we unlock it and, if we are unlucky enough, before the hrtimer_active() check. In this case we might hit a false positive. > return ret; > } > EXPORT_SYMBOL_GPL(__hrtimer_start_range_ns); > -- > 2.0.0.rc2 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Thu, 10 Jul 2014, Frederic Weisbecker wrote: > On Wed, Jul 09, 2014 at 12:25:33PM +0530, Viresh Kumar wrote: > > hrtimer_start*() family never fails to enqueue a hrtimer to a clock-base. The > > only special case is when the hrtimer was in past. If it is getting enqueued to > > local CPUs's clock-base, we raise a softirq and exit, else we handle that on > > next interrupt on remote CPU. > > > > At several places in kernel we check if hrtimer is enqueued properly with > > hrtimer_active(). This isn't required and can be dropped. > > > > Before fixing that, lets make sure hrtimer is always enqueued properly by adding > > > > WARN_ON_ONCE(!hrtimer_active(timer)); > > > > towards the end of __hrtimer_start_range_ns(). > > > > Suggested-by: Frederic Weisbecker <fweisbec@gmail.com> > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > > --- > > kernel/hrtimer.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c > > index 3ab2899..cf40209 100644 > > --- a/kernel/hrtimer.c > > +++ b/kernel/hrtimer.c > > @@ -1037,6 +1037,8 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim, > > > > unlock_hrtimer_base(timer, &flags); > > > > + /* Make sure timer is enqueued */ > > + WARN_ON_ONCE(!hrtimer_active(timer)); > > Hmm, after reading Thomas reply, I think it's possible that the hrtimer expires > right after we unlock it and, if we are unlucky enough, before the hrtimer_active() > check. > > In this case we might hit a false positive. Haha, I didn't even go down to this patch after reading 0/N. I knew right there that it's going to be pointless shite. But now that you point me to it, it's even worse. It's not only pointless shite it's actively wrong and outright stupid for someone who tries to "work" on this code for a couple of month now. Viresh, I'm really tired of this. Stop touching code you do not understand. I warned you more than once and now you really reached the level of complete incompetence. Welcome to my killfile. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 3ab2899..cf40209 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -1037,6 +1037,8 @@ int __hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim, unlock_hrtimer_base(timer, &flags); + /* Make sure timer is enqueued */ + WARN_ON_ONCE(!hrtimer_active(timer)); return ret; } EXPORT_SYMBOL_GPL(__hrtimer_start_range_ns);
hrtimer_start*() family never fails to enqueue a hrtimer to a clock-base. The only special case is when the hrtimer was in past. If it is getting enqueued to local CPUs's clock-base, we raise a softirq and exit, else we handle that on next interrupt on remote CPU. At several places in kernel we check if hrtimer is enqueued properly with hrtimer_active(). This isn't required and can be dropped. Before fixing that, lets make sure hrtimer is always enqueued properly by adding WARN_ON_ONCE(!hrtimer_active(timer)); towards the end of __hrtimer_start_range_ns(). Suggested-by: Frederic Weisbecker <fweisbec@gmail.com> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- kernel/hrtimer.c | 2 ++ 1 file changed, 2 insertions(+)