| Message ID | 20090320150914.GA22769@oksana.dev.rtsoft.ru (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
* Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > commit 40ada30f9621fbd831ac2437b9a2a399aad34b00 ("tracing: clean > up menu"), despite the "clean up" in its purpose, introduced > behavioural change for Kconfig symbols: we no longer able to > select tracing support on PPC32 (because IRQFLAGS_SUPPORT isn't > yet implemented). Could you please solve this by implementing proper irqflag-tracing support? It's been available upstream for almost three years. It's needed for lockdep support as well, etc. Ingo
On Fri, Mar 20, 2009 at 08:04:28PM +0100, Ingo Molnar wrote: > > * Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > > > commit 40ada30f9621fbd831ac2437b9a2a399aad34b00 ("tracing: clean > > up menu"), despite the "clean up" in its purpose, introduced > > behavioural change for Kconfig symbols: we no longer able to > > select tracing support on PPC32 (because IRQFLAGS_SUPPORT isn't > > yet implemented). > > Could you please solve this by implementing proper irqflag-tracing > support? It's been available upstream for almost three years. It's > needed for lockdep support as well, etc. Breaking things via clean up patches is an interesting method of encouraging something to implement. ;-) Surely I'll look into implementing irqflags tracing, but considering that no one ever needed this for almost three years, and that we explicitly have the code to deal with tracing-w/o-irqflags, can we please restore the old behaviour? At least for 2.6.30, because I don't think I'll have enough time to implement/test/push irqflags tracing support in time for 2.6.30-rc1. Thanks, p.s. It would make more sense if 40ada30f's commit message would state that tracing w/o irqflags is now obsolete, or better, if it was discussed on some mailinglist, so that we'd have some time to adapt the architecture-specific code.
* Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > On Fri, Mar 20, 2009 at 08:04:28PM +0100, Ingo Molnar wrote: > > > > * Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > > > > > commit 40ada30f9621fbd831ac2437b9a2a399aad34b00 ("tracing: clean > > > up menu"), despite the "clean up" in its purpose, introduced > > > behavioural change for Kconfig symbols: we no longer able to > > > select tracing support on PPC32 (because IRQFLAGS_SUPPORT isn't > > > yet implemented). > > > > Could you please solve this by implementing proper > > irqflag-tracing support? It's been available upstream for almost > > three years. It's needed for lockdep support as well, etc. > > Breaking things via clean up patches is an interesting method of > encouraging something to implement. ;-) > > Surely I'll look into implementing irqflags tracing, but > considering that no one ever needed this for almost three years, > [...] Weird, there's no lockdep support? Ingo
On Fri, Mar 20, 2009 at 08:57:43PM +0100, Ingo Molnar wrote: > > * Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > > > On Fri, Mar 20, 2009 at 08:04:28PM +0100, Ingo Molnar wrote: > > > > > > * Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > > > > > > > commit 40ada30f9621fbd831ac2437b9a2a399aad34b00 ("tracing: clean > > > > up menu"), despite the "clean up" in its purpose, introduced > > > > behavioural change for Kconfig symbols: we no longer able to > > > > select tracing support on PPC32 (because IRQFLAGS_SUPPORT isn't > > > > yet implemented). > > > > > > Could you please solve this by implementing proper > > > irqflag-tracing support? It's been available upstream for almost > > > three years. It's needed for lockdep support as well, etc. > > > > Breaking things via clean up patches is an interesting method of > > encouraging something to implement. ;-) > > > > Surely I'll look into implementing irqflags tracing, but > > considering that no one ever needed this for almost three years, > > [...] > > Weird, there's no lockdep support? *ashamed*: apparently no such support currently exist for PPC32. ;-)
Ug, My Red Hat email was not being updated. I totally missed this thread. On Fri, 2009-03-20 at 20:57 +0100, Ingo Molnar wrote: > * Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > > > On Fri, Mar 20, 2009 at 08:04:28PM +0100, Ingo Molnar wrote: > > > > > > * Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > > > > > > > commit 40ada30f9621fbd831ac2437b9a2a399aad34b00 ("tracing: clean > > > > up menu"), despite the "clean up" in its purpose, introduced > > > > behavioural change for Kconfig symbols: we no longer able to > > > > select tracing support on PPC32 (because IRQFLAGS_SUPPORT isn't > > > > yet implemented). > > > > > > Could you please solve this by implementing proper > > > irqflag-tracing support? It's been available upstream for almost > > > three years. It's needed for lockdep support as well, etc. > > > > Breaking things via clean up patches is an interesting method of > > encouraging something to implement. ;-) > > > > Surely I'll look into implementing irqflags tracing, but > > considering that no one ever needed this for almost three years, > > [...] > > Weird, there's no lockdep support? I've discussed this with several people before. lockdep exists for PPC64, but apparently it does not work for PPC32. There's been ongoing work in this area, but unfortunately, nothing stable has come out of it. I believe Dale was the last one to be working on this. -- Steve
* Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > On Fri, Mar 20, 2009 at 08:57:43PM +0100, Ingo Molnar wrote: > > > > * Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > > > > > On Fri, Mar 20, 2009 at 08:04:28PM +0100, Ingo Molnar wrote: > > > > > > > > * Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > > > > > > > > > commit 40ada30f9621fbd831ac2437b9a2a399aad34b00 ("tracing: clean > > > > > up menu"), despite the "clean up" in its purpose, introduced > > > > > behavioural change for Kconfig symbols: we no longer able to > > > > > select tracing support on PPC32 (because IRQFLAGS_SUPPORT isn't > > > > > yet implemented). > > > > > > > > Could you please solve this by implementing proper > > > > irqflag-tracing support? It's been available upstream for almost > > > > three years. It's needed for lockdep support as well, etc. > > > > > > Breaking things via clean up patches is an interesting method of > > > encouraging something to implement. ;-) > > > > > > Surely I'll look into implementing irqflags tracing, but > > > considering that no one ever needed this for almost three years, > > > [...] > > > > Weird, there's no lockdep support? > > *ashamed*: apparently no such support currently exist for PPC32. ;-) Hm, do all the tracers even compile on ppc32 with your patch? We had periodic build failures on weird, unmaintained architectures that had no irqflags-tracing support and hence didnt know the raw_irqs_save/restore primitives ... I'm not trying to make things more difficult for you (and we can apply your patch if it builds fine and does not cause problems elsewhere), but there were some real downsides to not having proper irq APIs ... Ingo
On Sat, 21 Mar 2009, Ingo Molnar wrote: > > * Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > > > On Fri, Mar 20, 2009 at 08:57:43PM +0100, Ingo Molnar wrote: > > > > > > * Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > > > > > > > On Fri, Mar 20, 2009 at 08:04:28PM +0100, Ingo Molnar wrote: > > > > > > > > > > * Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > > > > > > > > > > > commit 40ada30f9621fbd831ac2437b9a2a399aad34b00 ("tracing: clean > > > > > > up menu"), despite the "clean up" in its purpose, introduced > > > > > > behavioural change for Kconfig symbols: we no longer able to > > > > > > select tracing support on PPC32 (because IRQFLAGS_SUPPORT isn't > > > > > > yet implemented). > > > > > > > > > > Could you please solve this by implementing proper > > > > > irqflag-tracing support? It's been available upstream for almost > > > > > three years. It's needed for lockdep support as well, etc. > > > > > > > > Breaking things via clean up patches is an interesting method of > > > > encouraging something to implement. ;-) > > > > > > > > Surely I'll look into implementing irqflags tracing, but > > > > considering that no one ever needed this for almost three years, > > > > [...] > > > > > > Weird, there's no lockdep support? > > > > *ashamed*: apparently no such support currently exist for PPC32. ;-) > > Hm, do all the tracers even compile on ppc32 with your patch? > > We had periodic build failures on weird, unmaintained architectures > that had no irqflags-tracing support and hence didnt know the > raw_irqs_save/restore primitives ... > > I'm not trying to make things more difficult for you (and we can > apply your patch if it builds fine and does not cause problems > elsewhere), but there were some real downsides to not having proper > irq APIs ... Note, the issue is not with the hooks into local_irq_save/restore, but with the entry.S code. That code is very sensitive where the irqs are enabled and disabled. -- Steve
* Steven Rostedt <rostedt@goodmis.org> wrote: > > On Sat, 21 Mar 2009, Ingo Molnar wrote: > > > > > * Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > > > > > On Fri, Mar 20, 2009 at 08:57:43PM +0100, Ingo Molnar wrote: > > > > > > > > * Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > > > > > > > > > On Fri, Mar 20, 2009 at 08:04:28PM +0100, Ingo Molnar wrote: > > > > > > > > > > > > * Anton Vorontsov <avorontsov@ru.mvista.com> wrote: > > > > > > > > > > > > > commit 40ada30f9621fbd831ac2437b9a2a399aad34b00 ("tracing: clean > > > > > > > up menu"), despite the "clean up" in its purpose, introduced > > > > > > > behavioural change for Kconfig symbols: we no longer able to > > > > > > > select tracing support on PPC32 (because IRQFLAGS_SUPPORT isn't > > > > > > > yet implemented). > > > > > > > > > > > > Could you please solve this by implementing proper > > > > > > irqflag-tracing support? It's been available upstream for almost > > > > > > three years. It's needed for lockdep support as well, etc. > > > > > > > > > > Breaking things via clean up patches is an interesting method of > > > > > encouraging something to implement. ;-) > > > > > > > > > > Surely I'll look into implementing irqflags tracing, but > > > > > considering that no one ever needed this for almost three years, > > > > > [...] > > > > > > > > Weird, there's no lockdep support? > > > > > > *ashamed*: apparently no such support currently exist for PPC32. ;-) > > > > Hm, do all the tracers even compile on ppc32 with your patch? > > > > We had periodic build failures on weird, unmaintained architectures > > that had no irqflags-tracing support and hence didnt know the > > raw_irqs_save/restore primitives ... > > > > I'm not trying to make things more difficult for you (and we can > > apply your patch if it builds fine and does not cause problems > > elsewhere), but there were some real downsides to not having proper > > irq APIs ... > > Note, the issue is not with the hooks into local_irq_save/restore, > but with the entry.S code. That code is very sensitive where the > irqs are enabled and disabled. i know. What i'm talking about is that non-lockdep architectures have the habit of not defining raw_local_irq_save() - which the tracing core relies on. Ingo
On Sat, 21 Mar 2009, Ingo Molnar wrote: > > > > > > Hm, do all the tracers even compile on ppc32 with your patch? > > > > > > We had periodic build failures on weird, unmaintained architectures > > > that had no irqflags-tracing support and hence didnt know the > > > raw_irqs_save/restore primitives ... > > > > > > I'm not trying to make things more difficult for you (and we can > > > apply your patch if it builds fine and does not cause problems > > > elsewhere), but there were some real downsides to not having proper > > > irq APIs ... > > > > Note, the issue is not with the hooks into local_irq_save/restore, > > but with the entry.S code. That code is very sensitive where the > > irqs are enabled and disabled. > > i know. What i'm talking about is that non-lockdep architectures > have the habit of not defining raw_local_irq_save() - which the > tracing core relies on. Since we know that's not an issue with PPC32, perhaps we should add (I hate to do this)... depends on TRACE_IRQFLAGS_SUPPORT || PPC32 And document that the "|| PPC32" should go when PowerPC32 gets its act together. :-/ -- Steve
On Sat, 21 Mar 2009, Steven Rostedt wrote: > > Since we know that's not an issue with PPC32, perhaps we should add (I > hate to do this)... > > > depends on TRACE_IRQFLAGS_SUPPORT || PPC32 > > And document that the "|| PPC32" should go when PowerPC32 gets its act > together. :-/ Note, the only tracer broken on PPC32 is the IRQSOFF tracer, and that already depends on TRACE_IRQFLAGS_SUPPORT. -- Steve
* Steven Rostedt <rostedt@goodmis.org> wrote: > > On Sat, 21 Mar 2009, Steven Rostedt wrote: > > > > Since we know that's not an issue with PPC32, perhaps we should add (I > > hate to do this)... > > > > > > depends on TRACE_IRQFLAGS_SUPPORT || PPC32 > > > > And document that the "|| PPC32" should go when PowerPC32 gets its act > > together. :-/ > > Note, the only tracer broken on PPC32 is the IRQSOFF tracer, and > that already depends on TRACE_IRQFLAGS_SUPPORT. Ok, that's fine with me too. Perhaps we could add a TRACING_SUPPORT thing that architectures can enable - but it's probably overkill in this case. Ingo
> > Weird, there's no lockdep support? > > *ashamed*: apparently no such support currently exist for PPC32. ;-) Actually there is a patch that's been floating around (from Dale Farnsworth) for adding irqtrace/lockdep to ppc32 but it seems to be buggy :-) IE, it causes crashes and I haven't had a chance to try to find out yet. I'll probably give it another go one of these days, but if you want to play with it, I'll send it to you directly. Cheers, Ben.
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig index ee70841..774aba7 100644 --- a/kernel/trace/Kconfig +++ b/kernel/trace/Kconfig @@ -63,7 +63,6 @@ config TRACING # config TRACING_SUPPORT bool - depends on TRACE_IRQFLAGS_SUPPORT depends on STACKTRACE_SUPPORT default y
commit 40ada30f9621fbd831ac2437b9a2a399aad34b00 ("tracing: clean up menu"), despite the "clean up" in its purpose, introduced behavioural change for Kconfig symbols: we no longer able to select tracing support on PPC32 (because IRQFLAGS_SUPPORT isn't yet implemented). The IRQFLAGS_SUPPORT is not mandatory for most tracers, tracing core has a special case for platforms w/o irqflags (which, by the way, has become useless as of the commit above). This patch restores the old behaviour, and thus brings the tracing back on PPC32. p.s. The IRQSOFF_TRACER (which is the only tracer that requires IRQFLAGS support) still depends on TRACE_IRQFLAGS_SUPPORT Kconfig symbol. Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com> --- kernel/trace/Kconfig | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)