linux-next.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [GIT PULL] tracing: Syscalls trace events + perf support
       [not found] ` <20090812091133.GA21655@elte.hu>
@ 2009-08-18  0:46   ` Paul Mundt
  2009-08-18  7:32     ` Ingo Molnar
  2009-08-18 10:25     ` [GIT PULL] tracing: Syscalls trace events + perf support Frederic Weisbecker
  0 siblings, 2 replies; 10+ messages in thread
From: Paul Mundt @ 2009-08-18  0:46 UTC (permalink / raw)
  To: Ingo Molnar, Stephen Rothwell, Jason Baron
  Cc: Frederic Weisbecker, LKML, Lai Jiangshan, Steven Rostedt,
	Peter Zijlstra, Mathieu Desnoyers, Jiaying Zhang, Martin Bligh,
	Li Zefan, Masami Hiramatsu, Martin Schwidefsky, Wu Zhangjin,
	linux-next

[ Adding to Cc everyone that now has a broken tree thanks to this .. ]

On Wed, Aug 12, 2009 at 11:11:33AM +0200, Ingo Molnar wrote:
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > This pull request integrate one cleanup/fix for ftrace and an 
> > update for syscall tracing: the migration from old-style tracer to 
> > individual tracepoints/trace_events and the support for perf 
> > counter.
> > 
> > I've tested it with success either with ftrace (every syscall 
> > tracepoints enabled at the same time without problems) and with 
> > perfcounter.
> > 
> > May be one drawback: it creates so much trace events that the 
> > ftrace selftests can take some time :-)
> 
> Pulled, thanks a lot!
> 
And this has now subsequently broken every single SH and S390
configuration, and anyone else unfortunate enough to be supporting ftrace
syscall tracing that isn't x86, without so much as a Cc, well done!

The s390 case can be fixed up in-tree as support has already been merged,
but in the SH case we had ftrace syscall tracing queued up for 2.6.32, so
it doesn't show up in -tip, but the end result in -next is now completely
broken.

I'm not sure how we should handle this, if tracing/core in -tip isn't
rebased, should I just pull the topic-branch in to my tree, fix up the sh
support on top of that, and push the end result out? This seems like the
easiest option at least, but I don't know what other dependencies exist
for tracing/core. Alternative suggestions welcome.

This happens again and again with ftrace and -tip, where people just
randomly change existing interfaces, break all of the existing users, and
then fail to tell anyone about it until it shows up in -next. Even if we
had pushed all of the sh ftrace bits to the -tip tree early on it would
not have changed anything, evident by the fact that s390 and all of the
non ftrace syscall architectures were broken by this change as well (the
latter case was at least caught and corrected, although not by the
original authors of this patch series). Is it really that much to task
that people who are running around breaking ftrace interfaces actually
bother to Cc the architectures that are using it?

If -tip is going to perpetuate this sort of half-assed development
methodology, it has no place in -next.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] tracing: Syscalls trace events + perf support
  2009-08-18  0:46   ` [GIT PULL] tracing: Syscalls trace events + perf support Paul Mundt
@ 2009-08-18  7:32     ` Ingo Molnar
  2009-08-18  8:51       ` [S390] ftrace: update system call tracer support Ingo Molnar
       [not found]       ` <20090818085110.GA2074@elte.hu>
  2009-08-18 10:25     ` [GIT PULL] tracing: Syscalls trace events + perf support Frederic Weisbecker
  1 sibling, 2 replies; 10+ messages in thread
From: Ingo Molnar @ 2009-08-18  7:32 UTC (permalink / raw)
  To: Paul Mundt, Stephen Rothwell, Jason Baron, Frederic Weisbecker,
	LKML


* Paul Mundt <lethal@linux-sh.org> wrote:

> [ Adding to Cc everyone that now has a broken tree thanks to this .. ]
> 
> On Wed, Aug 12, 2009 at 11:11:33AM +0200, Ingo Molnar wrote:
> > * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > > This pull request integrate one cleanup/fix for ftrace and an 
> > > update for syscall tracing: the migration from old-style tracer to 
> > > individual tracepoints/trace_events and the support for perf 
> > > counter.
> > > 
> > > I've tested it with success either with ftrace (every syscall 
> > > tracepoints enabled at the same time without problems) and with 
> > > perfcounter.
> > > 
> > > May be one drawback: it creates so much trace events that the 
> > > ftrace selftests can take some time :-)
> > 
> > Pulled, thanks a lot!
> 
> And this has now subsequently broken every single SH and S390 
> configuration, [...]

I test SH cross-builds regularly. I just checked the SH defconfig 
and it builds just fine here:

$ make -j32 CROSS_COMPILE=sh3-linux- ARCH=sh vmlinux

...
  CC      init/version.o
  LD      init/built-in.o
  LD      .tmp_vmlinux1
  KSYM    .tmp_kallsyms1.S
  AS      .tmp_kallsyms1.o
  LD      .tmp_vmlinux2
  KSYM    .tmp_kallsyms2.S
  AS      .tmp_kallsyms2.o
  LD      vmlinux
  SYSMAP  System.map
  SYSMAP  .tmp_System.map

 phoenix:~/linux/linux> head .config 
 #
 # Automatically generated make config: don't edit
 # Linux kernel version: 2.6.31-rc6
 # Tue Aug 18 09:24:28 2009
 #
 CONFIG_SUPERH=y
 CONFIG_SUPERH32=y
 # CONFIG_SUPERH64 is not set
 CONFIG_ARCH_DEFCONFIG="arch/sh/configs/shx3_defconfig"

AFAICS SH does not even have any syscall tracing added upstream. 
Apparently you added them in the SH tree and then they got 
integrated in linux-next, and the integrated end result broke?

Mind putting those bits into a separate Git branch and sending them 
to the tracing tree too so that we can make sure it's properly 
integrated and tested and that any changes to the generic facility 
are propagated to SH too?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [S390] ftrace: update system call tracer support
  2009-08-18  7:32     ` Ingo Molnar
