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

[Eric W. Biederman]

Gleb Natapov <gleb@xxxxxxxxxx> writes:

> On Mon, Nov 26, 2012 at 09:08:54AM -0600, Eric W. Biederman wrote:
>> Zhang Yanfei <zhangyanfei@xxxxxxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/