linux-kernel - Re: [PATCH 2/2] tracing/events: Add kexec tracepoints

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:	Wed, 09 Sep 2009 11:19:11 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Daniel Walker <dwalker@...o99.com>
Cc:	Li Zefan <lizf@...fujitsu.com>, Ingo Molnar <mingo@...e.hu>,
	Frederic Weisbecker <fweisbec@...il.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Eric Biederman <ebiederm@...ssion.com>
Subject: Re: [PATCH 2/2] tracing/events: Add kexec tracepoints

On Wed, 2009-09-09 at 07:12 -0700, Daniel Walker wrote:
> On Wed, 2009-09-09 at 07:46 -0400, Steven Rostedt wrote:
> > On Tue, 2009-09-08 at 21:59 -0700, Daniel Walker wrote:
> > > On Wed, 2009-09-09 at 09:26 +0800, Li Zefan wrote:
> > > > Daniel Walker wrote:
> > > > > On Wed, 2009-09-09 at 09:15 +0800, Li Zefan wrote:
> > > > >> +       TP_STRUCT__entry(
> > > > >> +               __string(       msg,    msg     )
> > > > >> +       ),
> > > > > 
> > > > > Why the funny spacing here?
> > > > > 
> > > > 
> > > > To make the code better-looking.
> > > > 
> > > > This is the coding-style we use for the code in
> > > > include/trace/events/*.
> > > 
> > > It's part of Linux right? We already have a coding style ..
> > 
> > It's a special macro. What are you now, part of the style police?
> 
> I'm just like everyone else, someone who asks questions and makes
> comments .. By using a different style than what the rest of Linux uses
> your putting yourself at a disadvantage since you can't easily use
> checkpatch on that code (even the stuff that is compliant) ..

I'm fine with questions, but yours sounded a bit more cynical
 
> 
> Not to mention people might start sending you clean ups that you maybe
> won't accept. You could always modify checkpatch to recognize your
> style.

Not sure this is worthy of modifying checkpatch. I haven't looked at
what that would take.

Here's an example of what the code currently looks like:

TRACE_EVENT(sched_wait_task,

	TP_PROTO(struct rq *rq, struct task_struct *p),

	TP_ARGS(rq, p),

	TP_STRUCT__entry(
		__array(	char,	comm,	TASK_COMM_LEN	)
		__field(	pid_t,	pid			)
		__field(	int,	prio			)
	),

	TP_fast_assign(
		memcpy(__entry->comm, p->comm, TASK_COMM_LEN);
		__entry->pid	= p->pid;
		__entry->prio	= p->prio;
	),

	TP_printk("task %s:%d [%d]",
		  __entry->comm, __entry->pid, __entry->prio)
);

And here's that same code with normal Linux style:

TRACE_EVENT(sched_wait_task,
	TP_PROTO(struct rq *rq, struct task_struct *p),
	TP_ARGS(rq, p),
	TP_STRUCT__entry(__array(char, comm, TASK_COMM_LEN)
		__field(pid_t, pid)
		__field(int, prio)),
	TP_fast_assign(memcpy(__entry->comm, p->comm, TASK_COMM_LEN);
		__entry->pid	= p->pid;
		__entry->prio	= p->prio;
	),
	TP_printk("task %s:%d [%d]",
		  __entry->comm, __entry->pid, __entry->prio));

Not as nice to work with. The include/trace/events/ directory is quite
different than the rest of the kernel code. If checkpatch fails on it,
that's fine. Any changes here really need to be examined by eye anyway.
Having the extra spaces actually helps to see what is there.

Note, most changes in include/trace/ftrace.h failed checkpatch
horribly. ;-)  checkpatch does not handle crazy macros well.

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