@ 2009-08-18  8:51       ` Ingo Molnar
       [not found]       ` <20090818085110.GA2074@elte.hu>
  1 sibling, 0 replies; 10+ messages in thread
From: Ingo Molnar @ 2009-08-18  8:51 UTC (permalink / raw)
  To: Paul Mundt, Stephen Rothwell, Jason Baron, Frederic Weisbecker,
	LKML


* Ingo Molnar <mingo@elte.hu> wrote:

> * Paul Mundt <lethal@linux-sh.org> wrote:
> 
> > [ Adding to Cc everyone that now has a broken tree thanks to this .. ]
> > 
> > On Wed, Aug 12, 2009 at 11:11:33AM +0200, Ingo Molnar wrote:
> > > * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > > > This pull request integrate one cleanup/fix for ftrace and an 
> > > > update for syscall tracing: the migration from old-style tracer to 
> > > > individual tracepoints/trace_events and the support for perf 
> > > > counter.
> > > > 
> > > > I've tested it with success either with ftrace (every syscall 
> > > > tracepoints enabled at the same time without problems) and with 
> > > > perfcounter.
> > > > 
> > > > May be one drawback: it creates so much trace events that the 
> > > > ftrace selftests can take some time :-)
> > > 
> > > Pulled, thanks a lot!
> > 
> > And this has now subsequently broken every single SH and S390 
> > configuration, [...]
> 
> I test SH cross-builds regularly. I just checked the SH defconfig 
> and it builds just fine here:
> 
> $ make -j32 CROSS_COMPILE=sh3-linux- ARCH=sh vmlinux

The s390 build indeed broke. (This got masked by the s390 toolchain 
i'm using not having been able to build Linus's tree - i fixed 
that.)

Could you try the fix below? It does the trick here.

Martin, Heiko - does the fix look good to you? regs->gprs[2] seems 
to be the register used for both the syscall number (enter 
callback) and for the return code (exit callback).

Regarding SH, the fixup should be similarly trivial. Since SH's 
FTRACE_SYSCALLS code is not upstream yet it can (and should) be 
carried in the tree that integrates the SH tree and the tracing 
tree - linux-next in this case.

Thanks,

	Ingo

------------------------------>
>From a9008fd42b1c3c89f684d90bdfb9c2d05c7af119 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Tue, 18 Aug 2009 10:41:57 +0200
Subject: [PATCH] [S390] ftrace: update system call tracer support

Commit fb34a08c3 ("tracing: Add trace events for each syscall
entry/exit") changed the lowlevel API to ftrace syscall tracing
but did not update s390 which started making use of it recently.

This broke the s390 build, as reported by Paul Mundt.

Update the callbacks with the syscall number and the syscall
return code values. This allows per syscall tracepoints,
syscall argument enumeration /debug/tracing/events/syscalls/
and perfcounters support and integration on s390 too.

Reported-by: Paul Mundt <lethal@linux-sh.org>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Jason Baron <jbaron@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
LKML-Reference: <tip-fb34a08c3469b2be9eae626ccb96476b4687b810@git.kernel.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/s390/kernel/ptrace.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c
index 43acd73..05f57cd 100644
--- a/arch/s390/kernel/ptrace.c
+++ b/arch/s390/kernel/ptrace.c
@@ -662,7 +662,7 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs)
 	}
 
 	if (unlikely(test_thread_flag(TIF_SYSCALL_FTRACE)))
-		ftrace_syscall_enter(regs);
+		trace_syscall_enter(regs, regs->gprs[2]);
 
 	if (unlikely(current->audit_context))
 		audit_syscall_entry(is_compat_task() ?
@@ -680,7 +680,7 @@ asmlinkage void do_syscall_trace_exit(struct pt_regs *regs)
 				   regs->gprs[2]);
 
 	if (unlikely(test_thread_flag(TIF_SYSCALL_FTRACE)))
