lkml.org 
[lkml]   [2013]   [Sep]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH lttng-modules] Fix: use timekeeping_is_busy() to fix ktime_get() hard lockup
On Wed, 11 Sep 2013, Mathieu Desnoyers wrote:
> * John Stultz (john.stultz@linaro.org) wrote:
> > On 09/11/2013 08:12 AM, Mathieu Desnoyers wrote:
> > > LTTng uses ktime to have the same time-base across kernel and
> > > user-space, so traces gathered from LTTng-modules and LTTng-UST can be
> > > correlated. We plan on using ktime until a fast, scalable, and
> > > fine-grained time-source for tracing that can be used across kernel and
> > > user-space, and which does not rely on read seqlock for kernel-level
> > > synchronization, makes its way into the kernel.
> > >
> > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > Cc: Richard Cochran <richardcochran@gmail.com>
> > > Cc: Prarit Bhargava <prarit@redhat.com>
> > > Cc: John Stultz <john.stultz@linaro.org>
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > Cc: Steven Rostedt <rostedt@goodmis.org>
> > > Cc: Ingo Molnar <mingo@elte.hu>
> > > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > > ---
> > > diff --git a/wrapper/trace-clock.h b/wrapper/trace-clock.h
> > > index bced61c..2f9df7a 100644
> > > --- a/wrapper/trace-clock.h
> > > +++ b/wrapper/trace-clock.h
> > > @@ -32,6 +32,7 @@
> > > #include <linux/ktime.h>
> > > #include <linux/time.h>
> > > #include <linux/hrtimer.h>
> > > +#include <linux/version.h>
> > > #include "random.h"
> > >
> > > static inline u64 trace_clock_monotonic_wrapper(void)
> > > @@ -45,6 +46,10 @@ static inline u64 trace_clock_monotonic_wrapper(void)
> > > if (in_nmi())
> > > return (u64) -EIO;
> > >
> > > +#if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,10,0))
> > > + if (timekeeping_is_busy())
> > > + return (u64) -EIO;
> > > +#endif
> > > ktime = ktime_get();
> > > return ktime_to_ns(ktime);
> > > }
> >
> >
> > I guess the other question here is should this functionality be pushed
> > down into the timekeeping accessors themselves?
> >
> > I know any extra checks would probably be considered overhead in some
> > uses, but if we do the check only when we hit contention then it might
> > not be so bad.
>
> I thought about the exact same thing, but wanted to keep my initial
> kernel patch minimal, so I chose not to touch the fast paths initially.
>
> Indeed, if we only do this check after the seqretry has failed, we
> should be able to add this check without touching the fast-path.
>
> It might be cleaner to make ktime_get() return an error rather than
> cause a hard lockup in those cases. Especially if it can be done without
> performance regression.

Nope. ktime_get() is not going to fail ever. We want to deadlock when
its called from inside xtime_lock held code. Simply because it's wrong
to do so.

If there are special use cases, i.e. tracing, which need this kind of
check, then we rather add a new interface, e.g. ktime_get_tracetime(),
than adding a tasteless bogosity like timekeeping_busy().

Thanks,

tglx






\
 
 \ /
  Last update: 2013-09-12 22:21    [W:0.042 / U:0.712 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site