All of lore.kernel.org
 help / color / mirror / Atom feed
From: Baoquan He <bhe@redhat.com>
To: Eric DeVolder <eric.devolder@oracle.com>
Cc: linux-kernel@vger.kernel.org, x86@kernel.org,
	kexec@lists.infradead.org, ebiederm@xmission.com,
	dyoung@redhat.com, vgoyal@redhat.com, tglx@linutronix.de,
	mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
	hpa@zytor.com, nramas@linux.microsoft.com,
	thomas.lendacky@amd.com, robh@kernel.org, efault@gmx.de,
	rppt@kernel.org, david@redhat.com, sourabhjain@linux.ibm.com,
	konrad.wilk@oracle.com, boris.ostrovsky@oracle.com
Subject: Re: [PATCH v10 1/8] crash: introduce arch/*/asm/crash.h
Date: Fri, 12 Aug 2022 17:46:09 +0800	[thread overview]
Message-ID: <YvYhYdV6F2zH4o3X@MiWiFi-R3L-srv> (raw)
In-Reply-To: <52d40562-ee6f-bb89-6d21-2d6baf67053d@oracle.com>

On 08/08/22 at 10:18am, Eric DeVolder wrote:
> 
> 
> On 8/7/22 22:25, Baoquan He wrote:
> > Hi Eric,
> > 
> > On 07/21/22 at 02:17pm, Eric DeVolder wrote:
> > > The use of __weak is being eliminated within kexec sources.
> > > The technique uses macros mapped onto inline functions in
> > > order to replace __weak.
> > > 
> > > This patchset was using __weak and so in order to replace
> > > __weak, this patch introduces arch/*/asm/crash.h, patterned
> > > after how kexec is moving away from __weak and to the macro
> > > definitions.
> > 
> > Are you going to replace __weak in kexec of arll ARCHes? I don't see
> > your point why all these empty header files are introduced. Wondering
> > what's impacted if not adding these empty files?
> 
> Hi Baoquan,
> In this patchset, to file include/linux/crash_core.h I added the line #include <asm/crash.h>.
> I patterned this after how include/linux/kexec.h does #include <asm/kexec.h>.

I am sorry, Eric, it looks not so good. I understand you want to pattern
asm/kexe.h, but we need consider reality. Introducing a dozen of empty
header file and not being able to tell when they will be filled doesn't
make sense.

Includig <asm/crash.h> where needed is much simpler. I doubt if your way
can pass other reviewers' line. Can you reconsider?

diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 77f5f3591760..b0577bdcc491 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -15,6 +15,7 @@
 
 #include <asm/page.h>
 #include <asm/sections.h>
+#include <asm/crash.h>
 
 #include <crypto/sha1.h>


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

WARNING: multiple messages have this Message-ID (diff)
From: Baoquan He <bhe@redhat.com>
To: Eric DeVolder <eric.devolder@oracle.com>
Cc: linux-kernel@vger.kernel.org, x86@kernel.org,
	kexec@lists.infradead.org, ebiederm@xmission.com,
	dyoung@redhat.com, vgoyal@redhat.com, tglx@linutronix.de,
	mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
	hpa@zytor.com, nramas@linux.microsoft.com,
	thomas.lendacky@amd.com, robh@kernel.org, efault@gmx.de,
	rppt@kernel.org, david@redhat.com, sourabhjain@linux.ibm.com,
	konrad.wilk@oracle.com, boris.ostrovsky@oracle.com
Subject: Re: [PATCH v10 1/8] crash: introduce arch/*/asm/crash.h
Date: Fri, 12 Aug 2022 17:46:09 +0800	[thread overview]
Message-ID: <YvYhYdV6F2zH4o3X@MiWiFi-R3L-srv> (raw)
In-Reply-To: <52d40562-ee6f-bb89-6d21-2d6baf67053d@oracle.com>

On 08/08/22 at 10:18am, Eric DeVolder wrote:
> 
> 
> On 8/7/22 22:25, Baoquan He wrote:
> > Hi Eric,
> > 
> > On 07/21/22 at 02:17pm, Eric DeVolder wrote:
> > > The use of __weak is being eliminated within kexec sources.
> > > The technique uses macros mapped onto inline functions in
> > > order to replace __weak.
> > > 
> > > This patchset was using __weak and so in order to replace
> > > __weak, this patch introduces arch/*/asm/crash.h, patterned
> > > after how kexec is moving away from __weak and to the macro
> > > definitions.
> > 
> > Are you going to replace __weak in kexec of arll ARCHes? I don't see
> > your point why all these empty header files are introduced. Wondering
> > what's impacted if not adding these empty files?
> 
> Hi Baoquan,
> In this patchset, to file include/linux/crash_core.h I added the line #include <asm/crash.h>.
> I patterned this after how include/linux/kexec.h does #include <asm/kexec.h>.