-		ftrace_syscall_exit(regs);
+		trace_syscall_exit(regs, regs->gprs[2]);
 
 	if (test_thread_flag(TIF_SYSCALL_TRACE))
 		tracehook_report_syscall_exit(regs, 0);

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [S390] ftrace: update system call tracer support
       [not found]       ` <20090818085110.GA2074@elte.hu>
@ 2009-08-18  8:59         ` Martin Schwidefsky
  2009-08-18 10:05           ` Ingo Molnar
  2009-08-18 14:56         ` Jason Baron
  1 sibling, 1 reply; 10+ messages in thread
From: Martin Schwidefsky @ 2009-08-18  8:59 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Paul Mundt, Stephen Rothwell, Jason Baron, Frederic Weisbecker,
	LKML, Lai Jiangshan, Steven Rostedt, Peter Zijlstra,
	Mathieu Desnoyers, Jiaying Zhang, Martin Bligh, Li Zefan,
	Masami Hiramatsu, Wu Zhangjin, linux-next, Heiko Carstens

On Tue, 18 Aug 2009 10:51:10 +0200
Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Ingo Molnar <mingo@elte.hu> wrote:
> 
> > * Paul Mundt <lethal@linux-sh.org> wrote:
> > 
> > > [ Adding to Cc everyone that now has a broken tree thanks to this .. ]
> > > 
> > > On Wed, Aug 12, 2009 at 11:11:33AM +0200, Ingo Molnar wrote:
> > > > * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > > > > This pull request integrate one cleanup/fix for ftrace and an 
> > > > > update for syscall tracing: the migration from old-style tracer to 
> > > > > individual tracepoints/trace_events and the support for perf 
> > > > > counter.
> > > > > 
> > > > > I've tested it with success either with ftrace (every syscall 
> > > > > tracepoints enabled at the same time without problems) and with 
> > > > > perfcounter.
> > > > > 
> > > > > May be one drawback: it creates so much trace events that the 
> > > > > ftrace selftests can take some time :-)
> > > > 
> > > > Pulled, thanks a lot!
> > > 
> > > And this has now subsequently broken every single SH and S390 
> > > configuration, [...]
> > 
> > I test SH cross-builds regularly. I just checked the SH defconfig 
> > and it builds just fine here:
> > 
> > $ make -j32 CROSS_COMPILE=sh3-linux- ARCH=sh vmlinux
> 
> The s390 build indeed broke. (This got masked by the s390 toolchain 
> i'm using not having been able to build Linus's tree - i fixed 
> that.)
> 
> Could you try the fix below? It does the trick here.
> 
> Martin, Heiko - does the fix look good to you? regs->gprs[2] seems 
> to be the register used for both the syscall number (enter 
> callback) and for the return code (exit callback).

Correct, for do_syscall_trace_{enter,exit} the code in entry.S stores
the system call number in regs->gprs[2]. The fix is fine.
Thanks Ingo.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [S390] ftrace: update system call tracer support
  2009-08-18  8:59         ` Martin Schwidefsky
@ 2009-08-18 10:05           ` Ingo Molnar
  2009-08-18 10:22             ` Martin Schwidefsky
  0 siblings, 1 reply; 10+ messages in thread
From: Ingo Molnar @ 2009-08-18 10:05 UTC (permalink / raw)
  To: Martin Schwidefsky
  Cc: Paul Mundt, Stephen Rothwell, Jason Baron, Frederic Weisbecker,
	LKML, Lai Jiangshan, Steven Rostedt, Peter Zijlstra,
	Mathieu Desnoyers, Jiaying Zhang, Martin Bligh, Li Zefan,
	Masami Hiramatsu, Wu Zhangjin, linux-next, Heiko Carstens


* Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:

> On Tue, 18 Aug 2009 10:51:10 +0200
> Ingo Molnar <mingo@elte.hu> wrote:
> 
> > 
> > * Ingo Molnar <mingo@elte.hu> wrote:
> > 
> > > * Paul Mundt <lethal@linux-sh.org> wrote:
> > > 
> > > > [ Adding to Cc everyone that now has a broken tree thanks to this .. ]
> > > > 
> > > > On Wed, Aug 12, 2009 at 11:11:33AM +0200, Ingo Molnar wrote:
> > > > > * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > > > > > This pull request integrate one cleanup/fix for ftrace and an 
> > > > > > update for syscall tracing: the migration from old-style tracer to 
> > > > > > individual tracepoints/trace_events and the support for perf 
> > > > > > counter.
> > > > > > 
> > > > > > I've tested it with success either with ftrace (every syscall 
> > > > > > tracepoints enabled at the same time without problems) and with 
> > > > > > perfcounter.
> > > > > > 
> > > > > > May be one drawback: it creates so much trace events that the 
> > > > > > ftrace selftests can take some time :-)
> > > > > 
> > > > > Pulled, thanks a lot!
> > > > 
> > > > And this has now subsequently broken every single SH and S390 
> > > > configuration, [...]
> > > 
> > > I test SH cross-builds regularly. I just checked the SH defconfig 
> > > and it builds just fine here:
> > > 
> > > $ make -j32 CROSS_COMPILE=sh3-linux- ARCH=sh vmlinux
> > 
> > The s390 build indeed broke. (This got masked by the s390 toolchain 
> > i'm using not having been able to build Linus's tree - i fixed 
> > that.)
> > 
> > Could you try the fix below? It does the trick here.
> > 
> > Martin, Heiko - does the fix look good to you? regs->gprs[2] seems 
> > to be the register used for both the syscall number (enter 
> > callback) and for the return code (exit callback).
> 
> Correct, for do_syscall_trace_{enter,exit} the code in entry.S 
> stores the system call number in regs->gprs[2]. The fix is fine. 
> Thanks Ingo.

Thanks, i've added your Acked-by to the commit. I suspect you dont 
want to pull tracing infrastructure changes into the S390 tree, so 
keeping this fix in the tracing tree would be the best option?

	Ingo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [S390] ftrace: update system call tracer support
  2009-08-18 10:05           ` Ingo Molnar
