Tim Deegan
2010-Dec-15 11:59 UTC
[Xen-devel] [PATCH 0 of 3] Fix up locking in log-dirty code
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Tim Deegan
2010-Dec-15 11:59 UTC
[Xen-devel] [PATCH 1 of 3] x86/mm: move mfn_is_dirty along with the rest of the log-dirty code
# HG changeset patch
# User Tim Deegan <Tim.Deegan@citrix.com>
# Date 1292410310 0
# Node ID dd5fbb5b7a43c7509a08849b65c0d1921af5fe05
# Parent 01f3b350902385627d1fa9e8cd1c231953e7610c
x86/mm: move mfn_is_dirty along with the rest of the log-dirty code
Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>
diff -r 01f3b3509023 -r dd5fbb5b7a43 xen/arch/x86/mm/paging.c
--- a/xen/arch/x86/mm/paging.c Wed Dec 15 10:27:18 2010 +0000
+++ b/xen/arch/x86/mm/paging.c Wed Dec 15 10:51:50 2010 +0000
@@ -304,6 +304,62 @@ void paging_mark_dirty(struct domain *d,
log_dirty_unlock(d);
}
+
+/* Is this guest page dirty? */
+int paging_mfn_is_dirty(struct domain *d, mfn_t gmfn)
+{
+ unsigned long pfn;
+ mfn_t mfn, *l4, *l3, *l2;
+ unsigned long *l1;
+ int rv;
+
+ ASSERT(paging_mode_log_dirty(d));
+
+ /* We /really/ mean PFN here, even for non-translated guests. */
+ pfn = get_gpfn_from_mfn(mfn_x(gmfn));
+ /* Page sharing not supported for shadow domains */
+ BUG_ON(SHARED_M2P(pfn));
+ if ( unlikely(!VALID_M2P(pfn)) )
+ return 0;
+
+ if ( d->arch.paging.log_dirty.failed_allocs > 0 )
+ /* If we have any failed allocations our dirty log is bogus.
+ * Since we can''t signal an error here, be conservative and
+ * report "dirty" in this case. (The only current caller,
+ * _sh_propagate, leaves known-dirty pages writable, preventing
+ * subsequent dirty-logging faults from them.)
+ */
+ return 1;
+
+ l4 = paging_map_log_dirty_bitmap(d);
+ if ( !l4 )
+ return 0;
+
+ mfn = l4[L4_LOGDIRTY_IDX(pfn)];
+ unmap_domain_page(l4);
+ if ( !mfn_valid(mfn) )
+ return 0;
+
+ l3 = map_domain_page(mfn_x(mfn));
+ mfn = l3[L3_LOGDIRTY_IDX(pfn)];
+ unmap_domain_page(l3);
+ if ( !mfn_valid(mfn) )
+ return 0;
+
+ l2 = map_domain_page(mfn_x(mfn));
+ mfn = l2[L2_LOGDIRTY_IDX(pfn)];
+ unmap_domain_page(l2);
+ if ( !mfn_valid(mfn) )
+ return 0;
+
+ l1 = map_domain_page(mfn_x(mfn));
+ rv = test_bit(L1_LOGDIRTY_IDX(pfn), l1);
+ unmap_domain_page(l1);
+
+ return rv;
+}
+
+
/* Read a domain''s log-dirty bitmap and stats. If the operation is a
CLEAN,
* clear the bitmap and stats as well. */
int paging_log_dirty_op(struct domain *d, struct xen_domctl_shadow_op *sc)
diff -r 01f3b3509023 -r dd5fbb5b7a43 xen/arch/x86/mm/shadow/multi.c
--- a/xen/arch/x86/mm/shadow/multi.c Wed Dec 15 10:27:18 2010 +0000
+++ b/xen/arch/x86/mm/shadow/multi.c Wed Dec 15 10:51:50 2010 +0000
@@ -657,7 +657,7 @@ _sh_propagate(struct vcpu *v,
if ( mfn_valid(target_mfn) ) {
if ( ft & FETCH_TYPE_WRITE )
paging_mark_dirty(d, mfn_x(target_mfn));
- else if ( !sh_mfn_is_dirty(d, target_mfn) )
+ else if ( !paging_mfn_is_dirty(d, target_mfn) )
sflags &= ~_PAGE_RW;
}
}
diff -r 01f3b3509023 -r dd5fbb5b7a43 xen/arch/x86/mm/shadow/private.h
--- a/xen/arch/x86/mm/shadow/private.h Wed Dec 15 10:27:18 2010 +0000
+++ b/xen/arch/x86/mm/shadow/private.h Wed Dec 15 10:51:50 2010 +0000
@@ -568,67 +568,6 @@ sh_unmap_domain_page_global(void *p)
unmap_domain_page_global(p);
}
-/******************************************************************************
- * Log-dirty mode bitmap handling
- */
-
-extern void sh_mark_dirty(struct domain *d, mfn_t gmfn);
-
-static inline int
-sh_mfn_is_dirty(struct domain *d, mfn_t gmfn)
-/* Is this guest page dirty? Call only in log-dirty mode. */
-{
- unsigned long pfn;
- mfn_t mfn, *l4, *l3, *l2;
- unsigned long *l1;
- int rv;
-
- ASSERT(shadow_mode_log_dirty(d));
-
- /* We /really/ mean PFN here, even for non-translated guests. */
- pfn = get_gpfn_from_mfn(mfn_x(gmfn));
- /* Page sharing not supported for shadow domains */
- BUG_ON(SHARED_M2P(pfn));
- if ( unlikely(!VALID_M2P(pfn)) )
- return 0;
-
- if ( d->arch.paging.log_dirty.failed_allocs > 0 )
- /* If we have any failed allocations our dirty log is bogus.
- * Since we can''t signal an error here, be conservative and
- * report "dirty" in this case. (The only current caller,
- * _sh_propagate, leaves known-dirty pages writable, preventing
- * subsequent dirty-logging faults from them.)
- */
- return 1;
-
- l4 = paging_map_log_dirty_bitmap(d);
- if ( !l4 )
- return 0;
-
- mfn = l4[L4_LOGDIRTY_IDX(pfn)];
- unmap_domain_page(l4);
- if ( !mfn_valid(mfn) )
- return 0;
-
- l3 = map_domain_page(mfn_x(mfn));
- mfn = l3[L3_LOGDIRTY_IDX(pfn)];
- unmap_domain_page(l3);
- if ( !mfn_valid(mfn) )
- return 0;
-
- l2 = map_domain_page(mfn_x(mfn));
- mfn = l2[L2_LOGDIRTY_IDX(pfn)];
- unmap_domain_page(l2);
- if ( !mfn_valid(mfn) )
- return 0;
-
- l1 = map_domain_page(mfn_x(mfn));
- rv = test_bit(L1_LOGDIRTY_IDX(pfn), l1);
- unmap_domain_page(l1);
-
- return rv;
-}
-
/**************************************************************************/
/* Shadow-page refcounting. */
diff -r 01f3b3509023 -r dd5fbb5b7a43 xen/include/asm-x86/paging.h
--- a/xen/include/asm-x86/paging.h Wed Dec 15 10:27:18 2010 +0000
+++ b/xen/include/asm-x86/paging.h Wed Dec 15 10:51:50 2010 +0000
@@ -161,6 +161,9 @@ void paging_log_dirty_init(struct domain
/* mark a page as dirty */
void paging_mark_dirty(struct domain *d, unsigned long guest_mfn);
+/* is this guest page dirty? */
+int paging_mfn_is_dirty(struct domain *d, mfn_t gmfn);
+
/*
* Log-dirty radix tree indexing:
* All tree nodes are PAGE_SIZE bytes, mapped on-demand.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Tim Deegan
2010-Dec-15 11:59 UTC
[Xen-devel] [PATCH 2 of 3] x86/mm: fix up paging_mfn_is_dirty()
# HG changeset patch
# User Tim Deegan <Tim.Deegan@citrix.com>
# Date 1292411012 0
# Node ID 5bfe5ec812ff7019d4dcb5a7574926027e03f984
# Parent dd5fbb5b7a43c7509a08849b65c0d1921af5fe05
x86/mm: fix up paging_mfn_is_dirty()
Add locking, and don''t allocate the top-level page if it''s not
there.
Also gets rid of the default-to-1 case if there have been failed
allocations because the safer thing is actually to return 0 and avoid
modifying an un-dirtied page.
Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>
diff -r dd5fbb5b7a43 -r 5bfe5ec812ff xen/arch/x86/mm/paging.c
--- a/xen/arch/x86/mm/paging.c Wed Dec 15 10:51:50 2010 +0000
+++ b/xen/arch/x86/mm/paging.c Wed Dec 15 11:03:32 2010 +0000
@@ -311,51 +311,45 @@ int paging_mfn_is_dirty(struct domain *d
unsigned long pfn;
mfn_t mfn, *l4, *l3, *l2;
unsigned long *l1;
- int rv;
+ int rv = 0;
+ log_dirty_lock(d);
ASSERT(paging_mode_log_dirty(d));
/* We /really/ mean PFN here, even for non-translated guests. */
pfn = get_gpfn_from_mfn(mfn_x(gmfn));
- /* Page sharing not supported for shadow domains */
- BUG_ON(SHARED_M2P(pfn));
- if ( unlikely(!VALID_M2P(pfn)) )
- return 0;
-
- if ( d->arch.paging.log_dirty.failed_allocs > 0 )
- /* If we have any failed allocations our dirty log is bogus.
- * Since we can''t signal an error here, be conservative and
- * report "dirty" in this case. (The only current caller,
- * _sh_propagate, leaves known-dirty pages writable, preventing
- * subsequent dirty-logging faults from them.)
- */
- return 1;
+ /* Shared pages are always read-only; invalid pages can''t be
dirty. */
+ if ( unlikely(SHARED_M2P(pfn) || !VALID_M2P(pfn)) )
+ goto out;
- l4 = paging_map_log_dirty_bitmap(d);
- if ( !l4 )
- return 0;
+ mfn = d->arch.paging.log_dirty.top;
+ if ( !mfn_valid(mfn) )
+ goto out;
+ l4 = map_domain_page(mfn_x(mfn));
mfn = l4[L4_LOGDIRTY_IDX(pfn)];
unmap_domain_page(l4);
if ( !mfn_valid(mfn) )
- return 0;
+ goto out;
l3 = map_domain_page(mfn_x(mfn));
mfn = l3[L3_LOGDIRTY_IDX(pfn)];
unmap_domain_page(l3);
if ( !mfn_valid(mfn) )
- return 0;
+ goto out;
l2 = map_domain_page(mfn_x(mfn));
mfn = l2[L2_LOGDIRTY_IDX(pfn)];
unmap_domain_page(l2);
if ( !mfn_valid(mfn) )
- return 0;
+ goto out;
l1 = map_domain_page(mfn_x(mfn));
rv = test_bit(L1_LOGDIRTY_IDX(pfn), l1);
unmap_domain_page(l1);
+out:
+ log_dirty_unlock(d);
return rv;
}
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Tim Deegan
2010-Dec-15 11:59 UTC
[Xen-devel] [PATCH 3 of 3] x86/mm: make paging_map_log_dirty_bitmap() static
# HG changeset patch
# User Tim Deegan <Tim.Deegan@citrix.com>
# Date 1292411070 0
# Node ID b04de0671fd49adbf9d88d2fdea38d9c1f5c822e
# Parent 5bfe5ec812ff7019d4dcb5a7574926027e03f984
x86/mm: make paging_map_log_dirty_bitmap() static
now that its only caller outside paging.c has been removed.
Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>
diff -r 5bfe5ec812ff -r b04de0671fd4 xen/arch/x86/mm/paging.c
--- a/xen/arch/x86/mm/paging.c Wed Dec 15 11:03:32 2010 +0000
+++ b/xen/arch/x86/mm/paging.c Wed Dec 15 11:04:30 2010 +0000
@@ -132,7 +132,8 @@ static mfn_t paging_new_log_dirty_node(s
return mfn;
}
-mfn_t *paging_map_log_dirty_bitmap(struct domain *d)
+/* get the top of the log-dirty bitmap trie, allocating if necessary */
+static mfn_t *paging_map_log_dirty_bitmap(struct domain *d)
{
mfn_t *mapping;
diff -r 5bfe5ec812ff -r b04de0671fd4 xen/include/asm-x86/paging.h
--- a/xen/include/asm-x86/paging.h Wed Dec 15 11:03:32 2010 +0000
+++ b/xen/include/asm-x86/paging.h Wed Dec 15 11:04:30 2010 +0000
@@ -134,9 +134,6 @@ struct paging_mode {
/*****************************************************************************
* Log dirty code */
-/* get the top of the log-dirty bitmap trie, allocating if necessary */
-mfn_t *paging_map_log_dirty_bitmap(struct domain *d);
-
/* free log dirty bitmap resource */
void paging_free_log_dirty_bitmap(struct domain *d);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Keir Fraser
2010-Dec-15 12:15 UTC
Re: [Xen-devel] [PATCH 0 of 3] Fix up locking in log-dirty code
Is this needed for 4.0.2 also? If so you''ll need to submit a manual backport. -- Keir On 15/12/2010 11:59, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:> > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Tim Deegan
2010-Dec-15 12:21 UTC
Re: [Xen-devel] [PATCH 0 of 3] Fix up locking in log-dirty code
At 12:15 +0000 on 15 Dec (1292415345), Keir Fraser wrote:> Is this needed for 4.0.2 also? If so you''ll need to submit a manual > backport.No, AFAICS it shouldn''t be needed for 4.0.2; the bug was introduced when I tinkered with the log-dirty memory allocation code recently. Cheers, Tim. -- Tim Deegan <Tim.Deegan@citrix.com> Principal Software Engineer, Xen Platform Team Citrix Systems UK Ltd. (Company #02937203, SL9 0BG) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel