> On Tue, Apr 28, 2009 at 11:40:46AM -0400, Mathieu Desnoyers wrote:
> > * Ingo Molnar (
mingo@elte.hu) wrote:
> > >
> > > * Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> wrote:
> > >
> > > > Hi Ingo,
> > > >
> > > > Looking at the current -tip tree, I notice that the
> > > > TIF_SYSCALL_FTRACE flag is only implemented for x86.
> > > >
> > > > I have TIF_KERNEL_TRACE in my lttng tree which applies to all
> > > > architectures to do the exact same thing :
> > > >
> > > > lttng-kernel-trace-thread-flag-alpha.patch
> > > > lttng-kernel-trace-thread-flag-arm.patch
> > > > lttng-kernel-trace-thread-flag-avr32.patch
> > > > lttng-kernel-trace-thread-flag-blackfin.patch
> > > > lttng-kernel-trace-thread-flag-cris.patch
> > > > lttng-kernel-trace-thread-flag-frv.patch
> > > > lttng-kernel-trace-thread-flag-h8300.patch
> > > > lttng-kernel-trace-thread-flag-ia64.patch
> > > > lttng-kernel-trace-thread-flag-m32r.patch
> > > > lttng-kernel-trace-thread-flag-m68k.patch
> > > > lttng-kernel-trace-thread-flag-mips.patch
> > > > lttng-kernel-trace-thread-flag-parisc.patch
> > > > lttng-kernel-trace-thread-flag-powerpc.patch
> > > > lttng-kernel-trace-thread-flag-s390.patch
> > > > lttng-kernel-trace-thread-flag-sh.patch
> > > > lttng-kernel-trace-thread-flag-sparc.patch
> > > > lttng-kernel-trace-thread-flag-um.patch
> > > > lttng-kernel-trace-thread-flag-x86.patch
> > > > lttng-kernel-trace-thread-flag-xtensa.patch
> > > > lttng-kernel-trace-thread-flag-api.patch
> > > >
> > > > Is there any way we could get this merged ?
> > > >
> > > > One thing I like about the name TIF_KERNEL_TRACE compared to
> > > > TIF_SYSCALL_FTRACE is that it gives us a per-thread flag that
> > > > could eventually be used for more kernel tracing purposes than
> > > > just syscalls.
> > >
> > > Yeah - TIF_KERNEL_TRACE indeed sounds more descriptive and less
> > > restrictive. TIF_SYSCALL_FTRACE was a bit ad-hoc.
> > >
> >
> > Second question :
> >
> > LTTng :
> > read_lock(&tasklist_lock);
> > do_each_thread(p, t) {
> > set_tsk_thread_flag(t, TIF_KERNEL_TRACE);
> > } while_each_thread(p, t);
> > read_unlock(&tasklist_lock);
> >
> > Ftrace:
> > read_lock_irqsave(&tasklist_lock, flags);
> >
> > do_each_thread(g, t) {
> > clear_tsk_thread_flag(t, TIF_SYSCALL_FTRACE);
> > } while_each_thread(g, t);
> >
> > read_unlock_irqrestore(&tasklist_lock, flags);
> >
> > With or without irqsave ?
> >
> > Arguments against irqsave for this read lock :
> >
> > - it's not used consistently for this read lock all over the kernel.
> > Sometimes the read lock is taken without irqsave.
> > - it can be a long iteration, and therefore disables interrupts for a
> > long time.
> >
> > Arguments for irqsave for this read lock :
> >
> > - Taking any kind of spin/rwlock with inconsistent irq disabling leads
> > to races where interrupts can be disabled for an unbounded amount of
> > time if a spinlock with irqoff waits on a spinlock with irqs on. This
> > is a general problem with current kernel rwlock usage. See my
> > "priority sifting reader-writer lock" patchset for a fix to this
> > problem.
> >
> > Mathieu
> >
>
>
> I don't know why I used irqsave here, I guess I was tired.
>
> $ git-grep "read_lock_irqsave(&tasklist_lock)" | wc -l
> 0
> $ git-grep "write_lock_irqsave(&tasklist_lock)" | wc -l
> 0
>
> It is never used in an irq safe fashion, unless one of these sites
> has the irqs disabled.
> Lockdep should even have complained about this, when you hold
> a lock class in an irq safe fashion and thereafter you try to hold
> it in an irq unsafe fashion, then the state of the kernel becomes unsafe
> and lockdep is supposed to complain about that.