I am sorry, Eric, it looks not so good. I understand you want to pattern
asm/kexe.h, but we need consider reality. Introducing a dozen of empty
header file and not being able to tell when they will be filled doesn't
make sense.

Includig <asm/crash.h> where needed is much simpler. I doubt if your way
can pass other reviewers' line. Can you reconsider?

diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 77f5f3591760..b0577bdcc491 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -15,6 +15,7 @@
 
 #include <asm/page.h>
 #include <asm/sections.h>
+#include <asm/crash.h>
 
 #include <crypto/sha1.h>


  parent reply	other threads:[~2022-08-12  9:46 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-21 18:17 [PATCH v10 0/8] crash: Kernel handling of CPU and memory hot un/plug Eric DeVolder
2022-07-21 18:17 ` Eric DeVolder
2022-07-21 18:17 ` [PATCH v10 1/8] crash: introduce arch/*/asm/crash.h Eric DeVolder
2022-07-21 18:17   ` Eric DeVolder
2022-08-08  3:25   ` Baoquan He
2022-08-08  3:25     ` Baoquan He
2022-08-08 15:18     ` Eric DeVolder
2022-08-08 15:18       ` Eric DeVolder
2022-08-08 15:44       ` Rob Herring
2022-08-08 15:44         ` Rob Herring
2022-08-12  9:46       ` Baoquan He [this message]
2022-08-12  9:46         ` Baoquan He
2022-08-12 21:23         ` Eric DeVolder
2022-08-12 21:23           ` Eric DeVolder
2022-08-13  0:24           ` Baoquan He
2022-08-13  0:24             ` Baoquan He
2022-08-16 15:23             ` Eric DeVolder
2022-08-16 15:23               ` Eric DeVolder
2022-07-21 18:17 ` [PATCH v10 2/8] crash: move crash_prepare_elf64_headers Eric DeVolder
2022-07-21 18:17   ` Eric DeVolder
2022-07-21 18:17 ` [PATCH v10 3/8] crash: prototype change for crash_prepare_elf64_headers Eric DeVolder
2022-07-21 18:17   ` Eric DeVolder
2022-07-21 18:17 ` [PATCH v10 4/8] crash: add generic infrastructure for crash hotplug support Eric DeVolder
2022-07-21 18:17   ` Eric DeVolder
2022-08-08  9:30   ` Baoquan He
2022-08-08  9:30     ` Baoquan He
2022-08-08 15:20     ` Eric DeVolder
2022-08-08 15:20       ` Eric DeVolder
2022-07-21 18:17 ` [PATCH v10 5/8] kexec: exclude elfcorehdr from the segment digest Eric DeVolder
2022-07-21 18:17   ` Eric DeVolder
2022-07-21 18:17 ` [PATCH v10 6/8] kexec: exclude hot remove cpu from elfcorehdr notes Eric DeVolder
2022-07-21 18:17   ` Eric DeVolder
2022-07-21 18:17 ` [PATCH v10 7/8] crash: memory and cpu hotplug sysfs attributes Eric DeVolder
2022-07-21 18:17   ` Eric DeVolder
2022-08-08 10:41   ` Baoquan He
2022-08-08 10:41     ` Baoquan He
2022-08-16 15:24     ` Eric DeVolder
2022-08-16 15:24       ` Eric DeVolder
2022-07-21 18:17 ` [PATCH v10 8/8] x86/crash: Add x86 crash hotplug support Eric DeVolder
2022-07-21 18:17   ` Eric DeVolder
2022-08-13  0:34   ` Baoquan He
2022-08-13  0:34     ` Baoquan He
2022-08-16 15:23     ` Eric DeVolder
2022-08-16 15:23       ` Eric DeVolder
2022-08-25 19:42       ` Eric DeVolder
2022-08-25 19:42         ` Eric DeVolder
2022-08-26  4:35       ` Baoquan He
2022-08-26  4:35         ` Baoquan He
2022-08-04 14:42 ` [PATCH v10 0/8] crash: Kernel handling of CPU and memory hot un/plug Eric DeVolder
2022-08-04 14:42   ` Eric DeVolder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YvYhYdV6F2zH4o3X@MiWiFi-R3L-srv \
    --to=bhe@redhat.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=david@redhat.com \
    --cc=dyoung@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=efault@gmx.de \
    --cc=eric.devolder@oracle.com \
    --cc=hpa@zytor.com \
    --cc=kexec@lists.infradead.org \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=nramas@linux.microsoft.com \
    --cc=robh@kernel.org \
    --cc=rppt@kernel.org \
    --cc=sourabhjain@linux.ibm.com \
    --cc=tglx@linutronix.de \
    --cc=thomas.lendacky@amd.com \
    --cc=vgoyal@redhat.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.