? 2012?12?05? 04:14, Eric W. Biederman ??: > Zhang Yanfei <zhangyanfei at cn.fujitsu.com> writes: > >> This patch 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. > > Apologies for the delay I have been travelling, and I wanted > to at least read through the code. > > Overall I think this is good but I have one nit, and I see one real > problem with this code. > >> +/* >> + * This is used to VMCLEAR all VMCSs loaded on the >> + * processor. And when loading kvm_intel module, the >> + * callback function pointer will be assigned. >> + */ >> +void (*crash_vmclear_loaded_vmcss)(void) = NULL; >> +EXPORT_SYMBOL_GPL(crash_vmclear_loaded_vmcss); >> + >> +static inline void cpu_emergency_vmclear_loaded_vmcss(void) >> +{ >> + if (crash_vmclear_loaded_vmcss) >> + crash_vmclear_loaded_vmcss(); >> +} > > The nit is the use of emergency instead of crash in the name. ok, emergency -> crash > > The problem is that this is potentially a NULL pointer dereference if > kvm-intel is removed. The easist fix would be in your second patch to > just make it impossible to unload the kvm-intel module. Otherwise > there the deference of crash_vmclear_loaded_vmcss needs to be rcu > protected, with a syncrhonize_rcu after the pointer is set to NULL in > the unload path. Ah, thanks for this comment. I think I will use the rcu machanism to solve the problem. > > Otherwise I have no objections to this code. Thanks for your review. I will update the patch and resend it. Thanks Zhang Yanfei