From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Vrabel Subject: Re: [PATCH v2 1/2] xen/balloon: set a mapping for ballooned out pages Date: Tue, 23 Jul 2013 16:03:49 +0100 Message-ID: <51EE9B55.5040303@citrix.com> References: <1374510529-10395-1-git-send-email-stefano.stabellini@eu.citrix.com> <51ED6317.2060707@citrix.com> <1374513727.6623.17.camel@hastur.hellion.org.uk> <51ED6B56.6060807@citrix.com> <1374514334.6623.20.camel@hastur.hellion.org.uk> <51EE53A8.50408@citrix.com> <1374589626.6623.114.camel@hastur.hellion.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1374589626.6623.114.camel@hastur.hellion.org.uk> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: dcrisan@flexiant.com, xen-devel@lists.xensource.com, alex@alex.org.uk, Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On 23/07/13 15:27, Ian Campbell wrote: > On Tue, 2013-07-23 at 10:58 +0100, David Vrabel wrote: >> On 22/07/13 18:32, Ian Campbell wrote: >>> On Mon, 2013-07-22 at 18:26 +0100, David Vrabel wrote: >>>> On 22/07/13 18:22, Ian Campbell wrote: >>>>> On Mon, 2013-07-22 at 17:51 +0100, David Vrabel wrote: >>>>>> On 22/07/13 17:28, Stefano Stabellini wrote: >>>>>>> >>>>>>> #ifdef CONFIG_HIGHMEM >>>>>>> #define inc_totalhigh_pages() (totalhigh_pages++) >>>>>>> @@ -423,7 +426,8 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp) >>>>>>> if (xen_pv_domain() && !PageHighMem(page)) { >>>>>>> ret = HYPERVISOR_update_va_mapping( >>>>>>> (unsigned long)__va(pfn << PAGE_SHIFT), >>>>>>> - __pte_ma(0), 0); >>>>>>> + pfn_pte(page_to_pfn(get_balloon_trade_page()), >>>>>>> + PAGE_KERNEL_RO), 0); >>>>>> >>>>>> Preemption needs to be disabled while using the trade page, see >>>>>> suggestion below. >>>>> >>>>> Hopefully you mean just when setting up/manipulating it? >>>> >>>> Yes, sorry. >>>> >>>> get_...() >>>> update_va_mapping() >>>> put_...() >>> >>> I can see why it would matter in the unmap_and_replace+fixup case (since >>> you need them to happen "atomically") but why in this case? We don't >>> actually care which of the trade pages gets used for this purpose, so >>> even if we happen to get preempted and change CPU it doesn't really >>> matter. >> >> If a trade page from another CPU is used, it may concurrently have it's >> MFN cleared by a unmap_and_replace call on the other CPU. > > The call on the other CPU clears the virtual address, not the MFN, so > long as we have the MFN in our hand we are OK, I think? Nothing updates > the m2p or p2m does it? Yes, I think you're correct here. But, does this need to be mfn_pte(page_to_mfn(get_balloon_trade_page())? The pte parameter to update_va_mapping() needs to contains the machine address, right? David