linux-kernel - Re: [PATCH] tracing/irq: use softirq_to_name instead of __print

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 for Android: free password hash cracker in your pocket

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

Message-ID: <alpine.DEB.2.00.0906011045460.14994@gandalf.stny.rr.com>
Date:	Mon, 1 Jun 2009 11:06:17 -0400 (EDT)
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Christoph Hellwig <hch@...radead.org>
cc:	Li Zefan <lizf@...fujitsu.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Ingo Molnar <mingo@...e.hu>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] tracing/irq: use softirq_to_name instead of
 __print_symbolic


On Mon, 1 Jun 2009, Christoph Hellwig wrote:

> On Mon, Jun 01, 2009 at 04:52:23PM +0800, Li Zefan wrote:
> > It's great to boost recording of softirq events, but why not simply
> > use softirq_to_name in TP_printk()?
> 
> Because userspace programs using the binary trace buffer have no chance to
> retrieve the values from softirq_to_name.
> 
> > The above commit has 2 flaws:
> > 
> >   - we waste memory defining local static struct trace_print_flags array
> >     in each softirq TRACE_EVENT
> 
> That could be solved by declaring the array manually and just passing
> the address to __print_symbolic.  Steve, would that work?  (also for
> __print_flags)

I added the code to make it an array, but we still need to allocate it for 
every trace call. Otherwise, how do we export the information to 
userspace. One trace event (currently) has no way of depending on another 
trace event. If two trace events want the same array, currently they both 
must allocate it. I haven't looked too deeply into seeing how to manage 
that. It's not that much memory duplicated. I hate to make the code even 
more complex just to save a couple of hundred of bytes.

> 
> >   - if someone adds/removes a softirq, he may not know show_softirq_name()
> >     needs to be updated
> 
> Just make sure you have the translation defined next to the actual
> flags.  This is what I do in the XFS tracer:
> 
> typedef enum xfs_alloctype
> {
> 	XFS_ALLOCTYPE_ANY_AG,           /* allocate anywhere, use rotor */
> 	XFS_ALLOCTYPE_FIRST_AG,         /* ... start at ag 0 */
> 	XFS_ALLOCTYPE_START_AG,         /* anywhere, start in this a.g. */
> 	XFS_ALLOCTYPE_THIS_AG,          /* anywhere in this a.g. */
> 	XFS_ALLOCTYPE_START_BNO,	/* near this block else anywhere */
> 	XFS_ALLOCTYPE_NEAR_BNO,		/* in this a.g. and near this block */
> 	XFS_ALLOCTYPE_THIS_BNO		/* at exactly this block */
> } xfs_alloctype_t;
> 
> #define XFS_ALLOC_TYPES \
> 	{ XFS_ALLOCTYPE_ANY_AG,         "ANY_AG" }, \
> 	{ XFS_ALLOCTYPE_FIRST_AG,       "FIRST_AG" }, \
> 	{ XFS_ALLOCTYPE_START_AG,       "START_AG" }, \
> 	{ XFS_ALLOCTYPE_THIS_AG,	"THIS_AG" }, \
> 	{ XFS_ALLOCTYPE_START_BNO,	"START_BNO" }, \
> 	{ XFS_ALLOCTYPE_NEAR_BNO,	"NEAR_BNO" }, \
> 	{ XFS_ALLOCTYPE_THIS_BNO,	"THIS_BNO" }
> 
> > Another issue with the above commit, that the output of softirq events
> > becomes:
> > 
> >   X-1701  [000]  1595.220739: softirq_entry: softirq=1 action=TIMER_SOFTIRQ
> > 
> > Compared to the original output:
> > 
> >   X-1701  [000]  1595.220739: softirq_entry: softirq=1 action=TIMER
> 
> Which is trivially fixable, see above :)

Hmm, I thought I fixed that already. Perhaps it is in the code that Ingo 
has not pulled.

-- Steve

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