WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH] IDE BMDMAState structure corruption with DMA_MULTI_T

To: xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: [Xen-devel] [PATCH] IDE BMDMAState structure corruption with DMA_MULTI_THREAD
From: "Richard W.M. Jones" <rjones@xxxxxxxxxx>
Date: Tue, 15 May 2007 22:26:29 +0100
Cc: Dan Berrange <berrange@xxxxxxxxxx>
Delivery-date: Tue, 15 May 2007 14:24:34 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Thunderbird 1.5.0.10 (X11/20070302)
As I reported here:
http://lists.xensource.com/archives/html/xen-devel/2007-05/msg00492.html
I was experiencing qemu-dm segfaulting when trying to install FreeBSD 32 fullvirt on a heavily loaded machine.

Dan Berrange and I tracked this down today to the BMDMAState structure being corrupted when a second DMA request was initiated by the guest before the first one had been completed. Because the DMA thread and the main thread share a pointer to a single BMDMAState, bm->dma_cb is set to NULL by the main thread, and later the DMA thread jumps to this address (in dma_thread_loop, at the line 'len1 = bm->dma_cb(bm->ide_if, prd.addr, len);').

The attached patch corrects this by passing the whole structure between the threads, rather than a pointer to the structure (5 words rather than 1, so there is a small amount of extra overhead).

With this patch I have been able to complete the FreeBSD FV install under load successfully.

Rich.

--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom.  Registered in
England and Wales under Company Registration No. 03798903
--- tools/ioemu/hw/ide.c.old    2007-05-15 14:02:34.000000000 +0100
+++ tools/ioemu/hw/ide.c        2007-05-15 19:25:06.000000000 +0100
@@ -402,10 +402,36 @@
 static void ide_dma_loop(BMDMAState *bm);
 static void dma_thread_loop(BMDMAState *bm);
 
+static int
+really_read (int fd, void *buf, size_t size)
+{
+    int r;
+
+again:
+    r = read (fd, buf, size);
+    if (r <= 0 || r == size) return r;
+    buf += r;
+    size -= r;
+    goto again;
+}
+
+static int
+really_write (int fd, void *buf, size_t size)
+{
+    int r;
+
+again:
+    r = write (fd, buf, size);
+    if (r == -1 || r == size) return r;
+    buf += r;
+    size -= r;
+    goto again;
+}
+
 extern int suspend_requested;
 static void *dma_thread_func(void* opaque)
 {
-    BMDMAState* req;
+    BMDMAState req;
     fd_set fds;
     int rv, nfds = file_pipes[0] + 1;
     struct timeval tm;
@@ -420,9 +446,12 @@
         rv = select(nfds, &fds, NULL, NULL, &tm);
         
         if (rv != 0) {
-            if (read(file_pipes[0], &req, sizeof(req)) == 0)
+            rv = really_read(file_pipes[0], &req, sizeof(req));
+           if (rv <= 0) {
+               if (rv == -1) perror ("qemu-dm: read");
                 return NULL;
-            dma_thread_loop(req);
+           }
+            dma_thread_loop(&req);
         } else {
             if (suspend_requested)  {
                 /* Need to tidy up the DMA thread so that we don't end up 
@@ -2371,7 +2400,8 @@
 #ifdef DMA_MULTI_THREAD
 static void ide_dma_loop(BMDMAState *bm)
 {
-    write(file_pipes[1], &bm, sizeof(bm));
+    if (really_write(file_pipes[1], bm, sizeof(*bm)) == -1)
+       perror ("qemu-dm: write");
 }
 static void dma_thread_loop(BMDMAState *bm)
 #else  /* DMA_MULTI_THREAD */

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-devel] [PATCH] IDE BMDMAState structure corruption with DMA_MULTI_THREAD, Richard W.M. Jones <=