Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1220346 > unrolled thread

[PATCH] hyperv: fix build if KEXEC not enabled

Back to article view | Back to linux.kernel

Contents

  [PATCH] hyperv: fix build if KEXEC not enabled Stephen Hemminger <stephen@networkplumber.org> - 2015-09-07 19:30 +0200
    Re: [PATCH] hyperv: fix build if KEXEC not enabled Ingo Molnar <mingo@kernel.org> - 2015-09-08 09:20 +0200
      Re: [PATCH] hyperv: fix build if KEXEC not enabled Vitaly Kuznetsov <vkuznets@redhat.com> - 2015-09-08 09:40 +0200
        Re: [PATCH] hyperv: fix build if KEXEC not enabled Ingo Molnar <mingo@kernel.org> - 2015-09-08 10:00 +0200
          Re: [PATCH] hyperv: fix build if KEXEC not enabled Vitaly Kuznetsov <vkuznets@redhat.com> - 2015-09-08 23:40 +0200
    Re: [PATCH] hyperv: fix build if KEXEC not enabled Vitaly Kuznetsov <vkuznets@redhat.com> - 2015-09-08 09:30 +0200
      Re: [PATCH] hyperv: fix build if KEXEC not enabled Ingo Molnar <mingo@kernel.org> - 2015-09-08 09:40 +0200

#1220346 — [PATCH] hyperv: fix build if KEXEC not enabled

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 <vkuznets@redhat.com>
   Date:   Sat Aug 1 16:08:09 2015 -0700

       Drivers: hv: vmbus: add special crash handler


Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>


--- 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
 }
 
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1220535

* Stephen Hemminger <stephen@networkplumber.org> 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 <vkuznets@redhat.com>
>    Date:   Sat Aug 1 16:08:09 2015 -0700
> 
>        Drivers: hv: vmbus: add special crash handler
> 
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> 
> 
> --- 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.

These bugs came upstream via the driver tree:

 b4370df2b1f5 ("Drivers: hv: vmbus: add special crash handler")
 2517281d63a2 ("Drivers: hv: vmbus: add special kexec handler")

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1220555

Ingo Molnar <mingo@kernel.org> writes:

> * Stephen Hemminger <stephen@networkplumber.org> 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 <vkuznets@redhat.com>
>>    Date:   Sat Aug 1 16:08:09 2015 -0700
>> 
>>        Drivers: hv: vmbus: add special crash handler
>> 
>> 
>> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>> 
>> 
>> --- 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? crash_shutdown is defined in machine_ops unconditionally, I don't
see why we _need_ #ifdef here (and btw Greg insisted on removing them).

> These bugs came upstream via the driver tree:
>
>  b4370df2b1f5 ("Drivers: hv: vmbus: add special crash handler")
>  2517281d63a2 ("Drivers: hv: vmbus: add special kexec handler")
>
> Thanks,
>
> 	Ingo

-- 
  Vitaly
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1220560

* Vitaly Kuznetsov <vkuznets@redhat.com> wrote:

> Ingo Molnar <mingo@kernel.org> writes:
> 
> > * Stephen Hemminger <stephen@networkplumber.org> 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 <vkuznets@redhat.com>
> >>    Date:   Sat Aug 1 16:08:09 2015 -0700
> >> 
> >>        Drivers: hv: vmbus: add special crash handler
> >> 
> >> 
> >> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> >> 
> >> 
> >> --- 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1221092

Ingo Molnar <mingo@kernel.org> writes:

> * Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
>> Ingo Molnar <mingo@kernel.org> writes:
>> 
>> > * Stephen Hemminger <stephen@networkplumber.org> 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 <vkuznets@redhat.com>
>> >>    Date:   Sat Aug 1 16:08:09 2015 -0700
>> >> 
>> >>        Drivers: hv: vmbus: add special crash handler
>> >> 
>> >> 
>> >> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>> >> 
>> >> 
>> >> --- 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.

Greg,

I don't personally have a strong opinion here but I'm leaning towards
Ingo's suggestion. That would mean I have to break your 'no #ifdefs in C
files' rule (ok, I can move everything to mshyperv.h to respect it but
that would feel like a fraud). Please let me know if you prefer such fix
to the previously posted one (https://lkml.org/lkml/2015/8/11/417).

Thanks,

>
> 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

-- 
  Vitaly
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1220540

Stephen Hemminger <stephen@networkplumber.org> writes:

> 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 <vkuznets@redhat.com>
>    Date:   Sat Aug 1 16:08:09 2015 -0700
>
>        Drivers: hv: vmbus: add special crash handler
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
>
> --- 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
>  }

Greg in particular was against #ifdefs in C code and I sent the
following patch to fix the issue:

https://lkml.org/lkml/2015/8/11/417

-- 
  Vitaly
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1220554

* Vitaly Kuznetsov <vkuznets@redhat.com> wrote:

> Stephen Hemminger <stephen@networkplumber.org> writes:
> 
> > 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 <vkuznets@redhat.com>
> >    Date:   Sat Aug 1 16:08:09 2015 -0700
> >
> >        Drivers: hv: vmbus: add special crash handler
> >
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> >
> > --- 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
> >  }
> 
> Greg in particular was against #ifdefs in C code and I sent the
> following patch to fix the issue:
> 
> https://lkml.org/lkml/2015/8/11/417

Note that this is still somewhat buggy, the whole hv_machine_crash_shutdown() 
should be conditional - like in kvmclock.c.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web