Re: [PATCH v8 0/2] x86: vmclear vmcss on all cpus when doing kdump if necessary

Re: [PATCH v8 0/2] x86: vmclear vmcss on all cpus when doing kdump if necessary

[Gleb Natapov]

On Thu, Nov 22, 2012 at 04:22:26PM +0800, Zhang Yanfei wrote:
> Currently, kdump just makes all the logical processors leave VMX operation by
> executing VMXOFF instruction, so any VMCSs active on the logical processors may
> be corrupted. But, sometimes, we need the VMCSs to debug guest images contained
> in the host vmcore. To prevent the corruption, we should VMCLEAR the VMCSs before
> executing the VMXOFF instruction.
> 
> The patch set provides a way to VMCLEAR vmcss related to guests on all cpus before
> executing the VMXOFF when doing kdump. This is used to ensure the VMCSs in the
> vmcore updated and non-corrupted.
> 
Looks fine to me, but kexec part should get acks from exec maintainers.

> Changelog from v7 to v8:
> 1. KEXEC: regression for using name crash_notifier_list
>    and remove comments related to KVM
>    and just call function atomic_notifier_call_chain directly.
> 
> Changelog from v6 to v7:
> 1. KVM-INTEL: in hardware_disable, we needn't disable the
>    vmclear, so remove it.
> 
> Changelog from v5 to v6:
> 1. KEXEC: the atomic notifier list renamed:
>    crash_notifier_list --> vmclear_notifier_list
> 2. KVM-INTEL: provide empty functions if CONFIG_KEXEC is
>    not defined and remove unnecessary #ifdef's.
> 
> Changelog from v4 to v5:
> 1. use an atomic notifier instead of function call, so
>    have all the vmclear codes in vmx.c.
> 
> Changelog from v3 to v4:
> 1. add a new percpu variable vmclear_skipped to skip
>    vmclear in kdump in some conditions.
> 
> Changelog from v2 to v3:
> 1. remove unnecessary conditions in function
>    cpu_emergency_clear_loaded_vmcss as Marcelo suggested.
> 
> Changelog from v1 to v2:
> 1. remove the sysctl and clear VMCSs unconditionally.
> 
> Zhang Yanfei (2):
>   x86/kexec: add a new atomic notifier list for kdump
>   KVM-INTEL: add a notifier and a bitmap to support VMCLEAR in kdump
> 
>  arch/x86/include/asm/kexec.h |    2 +
>  arch/x86/kernel/crash.c      |    9 +++++
>  arch/x86/kvm/vmx.c           |   70 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 81 insertions(+), 0 deletions(-)

--
			Gleb.

Re: [PATCH v8 0/2] x86: vmclear vmcss on all cpus when doing kdump if necessary

[Zhang Yanfei]

于 2012年11月25日 22:26, Gleb Natapov 写道:
> On Thu, Nov 22, 2012 at 04:22:26PM +0800, Zhang Yanfei wrote:
>> Currently, kdump just makes all the logical processors leave VMX operation by
>> executing VMXOFF instruction, so any VMCSs active on the logical processors may
>> be corrupted. But, sometimes, we need the VMCSs to debug guest images contained
>> in the host vmcore. To prevent the corruption, we should VMCLEAR the VMCSs before
>> executing the VMXOFF instruction.
>>
>> The patch set provides a way to VMCLEAR vmcss related to guests on all cpus before
>> executing the VMXOFF when doing kdump. This is used to ensure the VMCSs in the
>> vmcore updated and non-corrupted.
>>
> Looks fine to me, but kexec part should get acks from exec maintainers.

Thanks. I will ask the exec maintainer.

