From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754204AbdKFQsr (ORCPT ); Mon, 6 Nov 2017 11:48:47 -0500 Received: from mga11.intel.com ([192.55.52.93]:61527 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752977AbdKFQsq (ORCPT ); Mon, 6 Nov 2017 11:48:46 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,353,1505804400"; d="scan'208";a="918148994" Date: Tue, 7 Nov 2017 00:48:47 +0800 From: "Liu, Changcheng" To: Josh Poimboeuf , Petr Mladek , changcheng.liu@intel.com Cc: sergey.senozhatsky.work@gmail.com, Andrew Morton , linux-kernel@vger.kernel.org Subject: Re: [PATCH 0001/0001] format idle IP output func+offset/length Message-ID: <20171106164847.GD69364@sofia> References: <20171106052511.GB69364@sofia> <20171106080528.GA1298@jagdpanzerIV> <20171106105203.GC69364@sofia> <20171106131147.ouihd4ug5ah3vb6a@treble> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171106131147.ouihd4ug5ah3vb6a@treble> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > Signed-off-by: Jerry Liu > > > > 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