@ 2009-08-18 10:22             ` Martin Schwidefsky
  0 siblings, 0 replies; 10+ messages in thread
From: Martin Schwidefsky @ 2009-08-18 10:22 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Paul Mundt, Stephen Rothwell, Jason Baron, Frederic Weisbecker,
	LKML, Lai Jiangshan, Steven Rostedt, Peter Zijlstra,
	Mathieu Desnoyers, Jiaying Zhang, Martin Bligh, Li Zefan,
	Masami Hiramatsu, Wu Zhangjin, linux-next, Heiko Carstens

On Tue, 18 Aug 2009 12:05:26 +0200
Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
> 
> > On Tue, 18 Aug 2009 10:51:10 +0200
> > Ingo Molnar <mingo@elte.hu> wrote:
> > 
> > > 
> > > * Ingo Molnar <mingo@elte.hu> wrote:
> > > 
> > > > * Paul Mundt <lethal@linux-sh.org> wrote:
> > > > 
> > > > > [ Adding to Cc everyone that now has a broken tree thanks to this .. ]
> > > > > 
> > > > > On Wed, Aug 12, 2009 at 11:11:33AM +0200, Ingo Molnar wrote:
> > > > > > * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > > > > > > This pull request integrate one cleanup/fix for ftrace and an 
> > > > > > > update for syscall tracing: the migration from old-style tracer to 
> > > > > > > individual tracepoints/trace_events and the support for perf 
> > > > > > > counter.
> > > > > > > 
> > > > > > > I've tested it with success either with ftrace (every syscall 
> > > > > > > tracepoints enabled at the same time without problems) and with 
> > > > > > > perfcounter.
> > > > > > > 
> > > > > > > May be one drawback: it creates so much trace events that the 
> > > > > > > ftrace selftests can take some time :-)
> > > > > > 
> > > > > > Pulled, thanks a lot!
> > > > > 
> > > > > And this has now subsequently broken every single SH and S390 
> > > > > configuration, [...]
> > > > 
> > > > I test SH cross-builds regularly. I just checked the SH defconfig 
> > > > and it builds just fine here:
> > > > 
> > > > $ make -j32 CROSS_COMPILE=sh3-linux- ARCH=sh vmlinux
> > > 
> > > The s390 build indeed broke. (This got masked by the s390 toolchain 
> > > i'm using not having been able to build Linus's tree - i fixed 
> > > that.)
> > > 
> > > Could you try the fix below? It does the trick here.
> > > 
> > > Martin, Heiko - does the fix look good to you? regs->gprs[2] seems 
> > > to be the register used for both the syscall number (enter 
> > > callback) and for the return code (exit callback).
> > 
> > Correct, for do_syscall_trace_{enter,exit} the code in entry.S 
> > stores the system call number in regs->gprs[2]. The fix is fine. 
> > Thanks Ingo.
> 
> Thanks, i've added your Acked-by to the commit. I suspect you dont 
> want to pull tracing infrastructure changes into the S390 tree, so 
> keeping this fix in the tracing tree would be the best option?

Indeed, the patch that introduced the api change is in the tracing tree
so it makes sense to put the s390 adaptions there too.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] tracing: Syscalls trace events + perf support
  2009-08-18  0:46   ` [GIT PULL] tracing: Syscalls trace events + perf support Paul Mundt
  2009-08-18  7:32     ` Ingo Molnar
@ 2009-08-18 10:25     ` Frederic Weisbecker
  2009-08-18 11:06       ` Ingo Molnar
  1 sibling, 1 reply; 10+ messages in thread
From: Frederic Weisbecker @ 2009-08-18 10:25 UTC (permalink / raw)
  To: Paul Mundt, Ingo Molnar, Stephen Rothwell, Jason Baron, LKML, Lai

On Tue, Aug 18, 2009 at 09:46:55AM +0900, Paul Mundt wrote:
> [ Adding to Cc everyone that now has a broken tree thanks to this .. ]
> 
> On Wed, Aug 12, 2009 at 11:11:33AM +0200, Ingo Molnar wrote:
> > * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > > This pull request integrate one cleanup/fix for ftrace and an 
> > > update for syscall tracing: the migration from old-style tracer to 
> > > individual tracepoints/trace_events and the support for perf 
> > > counter.
> > > 
> > > I've tested it with success either with ftrace (every syscall 
> > > tracepoints enabled at the same time without problems) and with 
> > > perfcounter.
> > > 
> > > May be one drawback: it creates so much trace events that the 
> > > ftrace selftests can take some time :-)
> > 
> > Pulled, thanks a lot!
> > 
> And this has now subsequently broken every single SH and S390
> configuration, and anyone else unfortunate enough to be supporting ftrace
> syscall tracing that isn't x86, without so much as a Cc, well done!
> 
> The s390 case can be fixed up in-tree as support has already been merged,
> but in the SH case we had ftrace syscall tracing queued up for 2.6.32, so
> it doesn't show up in -tip, but the end result in -next is now completely
> broken.
> 
> I'm not sure how we should handle this, if tracing/core in -tip isn't
> rebased, should I just pull the topic-branch in to my tree, fix up the sh
> support on top of that, and push the end result out? This seems like the
> easiest option at least, but I don't know what other dependencies exist
> for tracing/core. Alternative suggestions welcome.
> 
> This happens again and again with ftrace and -tip, where people just
> randomly change existing interfaces, break all of the existing users, and
> then fail to tell anyone about it until it shows up in -next. Even if we
> had pushed all of the sh ftrace bits to the -tip tree early on it would
> not have changed anything, evident by the fact that s390 and all of the
> non ftrace syscall architectures were broken by this change as well (the
> latter case was at least caught and corrected, although not by the
> original authors of this patch series). Is it really that much to task
> that people who are running around breaking ftrace interfaces actually
> bother to Cc the architectures that are using it?



I've just retrieved the concerned commit in the sh tree:

sh: Add ftrace syscall tracing support (c652d780c9cf7f860141de232b37160fe013feca)

Was I cc'ed on this one? I can't find it in my inbox. Unless I'm wrong
and I missed it, how could I guess I had to cc you and how am I supposed
to fix something I'm even not aware of?


I can't find the s390 patch in my inbox either (was I cc'ed ?)
([S390] ftrace: add system call tracer support) but we should have fixed
this one because it was already upstream and a git-grep ftrace_syscall_enter
would have warned us about that.

I didn't know another arch was supporting syscall tracing (except mips because
I was cc'ed, but it doesn't seem upstream nor in the mips tree).


> 
> If -tip is going to perpetuate this sort of half-assed development
> methodology, it has no place in -next.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] tracing: Syscalls trace events + perf support
  2009-08-18 10:25     ` [GIT PULL] tracing: Syscalls trace events + perf support Frederic Weisbecker
