Re: [PATCH v9 2/4] arm: ARMv7 dirty page logging inital mem region write protect (w/no huge PUD support)

[Mario Smarduch <m.smarduch@samsung.com>]

On 08/12/2014 02:36 AM, Christoffer Dall wrote:
> On Mon, Aug 11, 2014 at 06:36:14PM -0700, Mario Smarduch wrote:
>> On 08/11/2014 12:12 PM, Christoffer Dall wrote:
> 
> [...]
> 
>>>> +/**
>>>> + * stage2_wp_range() - write protect stage2 memory region range
>>>> + * @kvm:	The KVM pointer
>>>> + * @start:	Start address of range
>>>> + * &end:	End address of range
>>>> + */
>>>> +static void stage2_wp_range(struct kvm *kvm, phys_addr_t addr, phys_addr_t end)
>>>> +{
>>>> +	pgd_t *pgd;
>>>> +	phys_addr_t next;
>>>> +
>>>> +	pgd = kvm->arch.pgd + pgd_index(addr);
>>>> +	do {
>>>> +		/*
>>>> +		 * Release kvm_mmu_lock periodically if the memory region is
>>>> +		 * large features like detect hung task, lock detector or lock
>>>                    large.  Otherwise, we may see panics due to..
>>>> +		 * dep  may panic. In addition holding the lock this long will
>>>     extra white space ^^           Additionally, holding the lock for a
>>>     long timer will
>>>> +		 * also starve other vCPUs. Applies to huge VM memory regions.
>>>                                             ^^^ I don't understand this
>>> 					    last remark.
>> Sorry overlooked this.
>>
>> While testing - VM regions that were small (~1GB) holding the mmu_lock
>> caused not problems, but when I was running memory regions around 2GB large
>> some kernel lockup detection/lock contention options (some selected by default) 
>> caused deadlock warnings/panics in host kernel.
>>
>> This was in one my previous review comments sometime ago, I can go back
>> and find the options.
>>
> 
> Just drop the last part of the comment, so the whole thing reads:
> 
> /*
>  * Release kvm_mmu_lock periodically if the memory region is
>  * large. Otherwise, we may see kernel panics from debugging features
>  * such as "detect hung task", "lock detector" or "lock dep checks".
>  * Additionally, holding the lock too long will also starve other vCPUs.
>  */
> 
> And check the actual names of those debugging features or use the
> CONFIG_<WHATEVER> names and say "we may see kernel panics with CONFIG_X,
> CONFIG_Y, and CONFIG_Z.
> 
> Makes sense?

Yep sure does.

> 
> -Christoffer
>