From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xiao Guangrong Subject: Re: [PATCH 3/3] KVM: MMU: Consolidate common code in mmu_free_roots() Date: Mon, 13 May 2013 21:02:10 +0800 Message-ID: <5190E452.3080806@linux.vnet.ibm.com> References: <20130509154350.15b956c4.yoshikawa_takuya_b1@lab.ntt.co.jp> <20130509154602.da528c3b.yoshikawa_takuya_b1@lab.ntt.co.jp> <518B7653.60009@linux.vnet.ibm.com> <20130510100545.9e3ee98f.yoshikawa_takuya_b1@lab.ntt.co.jp> <518C50D6.3080805@linux.vnet.ibm.com> <20130513112415.GO10830@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: Takuya Yoshikawa , pbonzini@redhat.com, mtosatti@redhat.com, kvm@vger.kernel.org To: Gleb Natapov Return-path: Received: from e23smtp08.au.ibm.com ([202.81.31.141]:57058 "EHLO e23smtp08.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752285Ab3EMNCV (ORCPT ); Mon, 13 May 2013 09:02:21 -0400 Received: from /spool/local by e23smtp08.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 13 May 2013 22:59:53 +1000 Received: from d23relay03.au.ibm.com (d23relay03.au.ibm.com [9.190.235.21]) by d23dlp01.au.ibm.com (Postfix) with ESMTP id 1FCAF2CE804A for ; Mon, 13 May 2013 23:02:14 +1000 (EST) Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay03.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r4DD26vv21168268 for ; Mon, 13 May 2013 23:02:06 +1000 Received: from d23av03.au.ibm.com (loopback [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r4DD2DlK031842 for ; Mon, 13 May 2013 23:02:13 +1000 In-Reply-To: <20130513112415.GO10830@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 05/13/2013 07:24 PM, Gleb Natapov wrote: > On Fri, May 10, 2013 at 09:43:50AM +0800, Xiao Guangrong wrote: >> On 05/10/2013 09:05 AM, Takuya Yoshikawa wrote: >>> On Thu, 09 May 2013 18:11:31 +0800 >>> Xiao Guangrong 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 >>>>> --- >>>>> 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. >> > I agree that this is mostly code style issue and with Takuya patch the > indentation is dipper. Also the structure of mmu_free_roots() resembles > mmu_alloc_shadow_roots() currently. The latter has the same if(){return} > pattern. I also agree with Takuya that locking symmetry can be improved. > What about something like this (untested): It is good to me! ;)