LinuxLists.cc - [PATCH] tracing: Fix TRACING

[permalink] [raw]

Subject: Re: [PATCH] tracing: Fix TRACING_SUPPORT dependency

* Anton Vorontsov <[email protected]> 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

2009-03-20 19:39:19

[permalink] [raw]

Subject: Re: [PATCH] tracing: Fix TRACING_SUPPORT dependency

On Fri, Mar 20, 2009 at 08:04:28PM +0100, Ingo Molnar wrote:
>
> * Anton Vorontsov <[email protected]> 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
email: [email protected]
irc://irc.freenode.net/bd2

2009-03-20 19:58:24

[permalink] [raw]

Subject: Re: [PATCH] tracing: Fix TRACING_SUPPORT dependency

* Anton Vorontsov <[email protected]> wrote:

> On Fri, Mar 20, 2009 at 08:04:28PM +0100, Ingo Molnar wrote:
> >
> > * Anton Vorontsov <[email protected]> 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

2009-03-20 20:22:57

[permalink] [raw]

Subject: Re: [PATCH] tracing: Fix TRACING_SUPPORT dependency

On Fri, Mar 20, 2009 at 08:57:43PM +0100, Ingo Molnar wrote:
>
> * Anton Vorontsov <[email protected]> wrote:
>
> > On Fri, Mar 20, 2009 at 08:04:28PM +0100, Ingo Molnar wrote:
> > >
> > > * Anton Vorontsov <[email protected]> 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. ;-)

--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2

2009-03-21 03:10:56

[permalink] [raw]

Subject: Re: [PATCH] tracing: Fix TRACING_SUPPORT dependency

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 <[email protected]> wrote:
>
> > On Fri, Mar 20, 2009 at 08:04:28PM +0100, Ingo Molnar wrote:
> > >
> > > * Anton Vorontsov <[email protected]> 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

2009-03-21 16:18:57

[permalink] [raw]

Subject: Re: [PATCH] tracing: Fix TRACING_SUPPORT dependency

* Anton Vorontsov <[email protected]> wrote:

> On Fri, Mar 20, 2009 at 08:57:43PM +0100, Ingo Molnar wrote:
> >
> > * Anton Vorontsov <[email protected]> wrote:
> >
> > > On Fri, Mar 20, 2009 at 08:04:28PM +0100, Ingo Molnar wrote:
> > > >
> > > > * Anton Vorontsov <[email protected]> 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

2009-03-21 16:31:50

[permalink] [raw]

Subject: Re: [PATCH] tracing: Fix TRACING_SUPPORT dependency

On Sat, 21 Mar 2009, Ingo Molnar wrote:

>
> * Anton Vorontsov <[email protected]> wrote:
>
> > On Fri, Mar 20, 2009 at 08:57:43PM +0100, Ingo Molnar wrote:
> > >
> > > * Anton Vorontsov <[email protected]> wrote:
> > >
> > > > On Fri, Mar 20, 2009 at 08:04:28PM +0100, Ingo Molnar wrote:
> > > > >
> > > > > * Anton Vorontsov <[email protected]> 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

2009-03-21 16:34:00

[permalink] [raw]

Subject: Re: [PATCH] tracing: Fix TRACING_SUPPORT dependency

* Steven Rostedt <[email protected]> wrote:

>
> On Sat, 21 Mar 2009, Ingo Molnar wrote:
>
> >
> > * Anton Vorontsov <[email protected]> wrote:
> >
> > > On Fri, Mar 20, 2009 at 08:57:43PM +0100, Ingo Molnar wrote:
> > > >
> > > > * Anton Vorontsov <[email protected]> wrote:
> > > >
> > > > > On Fri, Mar 20, 2009 at 08:04:28PM +0100, Ingo Molnar wrote:
> > > > > >
> > > > > > * Anton Vorontsov <[email protected]> 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

2009-03-21 16:41:54

[permalink] [raw]

Subject: Re: [PATCH] tracing: Fix TRACING_SUPPORT dependency

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

2009-03-21 16:43:40

[permalink] [raw]

Subject: Re: [PATCH] tracing: Fix TRACING_SUPPORT dependency

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

2009-03-21 16:46:45

[permalink] [raw]

Subject: Re: [PATCH] tracing: Fix TRACING_SUPPORT dependency

* Steven Rostedt <[email protected]> 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

2009-03-23 22:07:37

[permalink] [raw]

Subject: [PATCH] tracing: Fix TRACING_SUPPORT dependency for PPC32

commit 40ada30f9621fbd831ac2437b9a2a399aa ("tracing: clean up menu"),
despite the "clean up" in its purpose, introduced a 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).

Though according to Ingo Molnar, there was periodic build failures on
weird, unmaintained architectures that had no irqflags-tracing support
and hence didn't know the raw_irqs_save/restore primitives. Thus we'd
better not enable irqflags-less tracing for all architectures.