@ 2009-08-18 11:06       ` Ingo Molnar
  2009-08-18 11:56         ` Paul Mundt
  0 siblings, 1 reply; 10+ messages in thread
From: Ingo Molnar @ 2009-08-18 11:06 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Paul Mundt, Stephen Rothwell, Jason Baron, LKML, Lai Jiangshan,
	Steven Rostedt, Peter Zijlstra, Mathieu Desnoyers, Jiaying Zhang,
	Martin Bligh, Li Zefan, Masami Hiramatsu, Martin Schwidefsky,
	Wu Zhangjin, linux-next


* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> On Tue, Aug 18, 2009 at 09:46:55AM +0900, Paul Mundt wrote:
> > [ Adding to Cc everyone that now has a broken tree thanks to this .. ]
> > 
> > On Wed, Aug 12, 2009 at 11:11:33AM +0200, Ingo Molnar wrote:
> > > * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > > > This pull request integrate one cleanup/fix for ftrace and an 
> > > > update for syscall tracing: the migration from old-style tracer to 
> > > > individual tracepoints/trace_events and the support for perf 
> > > > counter.
> > > > 
> > > > I've tested it with success either with ftrace (every syscall 
> > > > tracepoints enabled at the same time without problems) and with 
> > > > perfcounter.
> > > > 
> > > > May be one drawback: it creates so much trace events that the 
> > > > ftrace selftests can take some time :-)
> > > 
> > > Pulled, thanks a lot!
> > 
> > And this has now subsequently broken every single SH and S390 
> > configuration, and anyone else unfortunate enough to be 
> > supporting ftrace syscall tracing that isn't x86, without so 
> > much as a Cc, well done!
> > 
> > The s390 case can be fixed up in-tree as support has already 
> > been merged, but in the SH case we had ftrace syscall tracing 
> > queued up for 2.6.32, so it doesn't show up in -tip, but the 
> > end result in -next is now completely broken.
> > 
> > I'm not sure how we should handle this, if tracing/core in -tip 
> > isn't rebased, should I just pull the topic-branch in to my 
> > tree, fix up the sh support on top of that, and push the end 
> > result out? This seems like the easiest option at least, but I 
> > don't know what other dependencies exist for tracing/core. 
> > Alternative suggestions welcome.
> > 
> > This happens again and again with ftrace and -tip, where people 
> > just randomly change existing interfaces, break all of the 
> > existing users, and then fail to tell anyone about it until it 
> > shows up in -next. Even if we had pushed all of the sh ftrace 
> > bits to the -tip tree early on it would not have changed 
> > anything, evident by the fact that s390 and all of the non 
> > ftrace syscall architectures were broken by this change as well 
> > (the latter case was at least caught and corrected, although 
> > not by the original authors of this patch series). Is it really 
> > that much to task that people who are running around breaking 
> > ftrace interfaces actually bother to Cc the architectures that 
> > are using it?
> 
> I've just retrieved the concerned commit in the sh tree:
> 
> sh: Add ftrace syscall tracing support 
> (c652d780c9cf7f860141de232b37160fe013feca)
> 
> Was I cc'ed on this one? I can't find it in my inbox. Unless I'm 
> wrong and I missed it, how could I guess I had to cc you and how 
> am I supposed to fix something I'm even not aware of?
> 
> I can't find the s390 patch in my inbox either (was I cc'ed ?) 
> ([S390] ftrace: add system call tracer support) but we should 
> have fixed this one because it was already upstream and a 
> git-grep ftrace_syscall_enter would have warned us about that.
> 
> I didn't know another arch was supporting syscall tracing (except 
> mips because I was cc'ed, but it doesn't seem upstream nor in the 
> mips tree).

