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