[athena@fftw.org: [Qemu-devel] QEMU/KVM SCSI lock up]

[athena@fftw.org: [Qemu-devel] QEMU/KVM SCSI lock up]

[Marcelo Tosatti]

Paul,

Please apply.

----- Forwarded message from Matteo Frigo <athena@fftw.org> -----

From: Matteo Frigo <athena@fftw.org>
Date: Wed, 02 Apr 2008 20:41:24 -0400
To: qemu-devel@nongnu.org
Subject: [Qemu-devel] QEMU/KVM SCSI lock up
X-detected-kernel: by monty-python.gnu.org: Linux 2.6, seldom 2.4 (older, 4)
X-Mailman-Approved-At: Thu, 03 Apr 2008 00:59:59 -0400
Reply-To: qemu-devel@nongnu.org

kvm-64 hangs under heavy disk I/O with scsi disks.  To reproduce,
create a fresh qcow2 disk, boot linux, and execute

  dd if=/dev/sdX of=/dev/null bs=1M

on the fresh disk.  See also https://sourceforge.net/tracker/?func=detail&atid=893831&aid=1895893&group_id=180599

I have attached a patch that appears to fix the problem.  The bug
seems to be the following.  scsi_read_data() does the following

    bdrv_aio_read()
    r->sector += n;
    r->sector_count -= n;

For reasons that I do not fully understand, bdrv_aio_read() does
not return immediately, but instead it calls scsi_read_data()
recursively.  Since ``r->sector += n;'' has not been executed
yet, the re-entrant call triggers a read of the same sector, which
breaks the producer-consumer lockstep.  The fix is to swap the operations
as follows:

    r->sector += n;
    r->sector_count -= n;
    bdrv_aio_read()

A similar fix applies to scsi_write_data().

Thanks for developing kvm, it is truly an amazing piece of software.

Regards,
Matteo Frigo


diff -aur kvm-64.old/qemu/hw/scsi-disk.c kvm-64.new/qemu/hw/scsi-disk.c
--- kvm-64.old/qemu/hw/scsi-disk.c	2008-03-26 08:49:35.000000000 -0400
+++ kvm-64.new/qemu/hw/scsi-disk.c	2008-03-30 08:37:25.000000000 -0400
@@ -196,12 +196,12 @@
         n = SCSI_DMA_BUF_SIZE / 512;
 
     r->buf_len = n * 512;
-    r->aiocb = bdrv_aio_read(s->bdrv, r->sector, r->dma_buf, n,
+    r->sector += n;
+    r->sector_count -= n;
+    r->aiocb = bdrv_aio_read(s->bdrv, r->sector - n, r->dma_buf, n,
                              scsi_read_complete, r);
     if (r->aiocb == NULL)
         scsi_command_complete(r, SENSE_HARDWARE_ERROR);
-    r->sector += n;
-    r->sector_count -= n;
 }
 
 static void scsi_write_complete(void * opaque, int ret)
@@ -248,12 +248,12 @@
         BADF("Data transfer already in progress\n");
     n = r->buf_len / 512;
     if (n) {
-        r->aiocb = bdrv_aio_write(s->bdrv, r->sector, r->dma_buf, n,
+        r->sector += n;
+        r->sector_count -= n;
+        r->aiocb = bdrv_aio_write(s->bdrv, r->sector - n, r->dma_buf, n,
                                   scsi_write_complete, r);
         if (r->aiocb == NULL)
             scsi_command_complete(r, SENSE_HARDWARE_ERROR);
-        r->sector += n;
-        r->sector_count -= n;
     } else {
         /* Invoke completion routine to fetch data from host.  */
         scsi_write_complete(r, 0);