Skip to contentSkip to conversation

incorrect xfree for mini-os

PATCH Applied INAKOSHI Hiroya 3 participants 4 messages Apr 2, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 incorrect xfree for mini-os

xfree in extras/mini-os/lib/xmalloc.c is incorrect.

It has to check first if the memory to free is so big as to be freed directly by free_pages.

mini-os domains crash without this patch if they don't configure vfb correctly.

extras/mini-os/lib/xmalloc.c +7 -7
diff -r db943e8d1051 extras/mini-os/lib/xmalloc.c
@@ -208,6 +208,13 @@ void xfree(const void *p)
     pad = (struct xmalloc_pad *)p - 1;
     hdr = (struct xmalloc_hdr *)((char *)p - pad->hdr_size);
 
+    /* Big allocs free directly. */
+    if ( hdr->size >= PAGE_SIZE )
+    {
+        free_pages(hdr, get_order(hdr->size));
+        return;
+    }
+
     /* We know hdr will be on same page. */
     if(((long)p & PAGE_MASK) != ((long)hdr & PAGE_MASK))
     {
@@ -220,13 +227,6 @@ void xfree(const void *p)
     {
         printk("Should not be previously freed\n");
         *(int*)0=0;
-    }
-
-    /* Big allocs free directly. */
-    if ( hdr->size >= PAGE_SIZE )
-    {
-        free_pages(hdr, get_order(hdr->size));
-        return;
     }
 
     /* Merge with other free block, or put in list. */

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

Acked-by on v1 1/1
#1 lore raw
Quoted 9 lines · INAKOSHI Hiroya, le Wed 02 Apr 2008 19:43:29 +0900, a écrit :

xfree in extras/mini-os/lib/xmalloc.c is incorrect.

It has to check first if the memory to free is so big as to be freed directly by free_pages.

mini-os domains crash without this patch if they don't configure vfb correctly.

Quoted 36 lines

diff -r db943e8d1051 extras/mini-os/lib/xmalloc.c

extras/mini-os/lib/xmalloc.c +7 -7
@@ -208,6 +208,13 @@ void xfree(const void *p)
     pad = (struct xmalloc_pad *)p - 1;
     hdr = (struct xmalloc_hdr *)((char *)p - pad->hdr_size);
 
+    /* Big allocs free directly. */
+    if ( hdr->size >= PAGE_SIZE )
+    {
+        free_pages(hdr, get_order(hdr->size));
+        return;
+    }
+
     /* We know hdr will be on same page. */
     if(((long)p & PAGE_MASK) != ((long)hdr & PAGE_MASK))
     {
@@ -220,13 +227,6 @@ void xfree(const void *p)
     {
         printk("Should not be previously freed\n");
         *(int*)0=0;
-    }
-
-    /* Big allocs free directly. */
-    if ( hdr->size >= PAGE_SIZE )
-    {
-        free_pages(hdr, get_order(hdr->size));
-        return;
     }
 
     /* Merge with other free block, or put in list. */

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

Signature · 4 lines
Samuel
* x remarque qu'avec un peu de volonté, on peut faire du code de porc
dans d'importe quel langage Turing-complet
 -+- x sur #ens-mim - codons porc -+-
Applied
#2 lore raw
Quoted 6 lines · Samuel Thibault
Quoted 5 lines · INAKOSHI Hiroya, le Wed 02 Apr 2008 19:43:29 +0900, a écrit :

xfree in extras/mini-os/lib/xmalloc.c is incorrect.

It has to check first if the memory to free is so big as to be freed directly by free_pages.

Quoted 6 lines
Quoted 4 lines

mini-os domains crash without this patch if they don't configure vfb correctly.

Applied, but shouldn't the checks that this patch skips work okay even for big allocations made via xmalloc_whole_pages()? It works okay that way in Xen itself.

-- Keir

#3 lore raw
Quoted 11 lines · Keir Fraser, le Wed 02 Apr 2008 15:45:43 +0100, a écrit :
Quoted 9 lines · Samuel Thibault
Quoted 8 lines · INAKOSHI Hiroya, le Wed 02 Apr 2008 19:43:29 +0900, a écrit :

xfree in extras/mini-os/lib/xmalloc.c is incorrect.

It has to check first if the memory to free is so big as to be freed directly by free_pages.

mini-os domains crash without this patch if they don't configure vfb correctly.

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

Applied, but shouldn't the checks that this patch skips work okay even for big allocations made via xmalloc_whole_pages()?

The "hdr will be on same page" will typically not work when aligning the big area on pages.

Samuel