> 
>> Changelog from v7 to v8:
>> 1. KEXEC: regression for using name crash_notifier_list
>>    and remove comments related to KVM
>>    and just call function atomic_notifier_call_chain directly.
>>
>> Changelog from v6 to v7:
>> 1. KVM-INTEL: in hardware_disable, we needn't disable the
>>    vmclear, so remove it.
>>
>> Changelog from v5 to v6:
>> 1. KEXEC: the atomic notifier list renamed:
>>    crash_notifier_list --> vmclear_notifier_list
>> 2. KVM-INTEL: provide empty functions if CONFIG_KEXEC is
>>    not defined and remove unnecessary #ifdef's.
>>
>> Changelog from v4 to v5:
>> 1. use an atomic notifier instead of function call, so
>>    have all the vmclear codes in vmx.c.
>>
>> Changelog from v3 to v4:
>> 1. add a new percpu variable vmclear_skipped to skip
>>    vmclear in kdump in some conditions.
>>
>> Changelog from v2 to v3:
>> 1. remove unnecessary conditions in function
>>    cpu_emergency_clear_loaded_vmcss as Marcelo suggested.
>>
>> Changelog from v1 to v2:
>> 1. remove the sysctl and clear VMCSs unconditionally.
>>
>> Zhang Yanfei (2):
>>   x86/kexec: add a new atomic notifier list for kdump
>>   KVM-INTEL: add a notifier and a bitmap to support VMCLEAR in kdump
>>
>>  arch/x86/include/asm/kexec.h |    2 +
>>  arch/x86/kernel/crash.c      |    9 +++++
>>  arch/x86/kvm/vmx.c           |   70 ++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 81 insertions(+), 0 deletions(-)
> 
> --
> 			Gleb.
> 

Re: [PATCH v8 0/2] x86: vmclear vmcss on all cpus when doing kdump if necessary

[Zhang Yanfei]

Hello Eric

Would you help review the patch, and give some comments, please ? 

Thanks
Zhang

于 2012年11月22日 16:22, Zhang Yanfei 写道:
> Currently, kdump just makes all the logical processors leave VMX operation by
> executing VMXOFF instruction, so any VMCSs active on the logical processors may
> be corrupted. But, sometimes, we need the VMCSs to debug guest images contained
> in the host vmcore. To prevent the corruption, we should VMCLEAR the VMCSs before
> executing the VMXOFF instruction.
> 
> The patch set provides a way to VMCLEAR vmcss related to guests on all cpus before
> executing the VMXOFF when doing kdump. This is used to ensure the VMCSs in the
> vmcore updated and non-corrupted.
> 
> Changelog from v7 to v8:
> 1. KEXEC: regression for using name crash_notifier_list
>    and remove comments related to KVM
>    and just call function atomic_notifier_call_chain directly.
> 
> Changelog from v6 to v7:
> 1. KVM-INTEL: in hardware_disable, we needn't disable the
>    vmclear, so remove it.
> 
> Changelog from v5 to v6:
> 1. KEXEC: the atomic notifier list renamed:
>    crash_notifier_list --> vmclear_notifier_list
> 2. KVM-INTEL: provide empty functions if CONFIG_KEXEC is
>    not defined and remove unnecessary #ifdef's.
> 
> Changelog from v4 to v5:
> 1. use an atomic notifier instead of function call, so
>    have all the vmclear codes in vmx.c.
> 
> Changelog from v3 to v4:
> 1. add a new percpu variable vmclear_skipped to skip
>    vmclear in kdump in some conditions.
> 
> Changelog from v2 to v3:
> 1. remove unnecessary conditions in function
>    cpu_emergency_clear_loaded_vmcss as Marcelo suggested.
> 
> Changelog from v1 to v2:
> 1. remove the sysctl and clear VMCSs unconditionally.
> 
> Zhang Yanfei (2):
>   x86/kexec: add a new atomic notifier list for kdump
>   KVM-INTEL: add a notifier and a bitmap to support VMCLEAR in kdump
> 
>  arch/x86/include/asm/kexec.h |    2 +
>  arch/x86/kernel/crash.c      |    9 +++++
>  arch/x86/kvm/vmx.c           |   70 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 81 insertions(+), 0 deletions(-)

