Re: [GIT PULL] tracing: make signal tracepoints more useful

From: Ingo Molnar
Date: Mon Jan 16 2012 - 02:46:00 EST



* Oleg Nesterov <oleg@xxxxxxxxxx> wrote:

> On 01/13, Oleg Nesterov wrote:
> >
> > Hello,
>
> ping ;)
>
> > Please pull from
> >
> > git://github.com/utrace/linux sigtrace
> >
> >
> > Another (4th) attempt to push these simple changes, now in the form
> > of a pull request (yes, github, I still can't restore my korg account).
> >
> > 2/2 looks like a bugfix to me, but 1/2 changes the output from
> > trace_signal_generate() and removes trace_signal_overflow_fail.
> > In essence the change is:
> >
> > - TP_printk("sig=%d errno=%d code=%d comm=%s pid=%d",
> > + TP_printk("sig=%d errno=%d code=%d comm=%s pid=%d grp=%d res=%d",
> >
> > where
> > - grp=0/1 means private or shared
> >
> > - res is enum {
> > TRACE_SIGNAL_DELIVERED,
> > TRACE_SIGNAL_IGNORED,
> > TRACE_SIGNAL_ALREADY_PENDING,
> > TRACE_SIGNAL_OVERFLOW_FAIL,
> > TRACE_SIGNAL_LOSE_INFO,
> > };
> >
> > Obviously this is the user visible change. But personally I do
> > agree with Seiji who requested this feature. Currently
> > trace_signal_generate() just records the fact that __send_signal()
> > was called, you can't know whether the signal was actually sent
> > or not.
> >
> > Steven Rostedt says:
> >
> > Adding more to a tracepoint is never an issue, even without a library to
> > parse the data correctly (which we still need in the distros). Thus this
> > change should have no issues.
> >
> >
> >
> > Oleg Nesterov (2):
> > tracing: let trace_signal_generate() report more info, kill overflow_fail/lose_info
> > tracing: send_sigqueue() needs trace_signal_generate() too
> >
> > include/trace/events/signal.h | 85 +++++++++++------------------------------
> > kernel/signal.c | 28 +++++++++----
> > 2 files changed, 41 insertions(+), 72 deletions(-)

I've also Cc:-ed Masami-san who appears to have introduced most
of this trace information.

Looks good to me at a first (quick) sight, except this bit
which changes the ABI:

> > - TP_printk("sig=%d errno=%d code=%d comm=%s pid=%d",
> > + TP_printk("sig=%d errno=%d code=%d comm=%s pid=%d grp=%d res=%d",

That's not how we change tracepoints generally - we add a new
one and eventually phase out the old one. Which apps/tools rely
on the old tracepoint? If it's exactly zero apps then we might
be able to change it, but this needs to be investigated.

Note, it might make sense to send these as two patches to lkml
with me Cc:-ed to avoid any github trust issues, i can apply
them and push them to Linus.

Thanks,

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/