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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120116074540.GE15641@elte.hu>
Date:	Mon, 16 Jan 2012 08:45:40 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Ingo Molnar <mingo@...hat.com>,
	Masami Hiramatsu <mhiramat@...hat.com>,
	Seiji Aguchi <saguchi@...hat.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	linux-kernel@...r.kernel.org,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
Subject: Re: [GIT PULL] tracing: make signal tracepoints more useful


* Oleg Nesterov <oleg@...hat.com> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/