Yes, and note that Paul Mundt has not even done the minimal 
courtesy of describing/pasting the build failure he was seeing. But 
he had time for a 4-paragraph rant about how others are supposed to 
do their work... And then he complains about us not having 
considered a change he never Cc:-ed to us. How nice.

( I have meanwhile fixed the bug in s390 and have posted that fix -
  the SH fix should be an analogous twoliner. )

This reply from Paul Mundt shows an _incredibly_ arrogant attitude 
towards core kernel facilities: he almost never contributes to them 
(i just checked the logs from v2.6.12 to v2.6.31-rc6) but _THEY_ 
must do everything to keep SH running smoothly.

And he expects that work to benefit SH to happen regardless of how 
many actual Linux users use, develop on and report bugs from SH. 
Where's the many SH crash reports on kerneloops.org? I see _not a 
single SH report_, out of more than 200,000 kernel oopses reported 
and categorized ... Why?

The thing is, as things stand today SH is basically freeloding on 
the hard work, testing and core kernel work of other architectures 
- which is fine in itself - what is not fine is to is to then 
complain in an unacceptable tone about bugs that affect SH.

Of course SH never causes core kernel bugs because it essentially 
does no core kernel work at all!

Paul seems to think that freeloading upon 10 million lines of code 
coupled with loud, self-sure demands for instant fixes is fine, and 
dare anyone trying to advance Linux break the build of a single 
architecture out of 22!

Other architectures, powerpc, s390, sparc, x86 etc. all do lots of 
core kernel work and make sure it's all nicely reciprocal and 
friendly. SH needs to stop the whining and needs to start helping 
out for real - or it can get unmerged and go out of tree if it does 
not want to participate in the development process.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [GIT PULL] tracing: Syscalls trace events + perf support
  2009-08-18 11:06       ` Ingo Molnar
@ 2009-08-18 11:56         ` Paul Mundt
  0 siblings, 0 replies; 10+ messages in thread
From: Paul Mundt @ 2009-08-18 11:56 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Frederic Weisbecker, Stephen Rothwell, Jason Baron, LKML,
	Lai Jiangshan, Steven Rostedt, Peter Zijlstra, Mathieu Desnoyers,
	Jiaying Zhang, Martin Bligh, Li Zefan, Masami Hiramatsu,
	Martin Schwidefsky, Wu Zhangjin, linux-next

On Tue, Aug 18, 2009 at 01:06:16PM +0200, Ingo Molnar wrote:
> * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > I've just retrieved the concerned commit in the sh tree:
> > 
> > sh: Add ftrace syscall tracing support 
> > (c652d780c9cf7f860141de232b37160fe013feca)
> > 
> > Was I cc'ed on this one? I can't find it in my inbox. Unless I'm 
> > wrong and I missed it, how could I guess I had to cc you and how 
> > am I supposed to fix something I'm even not aware of?
> > 
> > I can't find the s390 patch in my inbox either (was I cc'ed ?) 
> > ([S390] ftrace: add system call tracer support) but we should 
> > have fixed this one because it was already upstream and a 
> > git-grep ftrace_syscall_enter would have warned us about that.
> > 
> > I didn't know another arch was supporting syscall tracing (except 
> > mips because I was cc'ed, but it doesn't seem upstream nor in the 
> > mips tree).
> 
> Yes, and note that Paul Mundt has not even done the minimal 
> courtesy of describing/pasting the build failure he was seeing. But 
> he had time for a 4-paragraph rant about how others are supposed to 
> do their work... And then he complains about us not having 
> considered a change he never Cc:-ed to us. How nice.
> 
I explained that all -next builds had broken due to that tree being
pulled in, which I assumed was sufficient. None of my initial post was
suggesting that there was a problem with making changes to the interfaces
or that I expected anyone else to fix them up for me, it is more just
general frustration at how often people blindly push things out to -next
without checking what impact that is going to have on the already merged
trees. This is not just -tip specific, and indeed most of my posts to the
-next list are fixing up these sorts of build bugs caused by other
people's trees.

