From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Mackerras Date: Thu, 10 Oct 2013 10:52:17 +0000 Subject: Re: [PATCH 3/4] kvm: powerpc: define a linux pte lookup function Message-Id: <20131010105217.GF9906@iris.ozlabs.ibm.com> List-Id: References: <1381212212-29641-1-git-send-email-Bharat.Bhushan@freescale.com> <1381212212-29641-4-git-send-email-Bharat.Bhushan@freescale.com> <1381268203.7979.276.camel@snotra.buserror.net> <6A3DF150A5B70D4F9B66A25E3F7C888D071A8AD0@039-SN2MPN1-012.039d.mgd.msft.net> <1381340851.7979.327.camel@snotra.buserror.net> <20131010103540.GD9906@iris.ozlabs.ibm.com> <6A3DF150A5B70D4F9B66A25E3F7C888D071AF2DD@039-SN2MPN1-012.039d.mgd.msft.net> In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D071AF2DD@039-SN2MPN1-012.039d.mgd.msft.net> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Bhushan Bharat-R65777 Cc: Wood Scott-B07421 , "agraf@suse.de" , Yoder Stuart-B08248 , "kvm@vger.kernel.org" , "kvm-ppc@vger.kernel.org" On Thu, Oct 10, 2013 at 10:44:54AM +0000, Bhushan Bharat-R65777 wrote: > > > > -----Original Message----- > > From: Paul Mackerras [mailto:paulus@samba.org] > > Sent: Thursday, October 10, 2013 4:06 PM > > To: Wood Scott-B07421 > > Cc: Bhushan Bharat-R65777; Wood Scott-B07421; agraf@suse.de; Yoder Stuart- > > B08248; kvm@vger.kernel.org; kvm-ppc@vger.kernel.org > > Subject: Re: [PATCH 3/4] kvm: powerpc: define a linux pte lookup function > > > > On Wed, Oct 09, 2013 at 12:47:31PM -0500, Scott Wood wrote: > > > On Wed, 2013-10-09 at 03:48 -0500, Bhushan Bharat-R65777 wrote: > > > > > > > > What lookup_linux_pte_and_update() does:- > > > > - find_linux_pte_or_hugepte() > > > > - does size and some other trivial checks > > > > - Then atomically update the pte:- > > > > => while() > > > > => wait till _PAGE_BUSY is clear > > > > => atomically update the pte > > > > => if not updated then go back to while() above else break > > > > > > > > > > > > While what lookup_linux_pte() does:- > > > > - find_linux_pte_or_hugepte() > > > > - does size and some other trivial checks > > > > - wait till _PAGE_BUSY is clear > > > > - return pte > > > > > > > > I am finding it difficult to call lookup_linux_pte() from > > lookup_linux_pte_and_update(). > > > > > > You could factor out a common lookup_linux_ptep(). > > > > I don't really think it's enough code to be worth wringing out the last drop of > > duplication. However, if he removed the checks for _PAGE_BUSY and _PAGE_PRESENT > > as I suggested in another mail, and made it return the pte pointer rather than > > the value, it would then essentially be a lookup_linux_ptep() as you suggest. > > Do we want to have lookup_linux_pte() or lookup_linux_ptep() or both where lookup_linux_pte() and lookup_linux_pte_and_update() calls lookup_linux_ptep() ? I don't think we want both lookup_linux_pte() and lookup_linux_ptep(). Either have a lookup_linux_ptep() and call it from your book E code and from lookup_linux_pte_and_update(), or keep a separate lookup_linux_pte() and don't make the _and_update() version call it. I don't really care; you decide which looks nicer in the end. Of course someone will then tell you to do it the other way, but at that point it's just a matter of taste. Paul. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Mackerras Subject: Re: [PATCH 3/4] kvm: powerpc: define a linux pte lookup function Date: Thu, 10 Oct 2013 21:52:17 +1100 Message-ID: <20131010105217.GF9906@iris.ozlabs.ibm.com> References: <1381212212-29641-1-git-send-email-Bharat.Bhushan@freescale.com> <1381212212-29641-4-git-send-email-Bharat.Bhushan@freescale.com> <1381268203.7979.276.camel@snotra.buserror.net> <6A3DF150A5B70D4F9B66A25E3F7C888D071A8AD0@039-SN2MPN1-012.039d.mgd.msft.net> <1381340851.7979.327.camel@snotra.buserror.net> <20131010103540.GD9906@iris.ozlabs.ibm.com> <6A3DF150A5B70D4F9B66A25E3F7C888D071AF2DD@039-SN2MPN1-012.039d.mgd.msft.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Wood Scott-B07421 , "agraf@suse.de" , Yoder Stuart-B08248 , "kvm@vger.kernel.org" , "kvm-ppc@vger.kernel.org" To: Bhushan Bharat-R65777 Return-path: Content-Disposition: inline In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D071AF2DD@039-SN2MPN1-012.039d.mgd.msft.net> Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On Thu, Oct 10, 2013 at 10:44:54AM +0000, Bhushan Bharat-R65777 wrote: > > > > -----Original Message----- > > From: Paul Mackerras [mailto:paulus@samba.org] > > Sent: Thursday, October 10, 2013 4:06 PM > > To: Wood Scott-B07421 > > Cc: Bhushan Bharat-R65777; Wood Scott-B07421; agraf@suse.de; Yoder Stuart- > > B08248; kvm@vger.kernel.org; kvm-ppc@vger.kernel.org > > Subject: Re: [PATCH 3/4] kvm: powerpc: define a linux pte lookup function > > > > On Wed, Oct 09, 2013 at 12:47:31PM -0500, Scott Wood wrote: > > > On Wed, 2013-10-09 at 03:48 -0500, Bhushan Bharat-R65777 wrote: > > > > > > > > What lookup_linux_pte_and_update() does:- > > > > - find_linux_pte_or_hugepte() > > > > - does size and some other trivial checks > > > > - Then atomically update the pte:- > > > > => while() > > > > => wait till _PAGE_BUSY is clear > > > > => atomically update the pte > > > > => if not updated then go back to while() above else break > > > > > > > > > > > > While what lookup_linux_pte() does:- > > > > - find_linux_pte_or_hugepte() > > > > - does size and some other trivial checks > > > > - wait till _PAGE_BUSY is clear > > > > - return pte > > > > > > > > I am finding it difficult to call lookup_linux_pte() from > > lookup_linux_pte_and_update(). > > > > > > You could factor out a common lookup_linux_ptep(). > > > > I don't really think it's enough code to be worth wringing out the last drop of > > duplication. However, if he removed the checks for _PAGE_BUSY and _PAGE_PRESENT > > as I suggested in another mail, and made it return the pte pointer rather than > > the value, it would then essentially be a lookup_linux_ptep() as you suggest. > > Do we want to have lookup_linux_pte() or lookup_linux_ptep() or both where lookup_linux_pte() and lookup_linux_pte_and_update() calls lookup_linux_ptep() ? I don't think we want both lookup_linux_pte() and lookup_linux_ptep(). Either have a lookup_linux_ptep() and call it from your book E code and from lookup_linux_pte_and_update(), or keep a separate lookup_linux_pte() and don't make the _and_update() version call it. I don't really care; you decide which looks nicer in the end. Of course someone will then tell you to do it the other way, but at that point it's just a matter of taste. Paul.