From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH 7/8] xen/arm: Set foreign page type to p2m_map_foreign Date: Thu, 05 Dec 2013 17:39:06 +0000 Message-ID: <52A0BA3A.3090105@linaro.org> References: <1386258131-755-1-git-send-email-julien.grall@linaro.org> <1386258131-755-8-git-send-email-julien.grall@linaro.org> <1386261264.20047.117.camel@kazak.uk.xensource.com> <52A0ACCC.3030903@linaro.org> <1386262443.20047.126.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VoctK-0002Ty-L7 for xen-devel@lists.xenproject.org; Thu, 05 Dec 2013 17:39:10 +0000 Received: by mail-ea0-f180.google.com with SMTP id f15so11623975eak.11 for ; Thu, 05 Dec 2013 09:39:09 -0800 (PST) In-Reply-To: <1386262443.20047.126.camel@kazak.uk.xensource.com> 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: xen-devel@lists.xenproject.org, tim@xen.org, stefano.stabellini@citrix.com, patches@linaro.org List-Id: xen-devel@lists.xenproject.org On 12/05/2013 04:54 PM, Ian Campbell wrote: > On Thu, 2013-12-05 at 16:41 +0000, Julien Grall wrote: > >>>> >>>> switch ( space ) >>>> { >>>> @@ -1019,8 +1020,8 @@ static int xenmem_add_to_physmap_one( >>>> break; >>>> case XENMAPSPACE_gmfn_foreign: >>>> { >>>> - paddr_t maddr; >>>> struct domain *od; >>>> + struct page_info *page; >>>> od = rcu_lock_domain_by_any_id(foreign_domid); >>>> if ( od == NULL ) >>>> return -ESRCH; >>>> @@ -1032,15 +1033,18 @@ static int xenmem_add_to_physmap_one( >>>> return rc; >>>> } >>>> >>>> - maddr = p2m_lookup(od, pfn_to_paddr(idx)); >>>> - if ( maddr == INVALID_PADDR ) >>>> + /* Take refcount to the foreign domain page. >>> >>> "on the foreign" (and maybe "domain's" or just "foreign page" >>> >>>> + * Refcount will be release in XENMEM_remove_from_physmap */ >>> >>> "will be released". >>> >>> The refcount will also be removed by p2m_teardown. That probably needs >>> changing to do an actual walk of the p2m tearing things down, which is a >>> pain. >>> >>> Stefano's now obsolete dma pinning series had a patch which added a >>> fairly generic walker in it which might be reusable. >> >> Do you have a link to this patch? > > I was afraid you'd say that... > /scrobbles > > "xen/arm: introduce a generic p2m walker and use it in p2m_lookup". v6 > is msgid > <1380298560-29352-2-git-send-email-stefano.stabellini@eu.citrix.com>. I > don't recall if that was the last one though. Actually the generic walker will not be usefull for p2m_teardown. It seems to be an extension of p2m_lookup, so will only search a specific gfn. >> >>> The walk might need to support continuations though. >>> >>> I wonder if this ref ought to be taken in create_p2m_entries for all >>> entries and not just foreign ones, and then released in the appropriate >>> places. >> >> If I'm not mistaken, Xen already takes a ref when the page is allocated >> for the domain. Why would we need to take another ref for these pages? > > There's a nice symmetry to all p2m entries taking a ref count, and it > makes the teardown a bit simpler since you can just drop the ref every > time without worrying about the type. If I'm not mistaken, even x86 code doesn't have 2 ref for each page. Or I didn't see where the ref is taken/release ... In any case, walk the whole p2m for looking for present page seems a bit tough and slow to release memory for a domain. I'm wondering if we can have a list of foreign page, and browse it when the domain is destroyed. -- Julien Grall