Re: [PATCH] x86/hyperv: Remove unregister syscore call from hyperv cleanup

[Wei Liu <wei.liu@kernel.org>]

On Wed, Nov 23, 2022 at 09:23:11PM -0800, Gaurav Kohli wrote:
> Hyperv cleanup codes comes under panic path where preemption and irq

Please use "Hyper-V" throughout.

> is already disabled. So calling of unregister_syscore_ops which has mutex
> from hyperv cleanup might schedule out the thread and never comes back.
> 

While on paper this looks problematic -- have you seen this issue
triggered in real life?

This looks to be only triggered when there is another thread already
holding the mutex, which seems rather rare in the life cycle of the
machine?

> To prevent the same remove unwanted unregister_syscore_ops function call.
> 
> Signed-off-by: Gaurav Kohli <gauravkohli@linux.microsoft.com>
> 
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index f49bc3ec76e6..c050de69dfde 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -537,7 +537,12 @@ void hyperv_cleanup(void)
>  	union hv_x64_msr_hypercall_contents hypercall_msr;
>  	union hv_reference_tsc_msr tsc_msr;
>  
> -	unregister_syscore_ops(&hv_syscore_ops);
> +	/*
> +	 * Avoid unregister_syscore_ops(&hv_syscore_ops) from cleanup code,
> +	 * as this is only called in crash path where irq and preemption disabled.
> +	 * If we add this, there is a chance that this get scheduled out due to mutex
> +	 * in unregister_syscore_ops and never comes back.
> +	 */

There is no need to document things we don't do, right?

>  
>  	/* Reset our OS id */
>  	wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
> -- 
> 2.17.1
>