> This reply from Paul Mundt shows an _incredibly_ arrogant attitude 
> towards core kernel facilities: he almost never contributes to them 
> (i just checked the logs from v2.6.12 to v2.6.31-rc6) but _THEY_ 
> must do everything to keep SH running smoothly.
> 
I don't think that's a fair generalization. Naturally the vast majority
of my work is in my own architecture, and I send patches for core bits
when we run in to trouble, have to extend certain parts of
infrastructure, discover something is broken whilst wiring up support for
a few feature, etc, etc. And this actually happens quite regularly, so
I'm unsure as to where you get your statistics from.

> And he expects that work to benefit SH to happen regardless of how 
> many actual Linux users use, develop on and report bugs from SH. 
> Where's the many SH crash reports on kerneloops.org? I see _not a 
> single SH report_, out of more than 200,000 kernel oopses reported 
> and categorized ... Why?
> 
I have enough trouble getting people to submit oopses at all, and the
ones that we do get are most often in the architecture code, so there's
little interest to kerneloops.org. Likewise, issues that we hit with
common code during development we try to debug and send patches for long
before it becomes anyone elses problem.

> The thing is, as things stand today SH is basically freeloding on 
> the hard work, testing and core kernel work of other architectures 
> - which is fine in itself - what is not fine is to is to then 
> complain in an unacceptable tone about bugs that affect SH.
> 
This is complete and utter nonsense. SH is one of the only embedded
architectures that aggressively implements and tests new kernel features,
and we send out patches for any issues we run in to on a pretty much
constant basis. Beyond that, most of the patches I send to -next are
fixing up issues introduced by other people that have negative
implications for architectures, and I try to subsequently take care of
those as quickly as possible. The core kernel work we do focus on happens
within that particular subsystem itself, so perhaps there is not so much
visibility outside of that context.

Regarding the ftrace syscall stuff, you did not just break one
architecture, you effectively broke all of them that weren't x86, and
this tends to be unfortunately more of a common trend than an odd
exception. Of course that is a natural part of development, and as long
as things are gradually fixed up it's really not that big of a deal. No
one expects instant fixes all around, the issue is more that no one is
paying attention to what impact these changes will have on others. In
this case we could have Cced you on the ftrace changes we made in the
sh tree so you had been aware of it, but this largely ignores the point.
I don't Cc people when we add new features on the architecture side as
for the most part, and I expect this is the case for most architecture
maintainers. How many times are architecture people told to grovel
around some -tip topic branch before making any changes in their own
trees? If architecture people have to poke around -tip to try and keep
things moving along, I don't think its unrealistic to expect -tip people
to pay attention to the things that are already on-going in linux-next
before merging things.

My "hostility" towards core kernel features tends to be inversely
proportional to how embedded architectures are treated by core kernel
people. We effectively have to fight for every minor change, and are
either dismissed out of hand or regarded with contempt when attempting to
push core changes through. One hardly has to look very long to find your
postings that routinely throw this 95% figure around for example. Folks
complain about embedded architectures not contributing, and then when
they do, this is the end result. As an embedded architecture maintainer I
resigned myself years ago to the fact that I would spend most of my time
outside of my own architecture directory fixing up other peoples code,
and it's only the rare times when this results in opposition -- largely
involving the -tip tree as of late. I even asked you directly how we can
fix this workflow issue in my initial mail, which you completely ignored.

In any event, if all of this constitutes freeloading, then there seems to
be very little point in carrying on dialogue. I'll just pull in the
tracing/core bits in to a topic branch and fix my platform up, as usual.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [S390] ftrace: update system call tracer support
       [not found]       ` <20090818085110.GA2074@elte.hu>
  2009-08-18  8:59         ` Martin Schwidefsky
