Re: [PATCH v14 3/7] crash: add generic infrastructure for crash hotplug support

[Baoquan He]

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.