This patch restores the old behaviour for PPC32, and thus brings the
tracing back. Other architectures can either add themselves to the
exception list or (better) implement TRACE_IRQFLAGS_SUPPORT.

Signed-off-by: Anton Vorontsov <[email protected]>
---

On Sat, Mar 21, 2009 at 05:18:14PM +0100, Ingo Molnar wrote:
[...]
> > > > 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?

Following config compiles fine:

$ grep TRAC .config
CONFIG_STACKTRACE_SUPPORT=y
# CONFIG_TREE_RCU_TRACE is not set
# CONFIG_PREEMPT_RCU_TRACE is not set
CONFIG_TRACEPOINTS=y
CONFIG_HAVE_ARCH_TRACEHOOK=y
CONFIG_STACKTRACE=y
CONFIG_NOP_TRACER=y
CONFIG_HAVE_FUNCTION_TRACER=y
CONFIG_HAVE_FUNCTION_GRAPH_TRACER=y
CONFIG_HAVE_DYNAMIC_FTRACE=y
CONFIG_HAVE_FTRACE_MCOUNT_RECORD=y
CONFIG_TRACER_MAX_TRACE=y
CONFIG_TRACING=y
CONFIG_TRACING_SUPPORT=y
CONFIG_FUNCTION_TRACER=y
CONFIG_FUNCTION_GRAPH_TRACER=y
CONFIG_SCHED_TRACER=y
CONFIG_CONTEXT_SWITCH_TRACER=y
CONFIG_EVENT_TRACER=y
CONFIG_BOOT_TRACER=y
# CONFIG_TRACE_BRANCH_PROFILING is not set
CONFIG_STACK_TRACER=y
CONFIG_KMEMTRACE=y
CONFIG_WORKQUEUE_TRACER=y
# CONFIG_BLK_DEV_IO_TRACE is not set
CONFIG_DYNAMIC_FTRACE=y
CONFIG_FTRACE_MCOUNT_RECORD=y
CONFIG_FTRACE_SELFTEST=y
CONFIG_FTRACE_STARTUP_TEST=y

With CONFIG_TRACE_BRANCH_PROFILING=y the kernel builds fine too,
but vmlinux is too big to boot (we still can boot it, but we'll
need a bit modified bootloader for that).

OK, how about the patch down below?

kernel/trace/Kconfig | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index ee70841..2246141 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -63,7 +63,11 @@ config TRACING
#
config TRACING_SUPPORT
bool
- depends on TRACE_IRQFLAGS_SUPPORT
+ # PPC32 has no irqflags tracing support, but it can use most of the
+ # tracers anyway, they were tested to build and work. Note that new
+ # exceptions to this list aren't welcomed, better implement the
+ # irqflags tracing for your architecture.
+ depends on TRACE_IRQFLAGS_SUPPORT || PPC32
depends on STACKTRACE_SUPPORT
default y

--
1.5.6.5

2009-03-23 22:25:45

[permalink] [raw]

Subject: [tip:tracing/ftrace] tracing: Fix TRACING_SUPPORT dependency for PPC32

Commit-ID: 45b9560895b07a4a09d55d49235c984db512c5aa
Gitweb: http://git.kernel.org/tip/45b9560895b07a4a09d55d49235c984db512c5aa
Author: Anton Vorontsov <[email protected]>
AuthorDate: Tue, 24 Mar 2009 01:07:24 +0300
Committer: Ingo Molnar <[email protected]>
CommitDate: Mon, 23 Mar 2009 23:23:03 +0100

tracing: Fix TRACING_SUPPORT dependency for PPC32

commit 40ada30f9621fbd831ac2437b9a2a399aa ("tracing: clean up menu"),
despite the "clean up" in its purpose, introduced a 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).

Though according to Ingo Molnar, there was periodic build failures on
weird, unmaintained architectures that had no irqflags-tracing support
and hence didn't know the raw_irqs_save/restore primitives. Thus we'd
better not enable irqflags-less tracing for all architectures.

This patch restores the old behaviour for PPC32, and thus brings the
tracing back. Other architectures can either add themselves to the
exception list or (better) implement TRACE_IRQFLAGS_SUPPORT.

Signed-off-by: Anton Vorontsov <[email protected]>
Acked-b: Steven Rostedt <[email protected]>
Cc: [email protected]
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>

---
kernel/trace/Kconfig | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index b0a46f8..8a4d729 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -63,7 +63,11 @@ config TRACING
#
config TRACING_SUPPORT
bool
- depends on TRACE_IRQFLAGS_SUPPORT
+ # PPC32 has no irqflags tracing support, but it can use most of the
+ # tracers anyway, they were tested to build and work. Note that new
+ # exceptions to this list aren't welcomed, better implement the
+ # irqflags tracing for your architecture.
+ depends on TRACE_IRQFLAGS_SUPPORT || PPC32
depends on STACKTRACE_SUPPORT
default y

2009-03-24 00:52:19