Re: [PATCH v8 1/2] x86/kexec: add a new atomic notifier list for kdump

[Eric W. Biederman]

Zhang Yanfei <zhangyanfei@cn.fujitsu.com> writes:

> This patch adds an atomic notifier list named crash_notifier_list.
> Currently, when loading kvm-intel module, a notifier will be registered
> in the list to enable vmcss loaded on all cpus to be VMCLEAR'd if
> needed.

crash_notifier_list ick gag please no.  Effectively this makes the kexec
on panic code path undebuggable.

Instead we need to use direct function calls to whatever you are doing.

If a direct function call is too complex then the piece of code you want
to call is almost certainly too complex to be calling on a code path
like this.

Eric

> Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
> ---
>  arch/x86/include/asm/kexec.h |    2 ++
>  arch/x86/kernel/crash.c      |    9 +++++++++
>  2 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
> index 317ff17..5e22b00 100644
> --- a/arch/x86/include/asm/kexec.h
> +++ b/arch/x86/include/asm/kexec.h
> @@ -163,6 +163,8 @@ struct kimage_arch {
>  };
>  #endif
>  
> +extern struct atomic_notifier_head crash_notifier_list;
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif /* _ASM_X86_KEXEC_H */
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index 13ad899..c5b2f70 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -16,6 +16,8 @@
>  #include <linux/delay.h>
>  #include <linux/elf.h>
>  #include <linux/elfcore.h>
> +#include <linux/module.h>
> +#include <linux/notifier.h>
>  
>  #include <asm/processor.h>
>  #include <asm/hardirq.h>
> @@ -30,6 +32,9 @@
>  
>  int in_crash_kexec;
>  
> +ATOMIC_NOTIFIER_HEAD(crash_notifier_list);
> +EXPORT_SYMBOL_GPL(crash_notifier_list);
> +
>  #if defined(CONFIG_SMP) && defined(CONFIG_X86_LOCAL_APIC)
>  
>  static void kdump_nmi_callback(int cpu, struct pt_regs *regs)
> @@ -46,6 +51,8 @@ static void kdump_nmi_callback(int cpu, struct pt_regs *regs)
>  #endif
>  	crash_save_cpu(regs, cpu);
>  
> +	atomic_notifier_call_chain(&crash_notifier_list, 0, NULL);
> +
>  	/* Disable VMX or SVM if needed.
>  	 *
>  	 * We need to disable virtualization on all CPUs.
> @@ -88,6 +95,8 @@ void native_machine_crash_shutdown(struct pt_regs *regs)
>  
>  	kdump_nmi_shootdown_cpus();
>  
> +	atomic_notifier_call_chain(&crash_notifier_list, 0, NULL);
> +
>  	/* Booting kdump kernel with VMX or SVM enabled won't work,
>  	 * because (among other limitations) we can't disable paging
>  	 * with the virt flags.

Re: [PATCH v8 1/2] x86/kexec: add a new atomic notifier list for kdump

[Gleb Natapov]

On Mon, Nov 26, 2012 at 09:08:54AM -0600, Eric W. Biederman wrote:
> Zhang Yanfei <zhangyanfei@cn.fujitsu.com> writes:
> 
> > This patch adds an atomic notifier list named crash_notifier_list.
> > Currently, when loading kvm-intel module, a notifier will be registered
> > in the list to enable vmcss loaded on all cpus to be VMCLEAR'd if
> > needed.
> 
> crash_notifier_list ick gag please no.  Effectively this makes the kexec
> on panic code path undebuggable.
> 
> Instead we need to use direct function calls to whatever you are doing.
> 
The code walks linked list in kvm-intel module and calls vmclear on
whatever it finds there. Since the function have to resides in kvm-intel
module it cannot be called directly. Is callback pointer that is set
by kvm-intel more acceptable?

> If a direct function call is too complex then the piece of code you want
> to call is almost certainly too complex to be calling on a code path
> like this.
> 
> Eric
> 
> > Signed-off-by: Zhang Yanfei <zhangyanfei@cn.fujitsu.com>
> > ---
> >  arch/x86/include/asm/kexec.h |    2 ++
> >  arch/x86/kernel/crash.c      |    9 +++++++++
> >  2 files changed, 11 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
> > index 317ff17..5e22b00 100644
> > --- a/arch/x86/include/asm/kexec.h
> > +++ b/arch/x86/include/asm/kexec.h
> > @@ -163,6 +163,8 @@ struct kimage_arch {
> >  };
> >  #endif
> >  
> > +extern struct atomic_notifier_head crash_notifier_list;
> > +
> >  #endif /* __ASSEMBLY__ */
> >  
> >  #endif /* _ASM_X86_KEXEC_H */
> > diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> > index 13ad899..c5b2f70 100644
> > --- a/arch/x86/kernel/crash.c
> > +++ b/arch/x86/kernel/crash.c
> > @@ -16,6 +16,8 @@
> >  #include <linux/delay.h>
> >  #include <linux/elf.h>
> >  #include <linux/elfcore.h>
> > +#include <linux/module.h>
> > +#include <linux/notifier.h>
> >  
> >  #include <asm/processor.h>
> >  #include <asm/hardirq.h>
> > @@ -30,6 +32,9 @@
> >  
> >  int in_crash_kexec;
> >  
> > +ATOMIC_NOTIFIER_HEAD(crash_notifier_list);
> > +EXPORT_SYMBOL_GPL(crash_notifier_list);
> > +
> >  #if defined(CONFIG_SMP) && defined(CONFIG_X86_LOCAL_APIC)
> >  
> >  static void kdump_nmi_callback(int cpu, struct pt_regs *regs)
> > @@ -46,6 +51,8 @@ static void kdump_nmi_callback(int cpu, struct pt_regs *regs)
> >  #endif
> >  	crash_save_cpu(regs, cpu);
> >  
> > +	atomic_notifier_call_chain(&crash_notifier_list, 0, NULL);
> > +
> >  	/* Disable VMX or SVM if needed.
> >  	 *
> >  	 * We need to disable virtualization on all CPUs.
> > @@ -88,6 +95,8 @@ void native_machine_crash_shutdown(struct pt_regs *regs)
> >  
> >  	kdump_nmi_shootdown_cpus();
> >  
> > +	atomic_notifier_call_chain(&crash_notifier_list, 0, NULL);
> > +
> >  	/* Booting kdump kernel with VMX or SVM enabled won't work,
> >  	 * because (among other limitations) we can't disable paging
> >  	 * with the virt flags.

--
			Gleb.

Re: [PATCH v8 1/2] x86/kexec: add a new atomic notifier list for kdump

[Eric W. Biederman]

Gleb Natapov <gleb@redhat.com> writes:

> On Mon, Nov 26, 2012 at 09:08:54AM -0600, Eric W. Biederman wrote:
>> Zhang Yanfei <zhangyanfei@cn.fujitsu.com> writes:
>> 
>> > This patch adds an atomic notifier list named crash_notifier_list.
>> > Currently, when loading kvm-intel module, a notifier will be registered
>> > in the list to enable vmcss loaded on all cpus to be VMCLEAR'd if
>> > needed.
>> 
>> crash_notifier_list ick gag please no.  Effectively this makes the kexec
>> on panic code path undebuggable.
>> 
>> Instead we need to use direct function calls to whatever you are doing.
>> 
> The code walks linked list in kvm-intel module and calls vmclear on
> whatever it finds there. Since the function have to resides in kvm-intel
> module it cannot be called directly. Is callback pointer that is set
> by kvm-intel more acceptable?

Yes a specific callback function is more acceptable.  Looking a little
deeper vmclear_local_loaded_vmcss is not particularly acceptable. It is
doing a lot of work that is unnecessary to save the virtual registers
on the kexec on panic path.

In fact I wonder if it might not just be easier to call vmcs_clear to a
fixed per cpu buffer.

Performing list walking in interrupt context without locking in
vmclear_local_loaded vmcss looks a bit scary.  Not that locking would
make it any better, as locking would simply add one more way to deadlock
the system.  Only an rcu list walk is at all safe.  A list walk that
modifies the list as vmclear_local_loaded_vmcss does is definitely not safe.

Eric

Re: [PATCH v8 1/2] x86/kexec: add a new atomic notifier list for kdump

[Gleb Natapov]

On Mon, Nov 26, 2012 at 11:43:10AM -0600, Eric W. Biederman wrote:
> Gleb Natapov <gleb@redhat.com> writes:
> 
> > On Mon, Nov 26, 2012 at 09:08:54AM -0600, Eric W. Biederman wrote:
> >> Zhang Yanfei <zhangyanfei@cn.fujitsu.com> writes:
> >> 
> >> > This patch adds an atomic notifier list named crash_notifier_list.
> >> > Currently, when loading kvm-intel module, a notifier will be registered
> >> > in the list to enable vmcss loaded on all cpus to be VMCLEAR'd if
> >> > needed.
> >> 
> >> crash_notifier_list ick gag please no.  Effectively this makes the kexec
> >> on panic code path undebuggable.
> >> 
> >> Instead we need to use direct function calls to whatever you are doing.
> >> 
> > The code walks linked list in kvm-intel module and calls vmclear on
> > whatever it finds there. Since the function have to resides in kvm-intel
> > module it cannot be called directly. Is callback pointer that is set
> > by kvm-intel more acceptable?
> 
> Yes a specific callback function is more acceptable.  Looking a little
> deeper vmclear_local_loaded_vmcss is not particularly acceptable. It is
> doing a lot of work that is unnecessary to save the virtual registers
> on the kexec on panic path.
> 
What work are you referring to in particular that may not be acceptable?

> In fact I wonder if it might not just be easier to call vmcs_clear to a
> fixed per cpu buffer.
> 
There may be more than one vmcs loaded on a cpu, hence the list.

> Performing list walking in interrupt context without locking in
> vmclear_local_loaded vmcss looks a bit scary.  Not that locking would
> make it any better, as locking would simply add one more way to deadlock
> the system.  Only an rcu list walk is at all safe.  A list walk that
> modifies the list as vmclear_local_loaded_vmcss does is definitely not safe.
> 
The list vmclear_local_loaded walks is per cpu. Zhang's kvm patch
disables kexec callback while list is modified.

--
			Gleb.

Re: [PATCH v8 1/2] x86/kexec: add a new atomic notifier list for kdump

[Eric W. Biederman]

Gleb Natapov <gleb@redhat.com> writes:

> On Mon, Nov 26, 2012 at 11:43:10AM -0600, Eric W. Biederman wrote:
>> Gleb Natapov <gleb@redhat.com> writes:
>> 
>> > On Mon, Nov 26, 2012 at 09:08:54AM -0600, Eric W. Biederman wrote:
>> >> Zhang Yanfei <zhangyanfei@cn.fujitsu.com> writes:
>> >> 
>> >> > This patch adds an atomic notifier list named crash_notifier_list.
>> >> > Currently, when loading kvm-intel module, a notifier will be registered
>> >> > in the list to enable vmcss loaded on all cpus to be VMCLEAR'd if
>> >> > needed.
>> >> 
>> >> crash_notifier_list ick gag please no.  Effectively this makes the kexec
>> >> on panic code path undebuggable.
>> >> 
>> >> Instead we need to use direct function calls to whatever you are doing.
>> >> 
>> > The code walks linked list in kvm-intel module and calls vmclear on
>> > whatever it finds there. Since the function have to resides in kvm-intel
>> > module it cannot be called directly. Is callback pointer that is set
>> > by kvm-intel more acceptable?
>> 
>> Yes a specific callback function is more acceptable.  Looking a little
>> deeper vmclear_local_loaded_vmcss is not particularly acceptable. It is
>> doing a lot of work that is unnecessary to save the virtual registers
>> on the kexec on panic path.
>> 
> What work are you referring to in particular that may not be
> acceptable?

The unnecessary work that I was see is all of the software state
changing.  Unlinking things from linked lists flipping variables.
None of that appears related to the fundamental issue saving cpu
state.

Simply reusing a function that does more than what is strictly required
makes me nervous.  What is the chance that the function will grow
with maintenance and add constructs that are not safe in a kexec on
panic situtation.

>> In fact I wonder if it might not just be easier to call vmcs_clear to a
>> fixed per cpu buffer.
>> 
> There may be more than one vmcs loaded on a cpu, hence the list.
>
>> Performing list walking in interrupt context without locking in
>> vmclear_local_loaded vmcss looks a bit scary.  Not that locking would
>> make it any better, as locking would simply add one more way to deadlock
>> the system.  Only an rcu list walk is at all safe.  A list walk that
>> modifies the list as vmclear_local_loaded_vmcss does is definitely not safe.
>> 
> The list vmclear_local_loaded walks is per cpu. Zhang's kvm patch
> disables kexec callback while list is modified.

If the list is only modified on it's cpu and we are running on that cpu
that does look like it will give the necessary protections.  It isn't
particularly clear at first glance that is the case unfortunately.

Eric

Re: [PATCH v8 1/2] x86/kexec: add a new atomic notifier list for kdump

[Zhang Yanfei]

于 2012年11月27日 02:18, Eric W. Biederman 写道:
> Gleb Natapov <gleb@redhat.com> writes:
> 
>> On Mon, Nov 26, 2012 at 11:43:10AM -0600, Eric W. Biederman wrote:
>>> Gleb Natapov <gleb@redhat.com> writes:
>>>
>>>> On Mon, Nov 26, 2012 at 09:08:54AM -0600, Eric W. Biederman wrote:
>>>>> Zhang Yanfei <zhangyanfei@cn.fujitsu.com> writes:
>>>>>
>>>>>> This patch adds an atomic notifier list named crash_notifier_list.
>>>>>> Currently, when loading kvm-intel module, a notifier will be registered
>>>>>> in the list to enable vmcss loaded on all cpus to be VMCLEAR'd if
>>>>>> needed.
>>>>>
>>>>> crash_notifier_list ick gag please no.  Effectively this makes the kexec
>>>>> on panic code path undebuggable.
>>>>>
>>>>> Instead we need to use direct function calls to whatever you are doing.
>>>>>
>>>> The code walks linked list in kvm-intel module and calls vmclear on
>>>> whatever it finds there. Since the function have to resides in kvm-intel
>>>> module it cannot be called directly. Is callback pointer that is set
>>>> by kvm-intel more acceptable?
>>>
>>> Yes a specific callback function is more acceptable.  Looking a little
>>> deeper vmclear_local_loaded_vmcss is not particularly acceptable. It is
>>> doing a lot of work that is unnecessary to save the virtual registers
>>> on the kexec on panic path.
>>>
>> What work are you referring to in particular that may not be
>> acceptable?
> 
> The unnecessary work that I was see is all of the software state
> changing.  Unlinking things from linked lists flipping variables.
> None of that appears related to the fundamental issue saving cpu
> state.
> 
> Simply reusing a function that does more than what is strictly required
> makes me nervous.  What is the chance that the function will grow
> with maintenance and add constructs that are not safe in a kexec on
> panic situtation.

So in summary,

1. a specific callback function instead of a notifier?

2. Instead of calling vmclear_local_loaded_vmcss, the vmclear operation
   will just call the vmclear on every vmcss loaded on the cpu?

   like below:

   static void crash_vmclear_local_loaded_vmcss(void)
   {
        int cpu = raw_smp_processor_id();
        struct loaded_vmcs *v, *n;

        if (!crash_local_vmclear_enabled(cpu))
                return;

        list_for_each_entry_safe(v, n, &per_cpu(loaded_vmcss_on_cpu, cpu),
                                 loaded_vmcss_on_cpu_link)
                vmcs_clear(v->vmcs);
   }

   right?

Thanks
Zhang

> 
>>> In fact I wonder if it might not just be easier to call vmcs_clear to a
>>> fixed per cpu buffer.
>>>
>> There may be more than one vmcs loaded on a cpu, hence the list.
>>
>>> Performing list walking in interrupt context without locking in
>>> vmclear_local_loaded vmcss looks a bit scary.  Not that locking would
>>> make it any better, as locking would simply add one more way to deadlock
>>> the system.  Only an rcu list walk is at all safe.  A list walk that
>>> modifies the list as vmclear_local_loaded_vmcss does is definitely not safe.
>>>
>> The list vmclear_local_loaded walks is per cpu. Zhang's kvm patch
>> disables kexec callback while list is modified.
> 
> If the list is only modified on it's cpu and we are running on that cpu
> that does look like it will give the necessary protections.  It isn't
> particularly clear at first glance that is the case unfortunately.
> 
> Eric
> 

Re: [PATCH v8 1/2] x86/kexec: add a new atomic notifier list for kdump

[Eric W. Biederman]

Zhang Yanfei <zhangyanfei@cn.fujitsu.com> writes:

> So in summary,
>
> 1. a specific callback function instead of a notifier?

Yes.

> 2. Instead of calling vmclear_local_loaded_vmcss, the vmclear operation
>    will just call the vmclear on every vmcss loaded on the cpu?
>
>    like below:
>
>    static void crash_vmclear_local_loaded_vmcss(void)
>    {
>         int cpu = raw_smp_processor_id();
>         struct loaded_vmcs *v, *n;
>
>         if (!crash_local_vmclear_enabled(cpu))
>                 return;
>
>         list_for_each_entry_safe(v, n, &per_cpu(loaded_vmcss_on_cpu, cpu),
>                                  loaded_vmcss_on_cpu_link)
>                 vmcs_clear(v->vmcs);
>    }
>
>    right?

Yeah that looks good.  I would do list_for_each_entry because the list
isn't changing.

Eric

Re: [PATCH v8 1/2] x86/kexec: add a new atomic notifier list for kdump

[Zhang Yanfei]

于 2012年11月27日 09:49, Eric W. Biederman 写道:
> Zhang Yanfei <zhangyanfei@cn.fujitsu.com> writes:
> 
>> So in summary,
>>
>> 1. a specific callback function instead of a notifier?
> 
> Yes.
> 
>> 2. Instead of calling vmclear_local_loaded_vmcss, the vmclear operation
>>    will just call the vmclear on every vmcss loaded on the cpu?
>>
>>    like below:
>>
>>    static void crash_vmclear_local_loaded_vmcss(void)
>>    {
>>         int cpu = raw_smp_processor_id();
>>         struct loaded_vmcs *v, *n;
>>
>>         if (!crash_local_vmclear_enabled(cpu))
>>                 return;
>>
>>         list_for_each_entry_safe(v, n, &per_cpu(loaded_vmcss_on_cpu, cpu),
>>                                  loaded_vmcss_on_cpu_link)
>>                 vmcs_clear(v->vmcs);
>>    }
>>
>>    right?
> 
> Yeah that looks good.  I would do list_for_each_entry because the list
> isn't changing.

OK.

I will update the patch and resend it.

Zhang