[PATCH] incorrect xfree for mini-os

[PATCH] incorrect xfree for mini-os

[INAKOSHI Hiroya]

[-- Attachment #1: Type: text/plain, Size: 292 bytes --]

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.

Signed-off-by: INAKOSHI Hiroya <inakoshi.hiroya@jp.fujitsu.com>

[-- Attachment #2: minios-incorrect-xfree.patch --]
[-- Type: text/plain, Size: 942 bytes --]

diff -r db943e8d1051 extras/mini-os/lib/xmalloc.c
--- a/extras/mini-os/lib/xmalloc.c	Tue Apr 01 10:09:33 2008 +0100
+++ b/extras/mini-os/lib/xmalloc.c	Wed Apr 02 19:34:40 2008 +0900
@@ -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. */

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] incorrect xfree for mini-os

[Samuel Thibault]

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.
> 
> Signed-off-by: INAKOSHI Hiroya <inakoshi.hiroya@jp.fujitsu.com>

Acked-by: Samuel Thibault <samuel.thibault@eu.citrix.com>

> diff -r db943e8d1051 extras/mini-os/lib/xmalloc.c
> --- a/extras/mini-os/lib/xmalloc.c	Tue Apr 01 10:09:33 2008 +0100
> +++ b/extras/mini-os/lib/xmalloc.c	Wed Apr 02 19:34:40 2008 +0900
> @@ -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
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel


-- 
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 -+-

Re: [PATCH] incorrect xfree for mini-os

[Keir Fraser]

On 2/4/08 11:52, "Samuel Thibault" <samuel.thibault@eu.citrix.com> wrote:

> 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.
>> 
>> Signed-off-by: INAKOSHI Hiroya <inakoshi.hiroya@jp.fujitsu.com>
> 
> Acked-by: Samuel Thibault <samuel.thibault@eu.citrix.com>

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

Re: [PATCH] incorrect xfree for mini-os

[Samuel Thibault]

Keir Fraser, le Wed 02 Apr 2008 15:45:43 +0100, a écrit :
> On 2/4/08 11:52, "Samuel Thibault" <samuel.thibault@eu.citrix.com> wrote:
> 
> > 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.
> >> 
> >> Signed-off-by: INAKOSHI Hiroya <inakoshi.hiroya@jp.fujitsu.com>
> > 
> > Acked-by: Samuel Thibault <samuel.thibault@eu.citrix.com>
> 
> 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