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

Christoffer Dall
2014-08-12T09:36:16Z

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?

-Christoffer