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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20090601143626.GA26981@infradead.org>
Date:	Mon, 1 Jun 2009 10:36:26 -0400
From:	Christoph Hellwig <hch@...radead.org>
To:	Li Zefan <lizf@...fujitsu.com>
Cc:	Steven Rostedt <rostedt@...dmis.org>,
	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, 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)

>   - 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 :)

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