[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 7/8] xen/arm: Set foreign page type to p2m_map_foreign





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@xxxxxxxxxxxxx>. 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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.