Messages in this thread | | | Date | Tue, 7 Nov 2017 00:48:47 +0800 | From | "Liu, Changcheng" <> | Subject | Re: [PATCH 0001/0001] format idle IP output func+offset/length |
| |
Thx Josh. I'll refine the patch. Answer question as below.
On 07:11 Mon 06 Nov, Josh Poimboeuf wrote: > 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@intel.com> > > Signed-off-by: Jerry Liu <primerlink@gmail.com> > > > > 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. I've got the point because the conversion between pointer and unsigned long. > > 2) When posting a new version of the patch, it should have a new version > in the subject, e.g. "PATCH v2". Thx for your guide. I'm on the way to work with community. > > 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 I'll use this subject. > > 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. That's fine. > > 6) The description should use complete sentences. Yes. > > 7) I'm not sure the "Signed-off-by:" chain is correct. Was Jerry Liu > the original author of the patch? This is my personal bad habit. I'll correct it. > > -- > Josh
| |