linux-kernel - Re: [PATCH] new irq tracer

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]

Date:	Fri, 27 Feb 2009 12:08:26 +0900 (JST)
From:	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
To:	Masami Hiramatsu <mhiramat@...hat.com>
Cc:	kosaki.motohiro@...fujitsu.com,
	Mathieu Desnoyers <compudj@...stal.dyndns.org>,
	Jason Baron <jbaron@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	"Frank Ch. Eigler" <fche@...hat.com>, mingo@...e.hu,
	rostedt@...dmis.org, linux-kernel@...r.kernel.org,
	acme@...stprotocols.net, fweisbec@...il.com
Subject: Re: [PATCH] new irq tracer

> >>>>  /**
> >>>>   * handle_IRQ_event - irq action chain handler
> >>>>   * @irq:	the interrupt number
> >>>> @@ -354,7 +358,9 @@ irqreturn_t handle_IRQ_event(unsigned int irq, struct irqaction *action)
> >>>>  		local_irq_enable_in_hardirq();
> >>>>  
> >>>>  	do {
> >>>> +		trace_irq_entry(irq);
> >>>>  		ret = action->handler(irq, action->dev_id);
> >>>> +		trace_irq_exit(irq, ret);
> >>>>  		if (ret == IRQ_HANDLED)
> >>>>  			status |= action->flags;
> >>>>  		retval |= ret;
> >>> Nobdy want unnecessary redundant tracepoint.
> >>> Please discuss with mathieu, and merge his tracepoint.
> >> Hmm, from the viewpoint of trouble shooting, the place of LTTng's tracepoint
> >> is enough. However, from the same viewpoint, it should pass irq-number
> >> to irq-exit event too, because we may lost some previous events by buffer-overflow
> >> etc.
> >>
> >>          trace_irq_entry(irq, NULL);
> >>          ret = _handle_IRQ_event(irq, action);
> >>          trace_irq_exit(irq, ret);
> >>                         ^^^^
> >>
> > 
> > I seriously doubt we should consider a trace with missing events as
> > "reliable". If your only argument is that when the buffers are not large
> > enough we could lose events, then I think we should just hint people at
> > doing the right thing, which is to tweak the tracer parameters (e.g.
> > larger buffers) so they stop losing events.
> 
> Hi Mathieu,
> 
> I think it depends on what events we'd like to trace on what environment
> (not have so much memories).
> Someone(like me) may want "reliability" for those events, someone not.

I don't think this viewpoint is valueable.
buffer overflow problem depend on the number of AVERAGE output from tracer.
typically, one irq has only one irq handler. 

if this change makes buffer overflow, the tracer buffer management is too silly.
Just change it.



> > A trace with events lost is really a scenario close to a corrupted
> > trace because we don't know which event has been lost, nor where. I
> > don't think we should increase the event size to support that kind of
> > broken scenario.
> 
> Why LTTng doesn't filter those unused arguments inside their handlers?
> Is there no room for compromise on sharing tracepoints with other
> tracers? IMHO, tracepoints in the kernel should be a common
> infrastructure for many users.

Agreed. last mathieu's mail only talk about lttng issue.
I can imazine systemtap folks need explicit irq number argument.


--
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/