[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y4VVm5gVWqa0GkbY@MiWiFi-R3L-srv>
Date: Tue, 29 Nov 2022 08:43:07 +0800
From: Baoquan He <bhe@...hat.com>
To: Eric DeVolder <eric.devolder@...cle.com>
Cc: linux-kernel@...r.kernel.org, x86@...nel.org,
kexec@...ts.infradead.org, ebiederm@...ssion.com,
dyoung@...hat.com, vgoyal@...hat.com, tglx@...utronix.de,
mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
hpa@...or.com, nramas@...ux.microsoft.com, thomas.lendacky@....com,
robh@...nel.org, efault@....de, rppt@...nel.org, david@...hat.com,
sourabhjain@...ux.ibm.com, konrad.wilk@...cle.com,
boris.ostrovsky@...cle.com
Subject: Re: [PATCH v14 3/7] crash: add generic infrastructure for crash
hotplug support
On 11/28/22 at 09:46am, Eric DeVolder wrote:
>
>
> On 11/24/22 21:26, Baoquan He wrote:
> > On 11/16/22 at 04:46pm, Eric DeVolder wrote:
> > ......
> > > diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> > > index ebf46c3b8f8b..b4dbc21f9081 100644
> > > --- a/include/linux/kexec.h
> > > +++ b/include/linux/kexec.h
> > > @@ -32,6 +32,7 @@ extern note_buf_t __percpu *crash_notes;
> > > #include <linux/compat.h>
> > > #include <linux/ioport.h>
> > > #include <linux/module.h>
> > > +#include <linux/highmem.h>
> > > #include <asm/kexec.h>
> > > /* Verify architecture specific macros are defined */
> > > @@ -374,6 +375,13 @@ struct kimage {
> > > struct purgatory_info purgatory_info;
> > > #endif
> > > +#ifdef CONFIG_CRASH_HOTPLUG
> >
> > This kernel config CRASH_HOTPLUG is added in patch 7, but we have used
> > it in the previous patch, not sure if this is acceptable.
> >
> I wasn't sure what to do here either. Patch 7 is the x86 arch-specific
> support patch, and CRASH_HOTPLUG is introduced in arch/x86/Kconfig. I did
> look at introducing CRASH_HOTPLUG as a generic/non-arch-specific option, but
> no location seemed appropriate given HOTPLUG_CPU is arch-specific and
> MEMORY_HOTPLUG is in mm/Kconfig.
arch/Kconfig?
Because CRASH_CORE/KEXEC_CORE are defined there.
>
> This doesn't break bisect, but as you point out, not sure if the location in patch 7 is acceptable.
> I'm not really sure how to resolve the question.
Hmm, since it's bisect-able, seems doesn't break rule. I could be too
sensitive. Do we have a precendent like this, to strengthen our
confidence?
If no concern from other people, it's also fine to me.
Powered by blists - more mailing lists