Re: [PATCH 3/3] KVM: MMU: Consolidate common code in mmu_free_roots()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/10/2013 09:05 AM, Takuya Yoshikawa wrote:
> On Thu, 09 May 2013 18:11:31 +0800
> Xiao Guangrong <xiaoguangrong@xxxxxxxxxxxxxxxxxx> wrote:
> 
>> On 05/09/2013 02:46 PM, Takuya Yoshikawa wrote:
>>> By making the last three statements common to both if/else cases, the
>>> symmetry between the locking and unlocking becomes clearer. One note
>>> here is that VCPU's root_hpa does not need to be protected by mmu_lock.
>>>
>>> Signed-off-by: Takuya Yoshikawa <yoshikawa_takuya_b1@xxxxxxxxxxxxx>
>>> ---
>>>  arch/x86/kvm/mmu.c |   39 +++++++++++++++++++--------------------
>>>  1 files changed, 19 insertions(+), 20 deletions(-)
>>
>> DO NOT think it makes any thing better.
>>
> 
> Why do we need to do the same thing differently in two paths?

They are different cases, one is the long mode, anther is PAE mode,
return from one case and continue to handle the anther case is common
used, and it can reduce the indentation and easily follow the code.

Anyway, this is the code style, using if-else is not better than
if() return.

> Especially one path looks like protecting root_hpa while the other does not.

The simple code, "vcpu->arch.mmu.root_hpa = INVALID_PAGE;", does not hurt
the lock. But moving them out of the lock is ok.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux