linux-kernel - Re: [PATCH v2 3/3] ftrace: add tracepoint for itimer

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:	Tue, 7 Jul 2009 10:06:00 -0400
From:	Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	Xiao Guangrong <xiaoguangrong@...fujitsu.com>,
	Ingo Molnar <mingo@...e.hu>,
	Steven Rostedt <rostedt@...dmis.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Zhaolei <zhaolei@...fujitsu.com>, kosaki.motohiro@...fujitsu.com,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 3/3] ftrace: add tracepoint for itimer

* Thomas Gleixner (tglx@...utronix.de) wrote:
> On Mon, 6 Jul 2009, Mathieu Desnoyers wrote:
> > * Xiao Guangrong (xiaoguangrong@...fujitsu.com) wrote:
> > > +
> > > +TRACE_EVENT(itimer_state,
> > > +
> > 
> > Hrm, I'd reserve the use of the *_state namespace for something
> > different if possible.
> 
> See below.
> 
> > In LTTng, we have the ability to list a set of data structures at trace
> > start (we call this state dump). In this case, we could add a list of
> > all active timers at trace start. This is useful if the trace started
> > after timer inits.
> 
> Can you please spare us the gory details of LTTng. We already know
> that it can do everything plus brewing coffee, so no need to advertise
> it in every tracing related mail.
> 
> > So having "_state" without much meaning here might eventually be
> > confusing.
> > 
> > "_action" or "_call" (like IPCs) might be more appropriate ?
> 
> Err, where is this an action or a call ? Care to look at the call site
> of this tracepoint. It's at the end of do_setitimer() and records the
> _state_ of the itimer _after_ the syscall was handled.
> 

Hrm, right. The patch context misled me. I thought it was at the end of
it_real_fn(), but it's actually in do_setitimer().

OK, looking at it from it's actual context, it makes perfect sense to
name it that way.

And it could even be used as-is to iterate on all timers at trace start
as we do in this tracer I won't name. ;-)

BTW, I was merely pointing out the risk of a name clash here. And I
had to refer to the tree where this piece of code exists to support the
point. My intent was not to advertise anything.

> > Mathieu
> 
> Can you please remove the hundred lines you are not commenting on next
> time ?
>  

OK,

Thanks,

Mathieu

> > > +	TP_PROTO(int which, struct itimerval *value, cputime_t expires),
> 
> Thanks,
> 
> 	tglx

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
--
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/