From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753045AbbIHH5e (ORCPT ); Tue, 8 Sep 2015 03:57:34 -0400 Received: from mail-wi0-f172.google.com ([209.85.212.172]:36388 "EHLO mail-wi0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751803AbbIHH5c (ORCPT ); Tue, 8 Sep 2015 03:57:32 -0400 Date: Tue, 8 Sep 2015 09:57:28 +0200 From: Ingo Molnar To: Vitaly Kuznetsov Cc: Stephen Hemminger , "K. Y. Srinivasan" , Haiyang Zhang , Greg Kroah-Hartman , x86@kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] hyperv: fix build if KEXEC not enabled Message-ID: <20150908075727.GA7480@gmail.com> References: <20150907102222.27d0477b@urahara> <20150908071458.GA5291@gmail.com> <87a8sxbc4m.fsf@vitty.brq.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87a8sxbc4m.fsf@vitty.brq.redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Vitaly Kuznetsov wrote: > Ingo Molnar writes: > > > * Stephen Hemminger wrote: > > > >> Fixes regression 4.3 mergw window in my config > >> where hyperv is enable but CONFIG_KEXEC not enabled. > >> > >> arch/x86/kernel/cpu/mshyperv.c:112: undefined reference to `native_machine_crash_shutdown' > >> > >> Introduced by: > >> commit b4370df2b1f5158de028e167974263c5757b34a6 > >> Author: Vitaly Kuznetsov > >> Date: Sat Aug 1 16:08:09 2015 -0700 > >> > >> Drivers: hv: vmbus: add special crash handler > >> > >> > >> Signed-off-by: Stephen Hemminger > >> > >> > >> --- a/arch/x86/kernel/cpu/mshyperv.c 2015-09-07 10:11:24.994885115 -0700 > >> +++ b/arch/x86/kernel/cpu/mshyperv.c 2015-09-07 10:14:20.995698615 -0700 > >> @@ -109,7 +109,9 @@ static void hv_machine_crash_shutdown(st > >> { > >> if (hv_crash_handler) > >> hv_crash_handler(regs); > >> +#ifdef CONFIG_KEXEC > >> native_machine_crash_shutdown(regs); > >> +#endif > > > > I think there's another related bug as well: > > > > machine_ops.crash_shutdown = hv_machine_crash_shutdown; > > > > that should be #ifdef CONFIG_KEXEC as well AFAICS. > > > > Why? [...] Because you are bloating the kernel. That's because machine_ops.crash_shutdown() does nothing outside of kexec and that's the existing pattern in the native and KVM code. (Xen does it inconsistently as well.) So you bloat the kernel at minimum, and also confuse the reader what it's all about. > [...] crash_shutdown is defined in machine_ops unconditionally, I don't see why > we _need_ #ifdef here (and btw Greg insisted on removing them). So arguably the kexec interface should be cleaned up as well, into something like: kexec_crash_handler_set(hv_machine_crash_shutdown); ... which would compile to no code at all in the !KEXEC case, and then we could also make ::crash_shutdown #ifdef KEXEC. At least one #ifdef is unavoidable unless we make KCONFIG an always-enabled facility - or merge it more intelligently with the regular reboot/shutdown code. I.e. I don't think there should be kexec specific 'handlers' per se - there should be reboot/shutdown handlers that will also serve kexec just fine. But until that's fixed we've got to make the best of the existing kexec design. Thanks, Ingo