linux-kernel - Re: [PATCH] ftrace: using pr_fmt for better printk output

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: <1342496521.8377.105.camel@joe2Laptop>
Date:	Mon, 16 Jul 2012 20:42:01 -0700
From:	Joe Perches <joe@...ches.com>
To:	Jovi Zhang <bookjovi@...il.com>
Cc:	rostedt@...dmis.org,
	Frédéric Weisbecker <fweisbec@...il.com>,
	Ingo Molnar <mingo@...hat.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ftrace: using pr_fmt for better printk output

On Tue, 2012-07-17 at 09:15 +0800, Jovi Zhang wrote:
> >From fe42b2f29e5968482b3129c71f81a58a0559cf04 Mon Sep 17 00:00:00 2001
[]
> There don't have subsystem name output in front ot ftrace related log entry,
> so use pr_fmt to enable better printk output, for output subsystem name in
> log entry.

Hi Jovi.  A few things:

Your patch has 80 column wrapping issues and doesn't
apply cleanly.

This sort of patch, because it's trivial and not really
important to apply this close to an actual release, should
be done against linux-next not current mainline.

The #define pr_fmt(fmt) should probably use KBUILD_MODNAME.
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

Please coalesce formats even though they then may exceed
80 columns and compress multiple lines that fit in 80 too.

> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
[]
> @@ -13,6 +13,8 @@
>   *  Copyright (C) 2004 William Lee Irwin III
>   */
> 
> +#define pr_fmt(fmt) "ftrace: " fmt

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

[]
> @@ -2187,12 +2189,12 @@ static int __init
> ftrace_dyn_table_alloc(unsigned long num_to_init)

wrapped

>  	int cnt;
> 
>  	if (!num_to_init) {
> -		pr_info("ftrace: No functions to be traced?\n");
> +		pr_info("No functions to be traced?\n");
>  		return -1;
>  	}
> 
>  	cnt = num_to_init / ENTRIES_PER_PAGE;
> -	pr_info("ftrace: allocating %ld entries in %d pages\n",
> +	pr_info("allocating %ld entries in %d pages\n",
>  		num_to_init, cnt + 1);

Single line:

	pr_info("allocating %ld entries in %d pages\n", num_to_init, cnt + 1);

> @@ -4495,7 +4497,7 @@ static int start_graph_tracing(void)
>  	if (!ret) {
>  		ret = register_trace_sched_switch(ftrace_graph_probe_sched_switch, NULL);
>  		if (ret)
> -			pr_info("ftrace_graph: Couldn't activate tracepoint"
> +			pr_info("Couldn't activate tracepoint"
>  				" probe to kernel_sched_switch\n");

Coalesce format:

			pr_info("Couldn't activate tracepoint probe to kernel_sched_switch\n");

etc...

cheers, Joe

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