@ 2009-08-18 14:56         ` Jason Baron
  1 sibling, 0 replies; 10+ messages in thread
From: Jason Baron @ 2009-08-18 14:56 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Paul Mundt, Stephen Rothwell, Frederic Weisbecker, LKML,
	Lai Jiangshan, Steven Rostedt, Peter Zijlstra, Mathieu Desnoyers,
	Jiaying Zhang, Martin Bligh, Li Zefan, Masami Hiramatsu,
	Martin Schwidefsky, Wu Zhangjin, linux-next, Heiko Carstens

On Tue, Aug 18, 2009 at 10:51:10AM +0200, Ingo Molnar wrote:
> * Ingo Molnar <mingo@elte.hu> wrote:
> 
> > * Paul Mundt <lethal@linux-sh.org> wrote:
> > 
> > > [ Adding to Cc everyone that now has a broken tree thanks to this .. ]
> > > 
> > > On Wed, Aug 12, 2009 at 11:11:33AM +0200, Ingo Molnar wrote:
> > > > * Frederic Weisbecker <fweisbec@gmail.com> wrote:
> > > > > This pull request integrate one cleanup/fix for ftrace and an 
> > > > > update for syscall tracing: the migration from old-style tracer to 
> > > > > individual tracepoints/trace_events and the support for perf 
> > > > > counter.
> > > > > 
> > > > > I've tested it with success either with ftrace (every syscall 
> > > > > tracepoints enabled at the same time without problems) and with 
> > > > > perfcounter.
> > > > > 
> > > > > May be one drawback: it creates so much trace events that the 
> > > > > ftrace selftests can take some time :-)
> > > > 
> > > > Pulled, thanks a lot!
> > > 
> > > And this has now subsequently broken every single SH and S390 
> > > configuration, [...]
> > 
> > I test SH cross-builds regularly. I just checked the SH defconfig 
> > and it builds just fine here:
> > 
> > $ make -j32 CROSS_COMPILE=sh3-linux- ARCH=sh vmlinux
> 
> The s390 build indeed broke. (This got masked by the s390 toolchain 
> i'm using not having been able to build Linus's tree - i fixed 
> that.)
> 
> Could you try the fix below? It does the trick here.
> 
> Martin, Heiko - does the fix look good to you? regs->gprs[2] seems 
> to be the register used for both the syscall number (enter 
> callback) and for the return code (exit callback).
> 
> Regarding SH, the fixup should be similarly trivial. Since SH's 
> FTRACE_SYSCALLS code is not upstream yet it can (and should) be 
> carried in the tree that integrates the SH tree and the tracing 
> tree - linux-next in this case.
> 
> Thanks,
> 
> 	Ingo
> 
> ------------------------------>
> From a9008fd42b1c3c89f684d90bdfb9c2d05c7af119 Mon Sep 17 00:00:00 2001
> From: Ingo Molnar <mingo@elte.hu>
> Date: Tue, 18 Aug 2009 10:41:57 +0200
> Subject: [PATCH] [S390] ftrace: update system call tracer support
> 
> Commit fb34a08c3 ("tracing: Add trace events for each syscall
> entry/exit") changed the lowlevel API to ftrace syscall tracing
> but did not update s390 which started making use of it recently.
> 
> This broke the s390 build, as reported by Paul Mundt.
> 
> Update the callbacks with the syscall number and the syscall
> return code values. This allows per syscall tracepoints,
> syscall argument enumeration /debug/tracing/events/syscalls/
> and perfcounters support and integration on s390 too.
> 
> Reported-by: Paul Mundt <lethal@linux-sh.org>
> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Cc: Jason Baron <jbaron@redhat.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> LKML-Reference: <tip-fb34a08c3469b2be9eae626ccb96476b4687b810@git.kernel.org>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
>  arch/s390/kernel/ptrace.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/kernel/ptrace.c b/arch/s390/kernel/ptrace.c
> index 43acd73..05f57cd 100644
> --- a/arch/s390/kernel/ptrace.c
> +++ b/arch/s390/kernel/ptrace.c
> @@ -662,7 +662,7 @@ asmlinkage long do_syscall_trace_enter(struct pt_regs *regs)
>  	}
>  
>  	if (unlikely(test_thread_flag(TIF_SYSCALL_FTRACE)))
> -		ftrace_syscall_enter(regs);
> +		trace_syscall_enter(regs, regs->gprs[2]);
>  
>  	if (unlikely(current->audit_context))
>  		audit_syscall_entry(is_compat_task() ?
> @@ -680,7 +680,7 @@ asmlinkage void do_syscall_trace_exit(struct pt_regs *regs)
>  				   regs->gprs[2]);
>  
>  	if (unlikely(test_thread_flag(TIF_SYSCALL_FTRACE)))
> -		ftrace_syscall_exit(regs);
> +		trace_syscall_exit(regs, regs->gprs[2]);
>  
>  	if (test_thread_flag(TIF_SYSCALL_TRACE))
>  		tracehook_report_syscall_exit(regs, 0);

thanks for fixing this up Ingo! Sorry for all the trouble.

perhaps, we should just reduce the entry/exit routines to just pass
'regs' since we already have the following abstractions for callbacks:

syscall_get_nr(current, regs);
syscall_get_return_value(current, regs);

Thus, we really only need to be passing 'regs'. I was passing more information
here, to make it easier for other users of the tracepoints...

thanks,

-Jason

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2009-08-18 14:58 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1250016545-6601-1-git-send-email-fweisbec@gmail.com>
     [not found] ` <20090812091133.GA21655@elte.hu>
2009-08-18  0:46   ` [GIT PULL] tracing: Syscalls trace events + perf support Paul Mundt
2009-08-18  7:32     ` Ingo Molnar
2009-08-18  8:51       ` [S390] ftrace: update system call tracer support Ingo Molnar
     [not found]       ` <20090818085110.GA2074@elte.hu>
2009-08-18  8:59         ` Martin Schwidefsky
2009-08-18 10:05           ` Ingo Molnar
2009-08-18 10:22             ` Martin Schwidefsky
2009-08-18 14:56         ` Jason Baron
2009-08-18 10:25     ` [GIT PULL] tracing: Syscalls trace events + perf support Frederic Weisbecker
2009-08-18 11:06       ` Ingo Molnar
2009-08-18 11:56         ` Paul Mundt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).