Re: [PATCH 0001/0001] format idle IP output func+offset/length

[Josh Poimboeuf]

On Mon, Nov 06, 2017 at 06:52:03PM +0800, Liu, Changcheng wrote:
> kaslr feature is enabled in kernel.
> Remove kernel text address when dumping idle IP info
> 
> Signed-off-by: Liu Changcheng <changcheng.liu@xxxxxxxxx>
> Signed-off-by: Jerry Liu <primerlink@xxxxxxxxx>
> 
> diff --git a/lib/nmi_backtrace.c b/lib/nmi_backtrace.c
> index 0bc0a35..9cc4178 100644
> --- a/lib/nmi_backtrace.c
> +++ b/lib/nmi_backtrace.c
> @@ -92,7 +92,7 @@ bool nmi_cpu_backtrace(struct pt_regs *regs)
>  	if (cpumask_test_cpu(cpu, to_cpumask(backtrace_mask))) {
>  		arch_spin_lock(&lock);
>  		if (regs && cpu_in_idle(instruction_pointer(regs))) {
> -			pr_warn("NMI backtrace for cpu %d skipped: idling at pc %#lx\n",
> +			pr_warn("NMI backtrace for cpu %d skipped: idling at %pS\n",
>  				cpu, instruction_pointer(regs));
>  		} else {
>  			pr_warn("NMI backtrace for cpu %d\n", cpu);

1) The patch introduces a compile warning.

2) When posting a new version of the patch, it should have a new version
   in the subject, e.g. "PATCH v2".

3) The subject is missing a prefix, like "lib_backtrace: "

4) The subject isn't very clear, how about something like:

   lib_backtrace: fix kernel text address leak

5) The description isn't correct.  KASLR isn't always enabled, it
   depends on the user's config.  But even without KASLR, we don't want
   to leak kernel text addresses.

6) The description should use complete sentences.

7) I'm not sure the "Signed-off-by:" chain is correct.  Was Jerry Liu
   the original author of the patch?

-- 
Josh