Skip to contentSkip to conversation

fix c/s 18720 & 18731

PATCH In review Jan Beulich 2 participants 3 messages Oct 29, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fix c/s 18720 & 18731

Entirely removing the tail of free_page_type() wasn't correct - the type reference must still be dropped for partial pages.

Admittedly, I'm not sure the flush time stamp update is also needed here, but it seems safer to also copy it from free_page_type()'s former tail.

xen/arch/x86/domain.c +11 -0
Index: 2008-10-27/xen/arch/x86/domain.c
===================================================================
@@ -42,6 +42,7 @@
 #include <asm/mpspec.h>
 #include <asm/ldt.h>
 #include <asm/hypercall.h>
+#include <asm/shadow.h>
 #include <asm/hvm/hvm.h>
 #include <asm/hvm/support.h>
 #include <asm/debugreg.h>
@@ -1687,6 +1688,16 @@ static int relinquish_memory(
             {
                 if ( free_page_type(page, x, 0) != 0 )
                     BUG();
+
+                if ( !(shadow_mode_enabled(d) &&
+                       (page->count_info & PGC_page_table)) )
+                    page->tlbflush_timestamp = tlbflush_current_time();
+
+                if ( x & PGT_partial )
+                {
+                    wmb();
+                    page->u.inuse.type_info--;
+                }
                 break;
             }
         }
#1 lore raw

Okay. This is simply because PGT_partial holds a type reference?

The patch can be much simplified because the tlbflush_timestamp is not required (this domain will never run again) and because shadow mode has already been torn down. I'll simplify and apply.

-- Keir

Quoted 46 lines · Jan Beulich

Entirely removing the tail of free_page_type() wasn't correct - the type reference must still be dropped for partial pages.

Admittedly, I'm not sure the flush time stamp update is also needed here, but it seems safer to also copy it from free_page_type()'s former tail.

Index: 2008-10-27/xen/arch/x86/domain.c
===================================================================

xen/arch/x86/domain.c 2008-10-28 15:31:15.000000000 +0100 renamed from xen/arch/x86/domain.c 2008-10-28 15:30:37.000000000 +0100 +11 -0
@@ -42,6 +42,7 @@
 #include <asm/mpspec.h>
 #include <asm/ldt.h>
 #include <asm/hypercall.h>
+#include <asm/shadow.h>
 #include <asm/hvm/hvm.h>
 #include <asm/hvm/support.h>
 #include <asm/debugreg.h>
@@ -1687,6 +1688,16 @@ static int relinquish_memory(
             {
                 if ( free_page_type(page, x, 0) != 0 )
                     BUG();
+
+                if ( !(shadow_mode_enabled(d) &&
+                       (page->count_info & PGC_page_table)) )
+                    page->tlbflush_timestamp = tlbflush_current_time();
+
+                if ( x & PGT_partial )
+                {
+                    wmb();
+                    page->u.inuse.type_info--;
+                }
                 break;
             }
         }

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#2 lore raw
Quoted 2 lines
Quoted 1 line
Quoted 1 line

Keir Fraser <[email protected]> 29.10.08 12:55 >>>

Okay. This is simply because PGT_partial holds a type reference?

Yes.

Quoted 3 lines

The patch can be much simplified because the tlbflush_timestamp is not required (this domain will never run again) and because shadow mode has already been torn down. I'll simplify and apply.

Understood.

Thanks, Jan