LTTng "TIF_KERNEL_TRACE"

LTTng "TIF_KERNEL_TRACE"

[Mathieu Desnoyers]

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.

Mathieu

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: LTTng "TIF_KERNEL_TRACE"

[Ingo Molnar]

* 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.

	Ingo

Re: LTTng "TIF_KERNEL_TRACE"

[Mathieu Desnoyers]

* 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.
> 
> 	Ingo

lttng-kernel-trace-thread-flag-api.patch adds the enable/disable API,
with refcount, in kernel/sched.c just because I did not know where to
put it. Should we move this to its own file in kernel/trace/???.c ?

One option is kernel/trace/trace.c, which is compiled as soon as
CONFIG_TRACING is enabled.

I added the prototypes to linux/sched.h, they should probably be moved
to... linux/trace.h ?

Mathieu

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: LTTng "TIF_KERNEL_TRACE"

[Mathieu Desnoyers]

* 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

> 	Ingo

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: LTTng "TIF_KERNEL_TRACE"

[Frederic Weisbecker]

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.

Frederic.

Re: LTTng "TIF_KERNEL_TRACE"

[Frédéric Weisbecker]

2009/4/28 Frederic Weisbecker <fweisbec@gmail.com>:
> 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.


I mean, it's usually a proof of an unsafe state. Not here because we
actually never hold
it from irq context.

Re: LTTng "TIF_KERNEL_TRACE"

[Mathieu Desnoyers]

* Frederic Weisbecker (fweisbec@gmail.com) wrote:
> 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.

No, read-write lock is a "special case" where it does not deadlock if
you have an interrupt handler taking the read lock over another read
lock. It's just the write lock that _must absolutely_ disable
interrupts.

However, the "latency race" scenario I explained above applies here,
because the write lock disables interrupts and the read locks doesn't.

Mathieu

> 
> Frederic.
> 
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: LTTng "TIF_KERNEL_TRACE"

[Steven Rostedt]

On Tue, 28 Apr 2009, Frederic Weisbecker wrote:
> 
> I don't know why I used irqsave here, I guess I was tired.

It was the "when in doubt use irqsave", or was it the "I don't want to 
think about it, use irqsave" ;-)

-- Steve

Re: LTTng "TIF_KERNEL_TRACE"

[Frederic Weisbecker]

On Tue, Apr 28, 2009 at 12:38:25PM -0400, Mathieu Desnoyers wrote:
> No, read-write lock is a "special case" where it does not deadlock if
> you have an interrupt handler taking the read lock over another read
> lock. It's just the write lock that _must absolutely_ disable
> interrupts.


Ah, you're right, I was thinking with spinlock rules in mind :)


> However, the "latency race" scenario I explained above applies here,
> because the write lock disables interrupts and the read locks doesn't.
> 
> Mathieu
> 

Re: LTTng "TIF_KERNEL_TRACE"

[Frederic Weisbecker]

On Tue, Apr 28, 2009 at 12:43:25PM -0400, Steven Rostedt wrote:
> 
> On Tue, 28 Apr 2009, Frederic Weisbecker wrote:
> > 
> > I don't know why I used irqsave here, I guess I was tired.
> 
> It was the "when in doubt use irqsave", or was it the "I don't want to 
> think about it, use irqsave" ;-)


A bit of the former and the latter...
It was "Argh, it's too late, I should have gone to my bed several hours ago,
just use irqsave..." :-/

 
> -- Steve
> 

Re: LTTng "TIF_KERNEL_TRACE"

[Steven Rostedt]

On Tue, 28 Apr 2009, Frederic Weisbecker wrote:

> On Tue, Apr 28, 2009 at 12:38:25PM -0400, Mathieu Desnoyers wrote:
> > No, read-write lock is a "special case" where it does not deadlock if
> > you have an interrupt handler taking the read lock over another read
> > lock. It's just the write lock that _must absolutely_ disable
> > interrupts.
> 
> 
> Ah, you're right, I was thinking with spinlock rules in mind :)
> 

The only time you would want to do a read_lock_irqsave is if the 
write_lock is taken in irq context. But I do not know of any lock where 
that is the case.

-- Steve