Messages in this thread | | | Date | Wed, 9 Nov 2022 16:12:24 -0600 | Subject | Re: [PATCH v13 7/7] x86/crash: add x86 crash hotplug support | From | Eric DeVolder <> |
| |
On 11/9/22 15:31, Borislav Petkov wrote: > On Wed, Nov 09, 2022 at 09:48:33AM -0600, Eric DeVolder wrote: >> ... >> which then defaults HOTPLUG_CPU to on and thus this code/ifdef in question. > > defconfig can sometimes lag reality. In this case, the majority of > machines have SMP=y because the majority of machines out there are, > well, multicore. > >> So at this point, I'm still not sure if you want the ifdef line: >> - removed altogether >> - transitioned to CRASH_HOTPLUG >> - leave as is > > So let's think out loud: > > * the majority of machines will have CONFIG_HOTPLUG_CPU=y because > they're SMP machines and we want the elfcorehdr updates to happen when > CPUs get offlined or onlined. > > CONFIG_MEMORY_HOTPLUG is most likely going to be =n on the majority of > machines out there. > > (Note how the deciding factor for all this is what would make sense on > the prevailing majority of machines out there.) > > And memory hotplug will be off for the simple reason that not so many > machines have memory hotplug hardware capability. > > Which then means, IMHO, this functionality should be separate: have a > CPU hotplug callback and a memory hotplug callback. > > And you kinda do that in > > Subject: [PATCH v13 3/7] crash: add generic infrastructure for crash hotplug support > > but then this all calls into a single handle_hotplug_event() and that > hp_action doesn't really matter. > > It is used in the call to > > arch_crash_handle_hotplug_event(image, hp_action); > > but that hp_action argument is unused in the x86 version. > > IOW, you can do this callback regardless whether it is a CPU or memory > hotplug event. > > So thinking about it, a single CONFIG_CRASH_HOTPLUG which unifies those > CPU and memory hotplug callback functionality makes most sense to me. > Because you don't really differentiate between the two in the callback > actions. > > Anyway, this is how I see it from here. I could very well be missing an > aspect, of course. > > Thx. > OK, I'll put in CRASH_HOTPLUG! Expect v14 soon! Thank you! eric
| |