|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH 3 of 3] Make p2m critical sections hold a ref on the underlying mfn
xen/arch/x86/mm.c | 18 +++++++++----
xen/arch/x86/mm/mem_sharing.c | 13 +++------
xen/arch/x86/mm/p2m.c | 56 +++++++++++++++++++++++++++++++++++++++++-
xen/common/grant_table.c | 4 +-
xen/common/memory.c | 10 +++---
xen/include/asm-x86/mm.h | 3 +-
xen/include/asm-x86/p2m.h | 10 ++++++-
xen/include/xen/paging.h | 2 +-
xen/include/xen/tmem_xen.h | 2 +-
9 files changed, 89 insertions(+), 29 deletions(-)
For translated domains, critical sections demarcated by a
get_gfn/put_gfn pair will hold an additional ref on the
underlying mfn.
This requires keeping tabs on special manipulations that
change said mfn:
- physmap remove page
- sharing
- steal page
Signed-off-by: Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>
diff -r 6203a0549d8a -r 4b684fa74636 xen/arch/x86/mm.c
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4128,7 +4128,7 @@ static int replace_grant_p2m_mapping(
type, mfn_x(old_mfn), frame);
return GNTST_general_error;
}
- guest_physmap_remove_page(d, gfn, frame, 0);
+ guest_physmap_remove_page(d, gfn, frame, 0, HOLDING_GFN);
put_gfn(d, gfn);
return GNTST_okay;
@@ -4248,8 +4248,10 @@ int donate_page(
}
int steal_page(
- struct domain *d, struct page_info *page, unsigned int memflags)
+ struct domain *d, struct page_info *page, unsigned int memflags,
+ int holding_gfn)
{
+ unsigned count;
unsigned long x, y;
bool_t drop_dom_ref = 0;
@@ -4261,11 +4263,14 @@ int steal_page(
/*
* We require there is just one reference (PGC_allocated). We temporarily
* drop this reference now so that we can safely swizzle the owner.
+ * If, however, this is invoked by a caller holding the p2m entry, there
+ * will be another reference.
*/
+ count = (holding_gfn) ? 1 : 2;
y = page->count_info;
do {
x = y;
- if ( (x & (PGC_count_mask|PGC_allocated)) != (1 | PGC_allocated) )
+ if ( (x & (PGC_count_mask|PGC_allocated)) != (count | PGC_allocated) )
goto fail;
y = cmpxchg(&page->count_info, x, x & ~PGC_count_mask);
} while ( y != x );
@@ -4276,7 +4281,7 @@ int steal_page(
do {
x = y;
BUG_ON((x & (PGC_count_mask|PGC_allocated)) != PGC_allocated);
- } while ( (y = cmpxchg(&page->count_info, x, x | 1)) != x );
+ } while ( (y = cmpxchg(&page->count_info, x, x | count)) != x );
/* Unlink from original owner. */
if ( !(memflags & MEMF_no_refcount) && !--d->tot_pages )
@@ -4779,7 +4784,8 @@ long arch_memory_op(int op, XEN_GUEST_HA
{
if ( is_xen_heap_mfn(prev_mfn) )
/* Xen heap frames are simply unhooked from this phys slot. */
- guest_physmap_remove_page(d, xatp.gpfn, prev_mfn, 0);
+ guest_physmap_remove_page(d, xatp.gpfn, prev_mfn, 0,
+ HOLDING_GFN);
else
/* Normal domain memory is freed, to avoid leaking memory. */
guest_remove_page(d, xatp.gpfn);
@@ -4791,7 +4797,7 @@ long arch_memory_op(int op, XEN_GUEST_HA
gpfn = get_gpfn_from_mfn(mfn);
ASSERT( gpfn != SHARED_M2P_ENTRY );
if ( gpfn != INVALID_M2P_ENTRY )
- guest_physmap_remove_page(d, gpfn, mfn, 0);
+ guest_physmap_remove_page(d, gpfn, mfn, 0, (gpfn == gfn));
/* Map at new location. */
rc = guest_physmap_add_page(d, xatp.gpfn, mfn, 0);
diff -r 6203a0549d8a -r 4b684fa74636 xen/arch/x86/mm/mem_sharing.c
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -561,7 +561,7 @@ int mem_sharing_share_pages(shr_handle_t
list_del(&gfn->list);
d = get_domain_by_id(gfn->domain);
BUG_ON(!d);
- BUG_ON(set_shared_p2m_entry(d, gfn->gfn, se->mfn) == 0);
+ BUG_ON(set_shared_p2m_entry(d, gfn->gfn, se->mfn, 0) == 0);
put_domain(d);
list_add(&gfn->list, &se->gfns);
put_page_and_type(cpage);
@@ -670,14 +670,9 @@ gfn_found:
unmap_domain_page(s);
unmap_domain_page(t);
- /* NOTE: set_shared_p2m_entry will switch the underlying mfn. If
- * we do get_page withing get_gfn, the correct sequence here
- * should be
- get_page(page);
- put_page(old_page);
- * so that the ref to the old page is dropped, and a ref to
- * the new page is obtained to later be dropped in put_gfn */
- BUG_ON(set_shared_p2m_entry(d, gfn, page_to_mfn(page)) == 0);
+ /* NOTE: set_shared_p2m_entry will swap the underlying mfn and the refs.
+ * It is safe to call put_gfn as usual after this. */
+ BUG_ON(set_shared_p2m_entry(d, gfn, page_to_mfn(page), HOLDING_GFN) == 0);
put_page_and_type(old_page);
private_page_found:
diff -r 6203a0549d8a -r 4b684fa74636 xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -182,6 +182,13 @@ mfn_t gfn_to_mfn_type_p2m(struct p2m_dom
}
#endif
+ /* Do a get page to get one additional ref on the mfn */
+ if ( mfn_valid(mfn) )
+ {
+ struct page_info *pg = mfn_to_page(mfn);
+ BUG_ON( !page_get_owner_and_reference(pg) );
+ }
+
return mfn;
}
@@ -195,6 +202,21 @@ void put_gfn(struct domain *d, unsigned
ASSERT(p2m_locked_by_me(p2m));
+ {
+ p2m_access_t a;
+ p2m_type_t t;
+ mfn_t mfn = p2m->get_entry(p2m, gfn, &t, &a,
+ p2m_query, NULL);
+
+ if ( mfn_valid(mfn) )
+ {
+#ifdef __x86_64__
+ if (likely( !(p2m_is_broken(t)) ))
+#endif
+ put_page(mfn_to_page(mfn));
+ }
+ }
+
p2m_unlock(p2m);
}
@@ -416,6 +438,28 @@ void p2m_final_teardown(struct domain *d
p2m_teardown_nestedp2m(d);
}
+/* If the caller has done get_gfn on this entry, then it has a ref on the
+ * old mfn. Swap the refs so put_gfn puts the appropriate ref */
+static inline void __p2m_swap_entry_refs(struct p2m_domain *p2m,
+ unsigned long gfn, mfn_t mfn)
+{
+ p2m_type_t t;
+ p2m_access_t a;
+ mfn_t omfn;
+
+ if ( !paging_mode_translate(p2m->domain) )
+ return;
+
+ ASSERT(gfn_locked_by_me(p2m, gfn));
+
+ omfn = p2m->get_entry(p2m, gfn, &t, &a, p2m_query, NULL);
+
+ if ( mfn_valid(mfn) )
+ BUG_ON( !page_get_owner_and_reference(mfn_to_page(mfn)) );
+
+ if ( mfn_valid(omfn) )
+ put_page(mfn_to_page(omfn));
+}
static void
p2m_remove_page(struct p2m_domain *p2m, unsigned long gfn, unsigned long mfn,
@@ -451,11 +495,14 @@ p2m_remove_page(struct p2m_domain *p2m,
void
guest_physmap_remove_page(struct domain *d, unsigned long gfn,
- unsigned long mfn, unsigned int page_order)
+ unsigned long mfn, unsigned int page_order,
+ int holding_gfn)
{
struct p2m_domain *p2m = p2m_get_hostp2m(d);
p2m_lock(p2m);
audit_p2m(p2m, 1);
+ if (holding_gfn)
+ __p2m_swap_entry_refs(p2m, gfn, _mfn(INVALID_MFN));
p2m_remove_page(p2m, gfn, mfn, page_order);
audit_p2m(p2m, 1);
p2m_unlock(p2m);
@@ -713,7 +760,8 @@ out:
}
int
-set_shared_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn)
+set_shared_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn,
+ int holding_gfn)
{
struct p2m_domain *p2m = p2m_get_hostp2m(d);
int rc = 0;
@@ -730,6 +778,10 @@ set_shared_p2m_entry(struct domain *d, u
* sharable first */
ASSERT(p2m_is_shared(ot));
ASSERT(mfn_valid(omfn));
+
+ if ( holding_gfn )
+ __p2m_swap_entry_refs(p2m, gfn, mfn);
+
/* XXX: M2P translations have to be handled properly for shared pages */
set_gpfn_from_mfn(mfn_x(omfn), INVALID_M2P_ENTRY);
diff -r 6203a0549d8a -r 4b684fa74636 xen/common/grant_table.c
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1497,7 +1497,7 @@ gnttab_transfer(
goto copyback;
}
- if ( steal_page(d, page, 0) < 0 )
+ if ( steal_page(d, page, 0, HOLDING_GFN) < 0 )
{
put_gfn(d, gop.mfn);
gop.status = GNTST_bad_page;
@@ -1505,7 +1505,7 @@ gnttab_transfer(
}
#ifndef __ia64__ /* IA64 implicitly replaces the old page in steal_page(). */
- guest_physmap_remove_page(d, gop.mfn, mfn, 0);
+ guest_physmap_remove_page(d, gop.mfn, mfn, 0, HOLDING_GFN);
#endif
flush_tlb_mask(d->domain_dirty_cpumask);
diff -r 6203a0549d8a -r 4b684fa74636 xen/common/memory.c
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -165,7 +165,7 @@ int guest_remove_page(struct domain *d,
mfn = mfn_x(get_gfn(d, gmfn, &p2mt));
if ( unlikely(p2m_is_paging(p2mt)) )
{
- guest_physmap_remove_page(d, gmfn, mfn, 0);
+ guest_physmap_remove_page(d, gmfn, mfn, 0, HOLDING_GFN);
p2m_mem_paging_drop_page(d, gmfn);
put_gfn(d, gmfn);
return 1;
@@ -188,7 +188,7 @@ int guest_remove_page(struct domain *d,
if(p2m_is_shared(p2mt))
{
put_page_and_type(page);
- guest_physmap_remove_page(d, gmfn, mfn, 0);
+ guest_physmap_remove_page(d, gmfn, mfn, 0, HOLDING_GFN);
put_gfn(d, gmfn);
return 1;
}
@@ -207,7 +207,7 @@ int guest_remove_page(struct domain *d,
if ( test_and_clear_bit(_PGC_allocated, &page->count_info) )
put_page(page);
- guest_physmap_remove_page(d, gmfn, mfn, 0);
+ guest_physmap_remove_page(d, gmfn, mfn, 0, HOLDING_GFN);
put_gfn(d, gmfn);
put_page(page);
@@ -387,7 +387,7 @@ static long memory_exchange(XEN_GUEST_HA
page = mfn_to_page(mfn);
- if ( unlikely(steal_page(d, page, MEMF_no_refcount)) )
+ if ( unlikely(steal_page(d, page, MEMF_no_refcount,
HOLDING_GFN)) )
{
put_gfn(d, gmfn + k);
rc = -EINVAL;
@@ -427,7 +427,7 @@ static long memory_exchange(XEN_GUEST_HA
gfn = mfn_to_gmfn(d, mfn);
/* Pages were unshared above */
BUG_ON(SHARED_M2P(gfn));
- guest_physmap_remove_page(d, gfn, mfn, 0);
+ guest_physmap_remove_page(d, gfn, mfn, 0, 0);
put_page(page);
}
diff -r 6203a0549d8a -r 4b684fa74636 xen/include/asm-x86/mm.h
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -578,7 +578,8 @@ int compat_arch_memory_op(int op, XEN_GU
int compat_subarch_memory_op(int op, XEN_GUEST_HANDLE(void));
int steal_page(
- struct domain *d, struct page_info *page, unsigned int memflags);
+ struct domain *d, struct page_info *page, unsigned int memflags,
+ int holding_gfn);
int donate_page(
struct domain *d, struct page_info *page, unsigned int memflags);
int page_make_sharable(struct domain *d,
diff -r 6203a0549d8a -r 4b684fa74636 xen/include/asm-x86/p2m.h
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -353,6 +353,10 @@ static inline unsigned long get_gfn_unty
/* Will release the p2m_lock and put the page behind this mapping. */
void put_gfn(struct domain *d, unsigned long gfn);
+/* Operations that change the underlying mfn in a p2m entry need to be
+ * told whether the caller is holding the current gfn */
+#define HOLDING_GFN 1
+
/* The intent is to have the caller not worry about put_gfn. They apply to
* very specific situations: debug printk's, dumps during a domain crash,
* or to peek at a p2m entry/type. Caller is not holding the p2m entry
@@ -410,7 +414,8 @@ static inline int guest_physmap_add_page
/* Remove a page from a domain's p2m table */
void guest_physmap_remove_page(struct domain *d,
unsigned long gfn,
- unsigned long mfn, unsigned int page_order);
+ unsigned long mfn, unsigned int page_order,
+ int holding_gfn);
/* Set a p2m range as populate-on-demand */
int guest_physmap_mark_populate_on_demand(struct domain *d, unsigned long gfn,
@@ -471,7 +476,8 @@ p2m_pod_offline_or_broken_replace(struct
#ifdef __x86_64__
/* Modify p2m table for shared gfn */
-int set_shared_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn);
+int set_shared_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn,
+ int holding_gfn);
/* Check if a nominated gfn is valid to be paged out */
int p2m_mem_paging_nominate(struct domain *d, unsigned long gfn);
diff -r 6203a0549d8a -r 4b684fa74636 xen/include/xen/paging.h
--- a/xen/include/xen/paging.h
+++ b/xen/include/xen/paging.h
@@ -21,7 +21,7 @@
#define paging_mode_translate(d) (0)
#define paging_mode_external(d) (0)
#define guest_physmap_add_page(d, p, m, o) (0)
-#define guest_physmap_remove_page(d, p, m, o) ((void)0)
+#define guest_physmap_remove_page(d, p, m, o, h) ((void)0)
#endif
diff -r 6203a0549d8a -r 4b684fa74636 xen/include/xen/tmem_xen.h
--- a/xen/include/xen/tmem_xen.h
+++ b/xen/include/xen/tmem_xen.h
@@ -198,7 +198,7 @@ static inline void _tmh_free_page_thispo
struct domain *d = page_get_owner(pi);
ASSERT(IS_VALID_PAGE(pi));
- if ( (d == NULL) || steal_page(d,pi,0) == 0 )
+ if ( (d == NULL) || steal_page(d,pi,0,0) == 0 )
tmh_page_list_put(pi);
else
{
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |