Xen devel - Sep 2006 - [PATCH] Fix BUG in alloc_heap_pages

I believe it is the case that if the last element of a list is deleted
with list_del(), and then the list is scanned with list_empty(),
undefined results can occur.  The following patch fixes a BUG that
triggers on one of my blades that has 8 GB of RAM.

I believe that the failing sequence is that when alloc_heap_pages
happens to exhaust a zone list and does a list_del on the last element
and returns, and then the next call to the same function looks through
the same list and decides that it is not empty and tries to do a
list_del, which triggers the BUG I am seeing.

This patch is based on a suggestion by Jimi Xenidis, and it does indeed
fix the problem on my blade.  I have walked through the code and have
mostly convinced myself that there are no negative side effects of
calling list_del_init, but I would appreciate any comments.

Signed-off-by: Jimi Xenidis <jimix@watson.ibm.com>
Acked-by: Amos Waterland <apw@us.ibm.com>

---

 page_alloc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -r 5418062d2da8 xen/common/page_alloc.c
--- a/xen/common/page_alloc.c	Tue Sep 19 11:26:00 2006 -0500
+++ b/xen/common/page_alloc.c	Thu Sep 21 17:38:41 2006 -0400
@@ -313,7 +313,7 @@ struct page_info *alloc_heap_pages(unsig
 
  found: 
     pg = list_entry(heap[zone][i].next, struct page_info, list);
-    list_del(&pg->list);
+    list_del_init(&pg->list);
 
     /* We may have to halve the chunk a number of times. */
     while ( i != order )


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

On 25/9/06 15:48, "Amos Waterland" <apw@us.ibm.com> wrote:
> I believe that the failing sequence is that when alloc_heap_pages
> happens to exhaust a zone list and does a list_del on the last element
> and returns, and then the next call to the same function looks through
> the same list and decides that it is not empty and tries to do a
> list_del, which triggers the BUG I am seeing.
Deleting the last item from a list using list_del() leaves the list in a
well-defined state. list_empty() will return TRUE for the list.
> This patch is based on a suggestion by Jimi Xenidis, and it does indeed
> fix the problem on my blade.  I have walked through the code and have
> mostly convinced myself that there are no negative side effects of
> calling list_del_init, but I would appreciate any comments.
list_del_init() initialises the node that is deleted from the list, not the
list itself. Initialising the node to be a new empty list should not be
necessary -- callers to alloc_heap_pages() should not rely on the list-node
pointers being in any way valid.

 -- Keir



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