[Qemu-devel] [PULL for-2.5 0/4] Block patches

[Qemu-devel] [PULL for-2.5 0/4] Block patches

[Stefan Hajnoczi]

The following changes since commit cf22132367a188426ac07cf1805b214dd2d0cc80:

  Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2015-12-02 17:05:34 +0000)

are available in the git repository at:

  git://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to 9cc0f19de213fcb7098f0ea8f42448728f2cfcde:

  iotests: Add regresion test case for write notifier assertion failure (2015-12-03 11:08:07 +0800)

----------------------------------------------------------------

----------------------------------------------------------------

Fam Zheng (3):
  block: Don't wait serialising for non-COR read requests
  iotests: Add "add_drive_raw" method
  iotests: Add regresion test case for write notifier assertion failure

Paolo Bonzini (1):
  iothread: include id in thread name

 block/backup.c                |  2 +-
 block/io.c                    | 12 +++++++-----
 include/block/block.h         |  4 ++--
 iothread.c                    |  7 ++++++-
 tests/qemu-iotests/056        | 25 +++++++++++++++++++++++++
 tests/qemu-iotests/056.out    |  4 ++--
 tests/qemu-iotests/iotests.py |  5 +++++
 trace-events                  |  2 +-
 8 files changed, 49 insertions(+), 12 deletions(-)

-- 
2.5.0

[Qemu-devel] [PULL for-2.5 1/4] iothread: include id in thread name

[Stefan Hajnoczi]

From: Paolo Bonzini <pbonzini@redhat.com>

This makes it easier to find the desired thread.  Use "IO" plus the id;
even with the 14 character limit on the thread name, enough of the id should
be readable (e.g. "IO iothreadNNN" with three characters for the number).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-id: 1448372804-5034-1-git-send-email-pbonzini@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 iothread.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/iothread.c b/iothread.c
index da6ce7b..1b8c2bb 100644
--- a/iothread.c
+++ b/iothread.c
@@ -72,6 +72,7 @@ static void iothread_complete(UserCreatable *obj, Error **errp)
 {
     Error *local_error = NULL;
     IOThread *iothread = IOTHREAD(obj);
+    char *name, *thread_name;
 
     iothread->stopping = false;
     iothread->thread_id = -1;
@@ -87,8 +88,12 @@ static void iothread_complete(UserCreatable *obj, Error **errp)
     /* This assumes we are called from a thread with useful CPU affinity for us
      * to inherit.
      */
-    qemu_thread_create(&iothread->thread, "iothread", iothread_run,
+    name = object_get_canonical_path_component(OBJECT(obj));
+    thread_name = g_strdup_printf("IO %s", name);
+    qemu_thread_create(&iothread->thread, thread_name, iothread_run,
                        iothread, QEMU_THREAD_JOINABLE);
+    g_free(thread_name);
+    g_free(name);
 
     /* Wait for initialization to complete */
     qemu_mutex_lock(&iothread->init_done_lock);
-- 
2.5.0

[Qemu-devel] [PULL for-2.5 2/4] block: Don't wait serialising for non-COR read requests

[Stefan Hajnoczi]

From: Fam Zheng <famz@redhat.com>

The assertion problem was noticed in 06c3916b35a, but it wasn't
completely fixed, because even though the req is not marked as
serialising, it still gets serialised by wait_serialising_requests
against other serialising requests, which could lead to the same
assertion failure.

Fix it by even more explicitly skipping the serialising for this
specific case.

Signed-off-by: Fam Zheng <famz@redhat.com>
Message-id: 1448962590-2842-2-git-send-email-famz@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/backup.c        |  2 +-
 block/io.c            | 12 +++++++-----
 include/block/block.h |  4 ++--
 trace-events          |  2 +-
 4 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/block/backup.c b/block/backup.c
index 3b39119..705bb77 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -132,7 +132,7 @@ static int coroutine_fn backup_do_cow(BlockDriverState *bs,
         qemu_iovec_init_external(&bounce_qiov, &iov, 1);
 
         if (is_write_notifier) {
-            ret = bdrv_co_no_copy_on_readv(bs,
+            ret = bdrv_co_readv_no_serialising(bs,
                                            start * BACKUP_SECTORS_PER_CLUSTER,
                                            n, &bounce_qiov);
         } else {
diff --git a/block/io.c b/block/io.c
index adc1eab..e00fb5d 100644
--- a/block/io.c
+++ b/block/io.c
@@ -863,7 +863,9 @@ static int coroutine_fn bdrv_aligned_preadv(BlockDriverState *bs,
         mark_request_serialising(req, bdrv_get_cluster_size(bs));
     }
 
-    wait_serialising_requests(req);
+    if (!(flags & BDRV_REQ_NO_SERIALISING)) {
+        wait_serialising_requests(req);
+    }
 
     if (flags & BDRV_REQ_COPY_ON_READ) {
         int pnum;
@@ -952,7 +954,7 @@ static int coroutine_fn bdrv_co_do_preadv(BlockDriverState *bs,
     }
 
     /* Don't do copy-on-read if we read data before write operation */
-    if (bs->copy_on_read && !(flags & BDRV_REQ_NO_COPY_ON_READ)) {
+    if (bs->copy_on_read && !(flags & BDRV_REQ_NO_SERIALISING)) {
         flags |= BDRV_REQ_COPY_ON_READ;
     }
 
@@ -1021,13 +1023,13 @@ int coroutine_fn bdrv_co_readv(BlockDriverState *bs, int64_t sector_num,
     return bdrv_co_do_readv(bs, sector_num, nb_sectors, qiov, 0);
 }
 
-int coroutine_fn bdrv_co_no_copy_on_readv(BlockDriverState *bs,
+int coroutine_fn bdrv_co_readv_no_serialising(BlockDriverState *bs,
     int64_t sector_num, int nb_sectors, QEMUIOVector *qiov)
 {
-    trace_bdrv_co_no_copy_on_readv(bs, sector_num, nb_sectors);
+    trace_bdrv_co_readv_no_serialising(bs, sector_num, nb_sectors);
 
     return bdrv_co_do_readv(bs, sector_num, nb_sectors, qiov,
-                            BDRV_REQ_NO_COPY_ON_READ);
+                            BDRV_REQ_NO_SERIALISING);
 }
 
 int coroutine_fn bdrv_co_copy_on_readv(BlockDriverState *bs,
diff --git a/include/block/block.h b/include/block/block.h
index 73edb1a..3477328 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -61,7 +61,7 @@ typedef enum {
      * opened with BDRV_O_UNMAP.
      */
     BDRV_REQ_MAY_UNMAP          = 0x4,
-    BDRV_REQ_NO_COPY_ON_READ    = 0x8,
+    BDRV_REQ_NO_SERIALISING     = 0x8,
 } BdrvRequestFlags;
 
 typedef struct BlockSizes {
@@ -248,7 +248,7 @@ int coroutine_fn bdrv_co_readv(BlockDriverState *bs, int64_t sector_num,
     int nb_sectors, QEMUIOVector *qiov);
 int coroutine_fn bdrv_co_copy_on_readv(BlockDriverState *bs,
     int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
-int coroutine_fn bdrv_co_no_copy_on_readv(BlockDriverState *bs,
+int coroutine_fn bdrv_co_readv_no_serialising(BlockDriverState *bs,
     int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
 int coroutine_fn bdrv_co_writev(BlockDriverState *bs, int64_t sector_num,
     int nb_sectors, QEMUIOVector *qiov);
diff --git a/trace-events b/trace-events
index 0b0ff02..2fce98e 100644
--- a/trace-events
+++ b/trace-events
@@ -69,7 +69,7 @@ bdrv_aio_write_zeroes(void *bs, int64_t sector_num, int nb_sectors, int flags, v
 bdrv_lock_medium(void *bs, bool locked) "bs %p locked %d"
 bdrv_co_readv(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
 bdrv_co_copy_on_readv(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
-bdrv_co_no_copy_on_readv(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
+bdrv_co_readv_no_serialising(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
 bdrv_co_writev(void *bs, int64_t sector_num, int nb_sector) "bs %p sector_num %"PRId64" nb_sectors %d"
 bdrv_co_write_zeroes(void *bs, int64_t sector_num, int nb_sector, int flags) "bs %p sector_num %"PRId64" nb_sectors %d flags %#x"
 bdrv_co_io_em(void *bs, int64_t sector_num, int nb_sectors, int is_write, void *acb) "bs %p sector_num %"PRId64" nb_sectors %d is_write %d acb %p"
-- 
2.5.0

[Qemu-devel] [PULL for-2.5 3/4] iotests: Add "add_drive_raw" method

[Stefan Hajnoczi]

From: Fam Zheng <famz@redhat.com>

This offers full manual control over the "-drive" options.

Signed-off-by: Fam Zheng <famz@redhat.com>
Message-id: 1448962590-2842-3-git-send-email-famz@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/iotests.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index ff5905f..e02245e 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -140,6 +140,11 @@ class VM(object):
         self._args.append('-monitor')
         self._args.append(args)
 
+    def add_drive_raw(self, opts):
+        self._args.append('-drive')
+        self._args.append(opts)
+        return self
+
     def add_drive(self, path, opts='', interface='virtio'):
         '''Add a virtio-blk drive to the VM'''
         options = ['if=%s' % interface,
-- 
2.5.0

[Qemu-devel] [PULL for-2.5 4/4] iotests: Add regresion test case for write notifier assertion failure

[Stefan Hajnoczi]

From: Fam Zheng <famz@redhat.com>

The idea is to let the top level bs have a big request alignment with
blkdebug, so that the aio_write request issued from monitor will be
serialised. This tests that QEMU doesn't crash upon the read request
from the backup job's write notifier, which is a very special case of
"reentrant" request.

Signed-off-by: Fam Zheng <famz@redhat.com>
Message-id: 1448962590-2842-4-git-send-email-famz@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/056     | 25 +++++++++++++++++++++++++
 tests/qemu-iotests/056.out |  4 ++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/056 b/tests/qemu-iotests/056
index 54e4bd0..04f2c3c 100755
--- a/tests/qemu-iotests/056
+++ b/tests/qemu-iotests/056
@@ -82,6 +82,31 @@ class TestSyncModesNoneAndTop(iotests.QMPTestCase):
         time.sleep(1)
         self.assertEqual(-1, qemu_io('-c', 'read -P0x41 0 512', target_img).find("verification failed"))
 
+class TestBeforeWriteNotifier(iotests.QMPTestCase):
+    def setUp(self):
+        self.vm = iotests.VM().add_drive_raw("file=blkdebug::null-co://,id=drive0,align=65536,driver=blkdebug")
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.remove(target_img)
+
+    def test_before_write_notifier(self):
+        self.vm.pause_drive("drive0")
+        result = self.vm.qmp('drive-backup', device='drive0',
+                             sync='full', target=target_img,
+                             format="file", speed=1)
+        self.assert_qmp(result, 'return', {})
+        result = self.vm.qmp('block-job-pause', device="drive0")
+        self.assert_qmp(result, 'return', {})
+        # Speed is low enough that this must be an uncopied range, which will
+        # trigger the before write notifier
+        self.vm.hmp_qemu_io('drive0', 'aio_write -P 1 512512 512')
+        self.vm.resume_drive("drive0")
+        result = self.vm.qmp('block-job-resume', device="drive0")
+        self.assert_qmp(result, 'return', {})
+        event = self.cancel_and_wait()
+        self.assert_qmp(event, 'data/type', 'backup')
 
 if __name__ == '__main__':
     iotests.main(supported_fmts=['qcow2', 'qed'])
diff --git a/tests/qemu-iotests/056.out b/tests/qemu-iotests/056.out
index fbc63e6..8d7e996 100644
--- a/tests/qemu-iotests/056.out
+++ b/tests/qemu-iotests/056.out
@@ -1,5 +1,5 @@
-..
+...
 ----------------------------------------------------------------------
-Ran 2 tests
+Ran 3 tests
 
 OK
-- 
2.5.0

Re: [Qemu-devel] [PULL for-2.5 0/4] Block patches

[Peter Maydell]

On 3 December 2015 at 04:59, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> The following changes since commit cf22132367a188426ac07cf1805b214dd2d0cc80:
>
>   Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2015-12-02 17:05:34 +0000)
>
> are available in the git repository at:
>
>   git://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to 9cc0f19de213fcb7098f0ea8f42448728f2cfcde:
>
>   iotests: Add regresion test case for write notifier assertion failure (2015-12-03 11:08:07 +0800)
>
> ----------------------------------------------------------------
>
> ----------------------------------------------------------------
>
> Fam Zheng (3):
>   block: Don't wait serialising for non-COR read requests
>   iotests: Add "add_drive_raw" method
>   iotests: Add regresion test case for write notifier assertion failure
>
> Paolo Bonzini (1):
>   iothread: include id in thread name
>

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PULL for-2.5 1/4] iothread: include id in thread name

[Cornelia Huck]

On Thu,  3 Dec 2015 12:59:59 +0800
Stefan Hajnoczi <stefanha@redhat.com> wrote:

> From: Paolo Bonzini <pbonzini@redhat.com>
> 
> This makes it easier to find the desired thread.  Use "IO" plus the id;
> even with the 14 character limit on the thread name, enough of the id should
> be readable (e.g. "IO iothreadNNN" with three characters for the number).
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Message-id: 1448372804-5034-1-git-send-email-pbonzini@redhat.com
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  iothread.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/iothread.c b/iothread.c
> index da6ce7b..1b8c2bb 100644
> --- a/iothread.c
> +++ b/iothread.c
> @@ -72,6 +72,7 @@ static void iothread_complete(UserCreatable *obj, Error **errp)
>  {
>      Error *local_error = NULL;
>      IOThread *iothread = IOTHREAD(obj);
> +    char *name, *thread_name;
> 
>      iothread->stopping = false;
>      iothread->thread_id = -1;
> @@ -87,8 +88,12 @@ static void iothread_complete(UserCreatable *obj, Error **errp)
>      /* This assumes we are called from a thread with useful CPU affinity for us
>       * to inherit.
>       */
> -    qemu_thread_create(&iothread->thread, "iothread", iothread_run,
> +    name = object_get_canonical_path_component(OBJECT(obj));

This one breaks x-data-plane:

... -device virtio-blk-ccw,x-data-plane=true ...

ERROR:/data/git/yyy/qemu/qom/object.c:1515:object_get_canonical_path_component: assertion failed: (obj->parent != NULL)

> +    thread_name = g_strdup_printf("IO %s", name);
> +    qemu_thread_create(&iothread->thread, thread_name, iothread_run,
>                         iothread, QEMU_THREAD_JOINABLE);
> +    g_free(thread_name);
> +    g_free(name);
> 
>      /* Wait for initialization to complete */
>      qemu_mutex_lock(&iothread->init_done_lock);

Re: [Qemu-devel] [PULL for-2.5 2/4] block: Don't wait serialising for non-COR read requests

[Cornelia Huck]

On Thu,  3 Dec 2015 13:00:00 +0800
Stefan Hajnoczi <stefanha@redhat.com> wrote:

> From: Fam Zheng <famz@redhat.com>
> 
> The assertion problem was noticed in 06c3916b35a, but it wasn't
> completely fixed, because even though the req is not marked as
> serialising, it still gets serialised by wait_serialising_requests
> against other serialising requests, which could lead to the same
> assertion failure.
> 
> Fix it by even more explicitly skipping the serialising for this
> specific case.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> Message-id: 1448962590-2842-2-git-send-email-famz@redhat.com
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/backup.c        |  2 +-
>  block/io.c            | 12 +++++++-----
>  include/block/block.h |  4 ++--
>  trace-events          |  2 +-
>  4 files changed, 11 insertions(+), 9 deletions(-)

This one causes segfaults for me:

Program received signal SIGSEGV, Segmentation fault.
bdrv_is_inserted (bs=0x800000000000) at /data/git/yyy/qemu/block.c:3071
3071	    if (!drv) {

(gdb) bt
#0  bdrv_is_inserted (bs=0x800000000000) at /data/git/yyy/qemu/block.c:3071
#1  0x0000000080216974 in blk_is_inserted (blk=<optimized out>)
    at /data/git/yyy/qemu/block/block-backend.c:986
#2  0x00000000802169c6 in blk_is_available (blk=blk@entry=0x3ffb17e7960)
    at /data/git/yyy/qemu/block/block-backend.c:991
#3  0x0000000080216d12 in blk_check_byte_request (blk=blk@entry=0x3ffb17e7960, 
    offset=offset@entry=4928966656, size=16384)
    at /data/git/yyy/qemu/block/block-backend.c:558
#4  0x0000000080216df2 in blk_check_request (blk=blk@entry=0x3ffb17e7960, 
    sector_num=sector_num@entry=9626888, nb_sectors=nb_sectors@entry=32)
    at /data/git/yyy/qemu/block/block-backend.c:589
#5  0x0000000080217ee8 in blk_aio_readv (blk=0x3ffb17e7960, sector_num=
    9626888, iov=0x8098c658, nb_sectors=<optimized out>, cb=
    0x80081150 <virtio_blk_rw_complete>, opaque=0x80980620)
    at /data/git/yyy/qemu/block/block-backend.c:727
#6  0x000000008008186e in submit_requests (niov=<optimized out>, 
    num_reqs=<optimized out>, start=<optimized out>, mrb=<optimized out>, 
    blk=<optimized out>) at /data/git/yyy/qemu/hw/block/virtio-blk.c:366
#7  virtio_blk_submit_multireq (mrb=<optimized out>, blk=<optimized out>)
    at /data/git/yyy/qemu/hw/block/virtio-blk.c:444
#8  virtio_blk_submit_multireq (blk=0x3ffb17e7960, mrb=0x3ffffffeb58)
    at /data/git/yyy/qemu/hw/block/virtio-blk.c:389
#9  0x00000000800823ee in virtio_blk_handle_output (vdev=<optimized out>, 
    vq=<optimized out>) at /data/git/yyy/qemu/hw/block/virtio-blk.c:615
#10 0x00000000801e367e in aio_dispatch (ctx=0x80918520)
    at /data/git/yyy/qemu/aio-posix.c:326
#11 0x00000000801d28b0 in aio_ctx_dispatch (source=<optimized out>, 
    callback=<optimized out>, user_data=<optimized out>)
    at /data/git/yyy/qemu/async.c:231
#12 0x000003fffd36a05a in g_main_context_dispatch ()
   from /lib64/libglib-2.0.so.0
#13 0x00000000801e0ffa in glib_pollfds_poll ()
    at /data/git/yyy/qemu/main-loop.c:211
#14 os_host_main_loop_wait (timeout=<optimized out>)
    at /data/git/yyy/qemu/main-loop.c:256
#15 main_loop_wait (nonblocking=<optimized out>)
    at /data/git/yyy/qemu/main-loop.c:504
#16 0x00000000800148a6 in main_loop () at /data/git/yyy/qemu/vl.c:1923
#17 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>)
    at /data/git/yyy/qemu/vl.c:4684

Relevant part of command line:

-drive file=/dev/sda,if=none,id=drive-virtio-disk0,format=raw,serial=ccwzfcp1,cache=none -device virtio-blk-ccw,devno=fe.0.0001,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,scsi=off

Re: [Qemu-devel] [PULL for-2.5 2/4] block: Don't wait serialising for non-COR read requests

[Cornelia Huck]

On Mon, 7 Dec 2015 11:02:51 +0100
Cornelia Huck <cornelia.huck@de.ibm.com> wrote:

> On Thu,  3 Dec 2015 13:00:00 +0800
> Stefan Hajnoczi <stefanha@redhat.com> wrote:
> 
> > From: Fam Zheng <famz@redhat.com>
> > 
> > The assertion problem was noticed in 06c3916b35a, but it wasn't
> > completely fixed, because even though the req is not marked as
> > serialising, it still gets serialised by wait_serialising_requests
> > against other serialising requests, which could lead to the same
> > assertion failure.
> > 
> > Fix it by even more explicitly skipping the serialising for this
> > specific case.
> > 
> > Signed-off-by: Fam Zheng <famz@redhat.com>
> > Message-id: 1448962590-2842-2-git-send-email-famz@redhat.com
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  block/backup.c        |  2 +-
> >  block/io.c            | 12 +++++++-----
> >  include/block/block.h |  4 ++--
> >  trace-events          |  2 +-
> >  4 files changed, 11 insertions(+), 9 deletions(-)
> 
> This one causes segfaults for me:
> 
> Program received signal SIGSEGV, Segmentation fault.
> bdrv_is_inserted (bs=0x800000000000) at /data/git/yyy/qemu/block.c:3071
> 3071	    if (!drv) {
> 
> (gdb) bt
> #0  bdrv_is_inserted (bs=0x800000000000) at /data/git/yyy/qemu/block.c:3071
> #1  0x0000000080216974 in blk_is_inserted (blk=<optimized out>)
>     at /data/git/yyy/qemu/block/block-backend.c:986
> #2  0x00000000802169c6 in blk_is_available (blk=blk@entry=0x3ffb17e7960)
>     at /data/git/yyy/qemu/block/block-backend.c:991
> #3  0x0000000080216d12 in blk_check_byte_request (blk=blk@entry=0x3ffb17e7960, 
>     offset=offset@entry=4928966656, size=16384)
>     at /data/git/yyy/qemu/block/block-backend.c:558
> #4  0x0000000080216df2 in blk_check_request (blk=blk@entry=0x3ffb17e7960, 
>     sector_num=sector_num@entry=9626888, nb_sectors=nb_sectors@entry=32)
>     at /data/git/yyy/qemu/block/block-backend.c:589
> #5  0x0000000080217ee8 in blk_aio_readv (blk=0x3ffb17e7960, sector_num=
>     9626888, iov=0x8098c658, nb_sectors=<optimized out>, cb=
>     0x80081150 <virtio_blk_rw_complete>, opaque=0x80980620)
>     at /data/git/yyy/qemu/block/block-backend.c:727
> #6  0x000000008008186e in submit_requests (niov=<optimized out>, 
>     num_reqs=<optimized out>, start=<optimized out>, mrb=<optimized out>, 
>     blk=<optimized out>) at /data/git/yyy/qemu/hw/block/virtio-blk.c:366
> #7  virtio_blk_submit_multireq (mrb=<optimized out>, blk=<optimized out>)
>     at /data/git/yyy/qemu/hw/block/virtio-blk.c:444
> #8  virtio_blk_submit_multireq (blk=0x3ffb17e7960, mrb=0x3ffffffeb58)
>     at /data/git/yyy/qemu/hw/block/virtio-blk.c:389
> #9  0x00000000800823ee in virtio_blk_handle_output (vdev=<optimized out>, 
>     vq=<optimized out>) at /data/git/yyy/qemu/hw/block/virtio-blk.c:615
> #10 0x00000000801e367e in aio_dispatch (ctx=0x80918520)
>     at /data/git/yyy/qemu/aio-posix.c:326
> #11 0x00000000801d28b0 in aio_ctx_dispatch (source=<optimized out>, 
>     callback=<optimized out>, user_data=<optimized out>)
>     at /data/git/yyy/qemu/async.c:231
> #12 0x000003fffd36a05a in g_main_context_dispatch ()
>    from /lib64/libglib-2.0.so.0
> #13 0x00000000801e0ffa in glib_pollfds_poll ()
>     at /data/git/yyy/qemu/main-loop.c:211
> #14 os_host_main_loop_wait (timeout=<optimized out>)
>     at /data/git/yyy/qemu/main-loop.c:256
> #15 main_loop_wait (nonblocking=<optimized out>)
>     at /data/git/yyy/qemu/main-loop.c:504
> #16 0x00000000800148a6 in main_loop () at /data/git/yyy/qemu/vl.c:1923
> #17 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>)
>     at /data/git/yyy/qemu/vl.c:4684
> 
> Relevant part of command line:
> 
> -drive file=/dev/sda,if=none,id=drive-virtio-disk0,format=raw,serial=ccwzfcp1,cache=none -device virtio-blk-ccw,devno=fe.0.0001,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,scsi=off

I played around a bit. The main part of this change seems to be calling
wait_serialising_requests() conditionally; reverting this makes the
guest boot again.

I then tried to find out when wait_serialising_requests() was NOT
called and added fprintfs: well, it was _always_ called. I then added a
fprintf for flags at the beginning of the function: this produced a
segfault no matter whether wait_serialising_requests() was called
conditionally or unconditionally. Weird race?

Anything further I can do? I guess this patch fixes a bug for someone,
but it means insta-death for my setup...

Re: [Qemu-devel] [PULL for-2.5 2/4] block: Don't wait serialising for non-COR read requests

[Fam Zheng]

On Mon, 12/07 17:42, Cornelia Huck wrote:
> On Mon, 7 Dec 2015 11:02:51 +0100
> Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
> 
> > On Thu,  3 Dec 2015 13:00:00 +0800
> > Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > 
> > > From: Fam Zheng <famz@redhat.com>
> > > 
> > > The assertion problem was noticed in 06c3916b35a, but it wasn't
> > > completely fixed, because even though the req is not marked as
> > > serialising, it still gets serialised by wait_serialising_requests
> > > against other serialising requests, which could lead to the same
> > > assertion failure.
> > > 
> > > Fix it by even more explicitly skipping the serialising for this
> > > specific case.
> > > 
> > > Signed-off-by: Fam Zheng <famz@redhat.com>
> > > Message-id: 1448962590-2842-2-git-send-email-famz@redhat.com
> > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > > ---
> > >  block/backup.c        |  2 +-
> > >  block/io.c            | 12 +++++++-----
> > >  include/block/block.h |  4 ++--
> > >  trace-events          |  2 +-
> > >  4 files changed, 11 insertions(+), 9 deletions(-)
> > 
> > This one causes segfaults for me:
> > 
> > Program received signal SIGSEGV, Segmentation fault.
> > bdrv_is_inserted (bs=0x800000000000) at /data/git/yyy/qemu/block.c:3071
> > 3071	    if (!drv) {
> > 
> > (gdb) bt
> > #0  bdrv_is_inserted (bs=0x800000000000) at /data/git/yyy/qemu/block.c:3071
> > #1  0x0000000080216974 in blk_is_inserted (blk=<optimized out>)
> >     at /data/git/yyy/qemu/block/block-backend.c:986
> > #2  0x00000000802169c6 in blk_is_available (blk=blk@entry=0x3ffb17e7960)
> >     at /data/git/yyy/qemu/block/block-backend.c:991
> > #3  0x0000000080216d12 in blk_check_byte_request (blk=blk@entry=0x3ffb17e7960, 
> >     offset=offset@entry=4928966656, size=16384)
> >     at /data/git/yyy/qemu/block/block-backend.c:558
> > #4  0x0000000080216df2 in blk_check_request (blk=blk@entry=0x3ffb17e7960, 
> >     sector_num=sector_num@entry=9626888, nb_sectors=nb_sectors@entry=32)
> >     at /data/git/yyy/qemu/block/block-backend.c:589
> > #5  0x0000000080217ee8 in blk_aio_readv (blk=0x3ffb17e7960, sector_num=
> >     9626888, iov=0x8098c658, nb_sectors=<optimized out>, cb=
> >     0x80081150 <virtio_blk_rw_complete>, opaque=0x80980620)
> >     at /data/git/yyy/qemu/block/block-backend.c:727
> > #6  0x000000008008186e in submit_requests (niov=<optimized out>, 
> >     num_reqs=<optimized out>, start=<optimized out>, mrb=<optimized out>, 
> >     blk=<optimized out>) at /data/git/yyy/qemu/hw/block/virtio-blk.c:366
> > #7  virtio_blk_submit_multireq (mrb=<optimized out>, blk=<optimized out>)
> >     at /data/git/yyy/qemu/hw/block/virtio-blk.c:444
> > #8  virtio_blk_submit_multireq (blk=0x3ffb17e7960, mrb=0x3ffffffeb58)
> >     at /data/git/yyy/qemu/hw/block/virtio-blk.c:389

I can't parse these lines in the backtrace, what is #8 -> #7 ?

But other than that, this looks like a normal read request. Can you dump "blk"
fields?

> > #9  0x00000000800823ee in virtio_blk_handle_output (vdev=<optimized out>, 
> >     vq=<optimized out>) at /data/git/yyy/qemu/hw/block/virtio-blk.c:615
> > #10 0x00000000801e367e in aio_dispatch (ctx=0x80918520)
> >     at /data/git/yyy/qemu/aio-posix.c:326
> > #11 0x00000000801d28b0 in aio_ctx_dispatch (source=<optimized out>, 
> >     callback=<optimized out>, user_data=<optimized out>)
> >     at /data/git/yyy/qemu/async.c:231
> > #12 0x000003fffd36a05a in g_main_context_dispatch ()
> >    from /lib64/libglib-2.0.so.0
> > #13 0x00000000801e0ffa in glib_pollfds_poll ()
> >     at /data/git/yyy/qemu/main-loop.c:211
> > #14 os_host_main_loop_wait (timeout=<optimized out>)
> >     at /data/git/yyy/qemu/main-loop.c:256
> > #15 main_loop_wait (nonblocking=<optimized out>)
> >     at /data/git/yyy/qemu/main-loop.c:504
> > #16 0x00000000800148a6 in main_loop () at /data/git/yyy/qemu/vl.c:1923
> > #17 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>)
> >     at /data/git/yyy/qemu/vl.c:4684
> > 
> > Relevant part of command line:
> > 
> > -drive file=/dev/sda,if=none,id=drive-virtio-disk0,format=raw,serial=ccwzfcp1,cache=none -device virtio-blk-ccw,devno=fe.0.0001,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,scsi=off
> 
> I played around a bit. The main part of this change seems to be calling
> wait_serialising_requests() conditionally; reverting this makes the
> guest boot again.
> 
> I then tried to find out when wait_serialising_requests() was NOT
> called and added fprintfs: well, it was _always_ called. I then added a
> fprintf for flags at the beginning of the function: this produced a
> segfault no matter whether wait_serialising_requests() was called
> conditionally or unconditionally. Weird race?

It looks like a race, any other operations happening at the time?  What jobs
are in the guest?

It would be nice if you can advise the reproducing approach? Apparently my
attempt failed..

Fam

> 
> Anything further I can do? I guess this patch fixes a bug for someone,
> but it means insta-death for my setup...
> 

Re: [Qemu-devel] [PULL for-2.5 2/4] block: Don't wait serialising for non-COR read requests

[Kevin Wolf]

Am 07.12.2015 um 17:42 hat Cornelia Huck geschrieben:
> On Mon, 7 Dec 2015 11:02:51 +0100
> Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
> 
> > On Thu,  3 Dec 2015 13:00:00 +0800
> > Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > 
> > > From: Fam Zheng <famz@redhat.com>
> > > 
> > > The assertion problem was noticed in 06c3916b35a, but it wasn't
> > > completely fixed, because even though the req is not marked as
> > > serialising, it still gets serialised by wait_serialising_requests
> > > against other serialising requests, which could lead to the same
> > > assertion failure.
> > > 
> > > Fix it by even more explicitly skipping the serialising for this
> > > specific case.
> > > 
> > > Signed-off-by: Fam Zheng <famz@redhat.com>
> > > Message-id: 1448962590-2842-2-git-send-email-famz@redhat.com
> > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > > ---
> > >  block/backup.c        |  2 +-
> > >  block/io.c            | 12 +++++++-----
> > >  include/block/block.h |  4 ++--
> > >  trace-events          |  2 +-
> > >  4 files changed, 11 insertions(+), 9 deletions(-)
> > 
> > This one causes segfaults for me:
> > 
> > Program received signal SIGSEGV, Segmentation fault.
> > bdrv_is_inserted (bs=0x800000000000) at /data/git/yyy/qemu/block.c:3071
> > 3071	    if (!drv) {
> > 
> > (gdb) bt
> > #0  bdrv_is_inserted (bs=0x800000000000) at /data/git/yyy/qemu/block.c:3071

This looks like some kind of memory corruption that hit blk->bs. It's
most definitely not a valid pointer anyway.

> > #1  0x0000000080216974 in blk_is_inserted (blk=<optimized out>)
> >     at /data/git/yyy/qemu/block/block-backend.c:986
> > #2  0x00000000802169c6 in blk_is_available (blk=blk@entry=0x3ffb17e7960)
> >     at /data/git/yyy/qemu/block/block-backend.c:991
> > #3  0x0000000080216d12 in blk_check_byte_request (blk=blk@entry=0x3ffb17e7960, 
> >     offset=offset@entry=4928966656, size=16384)
> >     at /data/git/yyy/qemu/block/block-backend.c:558
> > #4  0x0000000080216df2 in blk_check_request (blk=blk@entry=0x3ffb17e7960, 
> >     sector_num=sector_num@entry=9626888, nb_sectors=nb_sectors@entry=32)
> >     at /data/git/yyy/qemu/block/block-backend.c:589
> > #5  0x0000000080217ee8 in blk_aio_readv (blk=0x3ffb17e7960, sector_num=
> >     9626888, iov=0x8098c658, nb_sectors=<optimized out>, cb=
> >     0x80081150 <virtio_blk_rw_complete>, opaque=0x80980620)
> >     at /data/git/yyy/qemu/block/block-backend.c:727
> > #6  0x000000008008186e in submit_requests (niov=<optimized out>, 
> >     num_reqs=<optimized out>, start=<optimized out>, mrb=<optimized out>, 
> >     blk=<optimized out>) at /data/git/yyy/qemu/hw/block/virtio-blk.c:366
> > #7  virtio_blk_submit_multireq (mrb=<optimized out>, blk=<optimized out>)
> >     at /data/git/yyy/qemu/hw/block/virtio-blk.c:444
> > #8  virtio_blk_submit_multireq (blk=0x3ffb17e7960, mrb=0x3ffffffeb58)
> >     at /data/git/yyy/qemu/hw/block/virtio-blk.c:389
> > #9  0x00000000800823ee in virtio_blk_handle_output (vdev=<optimized out>, 
> >     vq=<optimized out>) at /data/git/yyy/qemu/hw/block/virtio-blk.c:615
> > #10 0x00000000801e367e in aio_dispatch (ctx=0x80918520)
> >     at /data/git/yyy/qemu/aio-posix.c:326
> > #11 0x00000000801d28b0 in aio_ctx_dispatch (source=<optimized out>, 
> >     callback=<optimized out>, user_data=<optimized out>)
> >     at /data/git/yyy/qemu/async.c:231
> > #12 0x000003fffd36a05a in g_main_context_dispatch ()
> >    from /lib64/libglib-2.0.so.0
> > #13 0x00000000801e0ffa in glib_pollfds_poll ()
> >     at /data/git/yyy/qemu/main-loop.c:211
> > #14 os_host_main_loop_wait (timeout=<optimized out>)
> >     at /data/git/yyy/qemu/main-loop.c:256
> > #15 main_loop_wait (nonblocking=<optimized out>)
> >     at /data/git/yyy/qemu/main-loop.c:504
> > #16 0x00000000800148a6 in main_loop () at /data/git/yyy/qemu/vl.c:1923
> > #17 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>)
> >     at /data/git/yyy/qemu/vl.c:4684
> > 
> > Relevant part of command line:
> > 
> > -drive file=/dev/sda,if=none,id=drive-virtio-disk0,format=raw,serial=ccwzfcp1,cache=none -device virtio-blk-ccw,devno=fe.0.0001,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,scsi=off
> 
> I played around a bit. The main part of this change seems to be calling
> wait_serialising_requests() conditionally; reverting this makes the
> guest boot again.
> 
> I then tried to find out when wait_serialising_requests() was NOT
> called and added fprintfs: well, it was _always_ called. I then added a
> fprintf for flags at the beginning of the function: this produced a
> segfault no matter whether wait_serialising_requests() was called
> conditionally or unconditionally. Weird race?
> 
> Anything further I can do? I guess this patch fixes a bug for someone,
> but it means insta-death for my setup...

If it happens immediately, perhaps running under valgrind is possible
and could give some hints about what happened with blk->bs?

Kevin

Re: [Qemu-devel] [PULL for-2.5 2/4] block: Don't wait serialising for non-COR read requests

[Cornelia Huck]

On Tue, 8 Dec 2015 10:59:54 +0100
Kevin Wolf <kwolf@redhat.com> wrote:

> Am 07.12.2015 um 17:42 hat Cornelia Huck geschrieben:
> > On Mon, 7 Dec 2015 11:02:51 +0100
> > Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
> > 
> > > On Thu,  3 Dec 2015 13:00:00 +0800
> > > Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > > 
> > > > From: Fam Zheng <famz@redhat.com>
> > > > 
> > > > The assertion problem was noticed in 06c3916b35a, but it wasn't
> > > > completely fixed, because even though the req is not marked as
> > > > serialising, it still gets serialised by wait_serialising_requests
> > > > against other serialising requests, which could lead to the same
> > > > assertion failure.
> > > > 
> > > > Fix it by even more explicitly skipping the serialising for this
> > > > specific case.
> > > > 
> > > > Signed-off-by: Fam Zheng <famz@redhat.com>
> > > > Message-id: 1448962590-2842-2-git-send-email-famz@redhat.com
> > > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > > > ---
> > > >  block/backup.c        |  2 +-
> > > >  block/io.c            | 12 +++++++-----
> > > >  include/block/block.h |  4 ++--
> > > >  trace-events          |  2 +-
> > > >  4 files changed, 11 insertions(+), 9 deletions(-)
> > > 
> > > This one causes segfaults for me:
> > > 
> > > Program received signal SIGSEGV, Segmentation fault.
> > > bdrv_is_inserted (bs=0x800000000000) at /data/git/yyy/qemu/block.c:3071
> > > 3071	    if (!drv) {
> > > 
> > > (gdb) bt
> > > #0  bdrv_is_inserted (bs=0x800000000000) at /data/git/yyy/qemu/block.c:3071
> 
> This looks like some kind of memory corruption that hit blk->bs. It's
> most definitely not a valid pointer anyway.
> 
> > > #1  0x0000000080216974 in blk_is_inserted (blk=<optimized out>)
> > >     at /data/git/yyy/qemu/block/block-backend.c:986
> > > #2  0x00000000802169c6 in blk_is_available (blk=blk@entry=0x3ffb17e7960)
> > >     at /data/git/yyy/qemu/block/block-backend.c:991
> > > #3  0x0000000080216d12 in blk_check_byte_request (blk=blk@entry=0x3ffb17e7960, 
> > >     offset=offset@entry=4928966656, size=16384)
> > >     at /data/git/yyy/qemu/block/block-backend.c:558
> > > #4  0x0000000080216df2 in blk_check_request (blk=blk@entry=0x3ffb17e7960, 
> > >     sector_num=sector_num@entry=9626888, nb_sectors=nb_sectors@entry=32)
> > >     at /data/git/yyy/qemu/block/block-backend.c:589
> > > #5  0x0000000080217ee8 in blk_aio_readv (blk=0x3ffb17e7960, sector_num=
> > >     9626888, iov=0x8098c658, nb_sectors=<optimized out>, cb=
> > >     0x80081150 <virtio_blk_rw_complete>, opaque=0x80980620)
> > >     at /data/git/yyy/qemu/block/block-backend.c:727
> > > #6  0x000000008008186e in submit_requests (niov=<optimized out>, 
> > >     num_reqs=<optimized out>, start=<optimized out>, mrb=<optimized out>, 
> > >     blk=<optimized out>) at /data/git/yyy/qemu/hw/block/virtio-blk.c:366
> > > #7  virtio_blk_submit_multireq (mrb=<optimized out>, blk=<optimized out>)
> > >     at /data/git/yyy/qemu/hw/block/virtio-blk.c:444
> > > #8  virtio_blk_submit_multireq (blk=0x3ffb17e7960, mrb=0x3ffffffeb58)
> > >     at /data/git/yyy/qemu/hw/block/virtio-blk.c:389
> > > #9  0x00000000800823ee in virtio_blk_handle_output (vdev=<optimized out>, 
> > >     vq=<optimized out>) at /data/git/yyy/qemu/hw/block/virtio-blk.c:615
> > > #10 0x00000000801e367e in aio_dispatch (ctx=0x80918520)
> > >     at /data/git/yyy/qemu/aio-posix.c:326
> > > #11 0x00000000801d28b0 in aio_ctx_dispatch (source=<optimized out>, 
> > >     callback=<optimized out>, user_data=<optimized out>)
> > >     at /data/git/yyy/qemu/async.c:231
> > > #12 0x000003fffd36a05a in g_main_context_dispatch ()
> > >    from /lib64/libglib-2.0.so.0
> > > #13 0x00000000801e0ffa in glib_pollfds_poll ()
> > >     at /data/git/yyy/qemu/main-loop.c:211
> > > #14 os_host_main_loop_wait (timeout=<optimized out>)
> > >     at /data/git/yyy/qemu/main-loop.c:256
> > > #15 main_loop_wait (nonblocking=<optimized out>)
> > >     at /data/git/yyy/qemu/main-loop.c:504
> > > #16 0x00000000800148a6 in main_loop () at /data/git/yyy/qemu/vl.c:1923
> > > #17 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>)
> > >     at /data/git/yyy/qemu/vl.c:4684
> > > 
> > > Relevant part of command line:
> > > 
> > > -drive file=/dev/sda,if=none,id=drive-virtio-disk0,format=raw,serial=ccwzfcp1,cache=none -device virtio-blk-ccw,devno=fe.0.0001,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,scsi=off
> > 
> > I played around a bit. The main part of this change seems to be calling
> > wait_serialising_requests() conditionally; reverting this makes the
> > guest boot again.
> > 
> > I then tried to find out when wait_serialising_requests() was NOT
> > called and added fprintfs: well, it was _always_ called. I then added a
> > fprintf for flags at the beginning of the function: this produced a
> > segfault no matter whether wait_serialising_requests() was called
> > conditionally or unconditionally. Weird race?
> > 
> > Anything further I can do? I guess this patch fixes a bug for someone,
> > but it means insta-death for my setup...
> 
> If it happens immediately, perhaps running under valgrind is possible
> and could give some hints about what happened with blk->bs?

Just a quick update: This triggers on a qemu built with a not-so-fresh
gcc 4.7.2 (and it seems to depend on compiler optimizations as well).
We can't trigger it with newer gccs. No hints from valgrind, sadly.
Investigation ongoing.

Re: [Qemu-devel] [PULL for-2.5 2/4] block: Don't wait serialising for non-COR read requests

[Christian Borntraeger]

On 12/08/2015 01:00 PM, Cornelia Huck wrote:
> On Tue, 8 Dec 2015 10:59:54 +0100
> Kevin Wolf <kwolf@redhat.com> wrote:
> 
>> Am 07.12.2015 um 17:42 hat Cornelia Huck geschrieben:
>>> On Mon, 7 Dec 2015 11:02:51 +0100
>>> Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
>>>
>>>> On Thu,  3 Dec 2015 13:00:00 +0800
>>>> Stefan Hajnoczi <stefanha@redhat.com> wrote:
>>>>
>>>>> From: Fam Zheng <famz@redhat.com>
>>>>>
>>>>> The assertion problem was noticed in 06c3916b35a, but it wasn't
>>>>> completely fixed, because even though the req is not marked as
>>>>> serialising, it still gets serialised by wait_serialising_requests
>>>>> against other serialising requests, which could lead to the same
>>>>> assertion failure.
>>>>>
>>>>> Fix it by even more explicitly skipping the serialising for this
>>>>> specific case.
>>>>>
>>>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>>>> Message-id: 1448962590-2842-2-git-send-email-famz@redhat.com
>>>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>>>> ---
>>>>>  block/backup.c        |  2 +-
>>>>>  block/io.c            | 12 +++++++-----
>>>>>  include/block/block.h |  4 ++--
>>>>>  trace-events          |  2 +-
>>>>>  4 files changed, 11 insertions(+), 9 deletions(-)
>>>>
>>>> This one causes segfaults for me:
>>>>
>>>> Program received signal SIGSEGV, Segmentation fault.
>>>> bdrv_is_inserted (bs=0x800000000000) at /data/git/yyy/qemu/block.c:3071
>>>> 3071	    if (!drv) {
>>>>
>>>> (gdb) bt
>>>> #0  bdrv_is_inserted (bs=0x800000000000) at /data/git/yyy/qemu/block.c:3071
>>
>> This looks like some kind of memory corruption that hit blk->bs. It's
>> most definitely not a valid pointer anyway.
>>
>>>> #1  0x0000000080216974 in blk_is_inserted (blk=<optimized out>)
>>>>     at /data/git/yyy/qemu/block/block-backend.c:986
>>>> #2  0x00000000802169c6 in blk_is_available (blk=blk@entry=0x3ffb17e7960)
>>>>     at /data/git/yyy/qemu/block/block-backend.c:991
>>>> #3  0x0000000080216d12 in blk_check_byte_request (blk=blk@entry=0x3ffb17e7960, 
>>>>     offset=offset@entry=4928966656, size=16384)
>>>>     at /data/git/yyy/qemu/block/block-backend.c:558
>>>> #4  0x0000000080216df2 in blk_check_request (blk=blk@entry=0x3ffb17e7960, 
>>>>     sector_num=sector_num@entry=9626888, nb_sectors=nb_sectors@entry=32)
>>>>     at /data/git/yyy/qemu/block/block-backend.c:589
>>>> #5  0x0000000080217ee8 in blk_aio_readv (blk=0x3ffb17e7960, sector_num=
>>>>     9626888, iov=0x8098c658, nb_sectors=<optimized out>, cb=
>>>>     0x80081150 <virtio_blk_rw_complete>, opaque=0x80980620)
>>>>     at /data/git/yyy/qemu/block/block-backend.c:727
>>>> #6  0x000000008008186e in submit_requests (niov=<optimized out>, 
>>>>     num_reqs=<optimized out>, start=<optimized out>, mrb=<optimized out>, 
>>>>     blk=<optimized out>) at /data/git/yyy/qemu/hw/block/virtio-blk.c:366
>>>> #7  virtio_blk_submit_multireq (mrb=<optimized out>, blk=<optimized out>)
>>>>     at /data/git/yyy/qemu/hw/block/virtio-blk.c:444
>>>> #8  virtio_blk_submit_multireq (blk=0x3ffb17e7960, mrb=0x3ffffffeb58)
>>>>     at /data/git/yyy/qemu/hw/block/virtio-blk.c:389
>>>> #9  0x00000000800823ee in virtio_blk_handle_output (vdev=<optimized out>, 
>>>>     vq=<optimized out>) at /data/git/yyy/qemu/hw/block/virtio-blk.c:615
>>>> #10 0x00000000801e367e in aio_dispatch (ctx=0x80918520)
>>>>     at /data/git/yyy/qemu/aio-posix.c:326
>>>> #11 0x00000000801d28b0 in aio_ctx_dispatch (source=<optimized out>, 
>>>>     callback=<optimized out>, user_data=<optimized out>)
>>>>     at /data/git/yyy/qemu/async.c:231
>>>> #12 0x000003fffd36a05a in g_main_context_dispatch ()
>>>>    from /lib64/libglib-2.0.so.0
>>>> #13 0x00000000801e0ffa in glib_pollfds_poll ()
>>>>     at /data/git/yyy/qemu/main-loop.c:211
>>>> #14 os_host_main_loop_wait (timeout=<optimized out>)
>>>>     at /data/git/yyy/qemu/main-loop.c:256
>>>> #15 main_loop_wait (nonblocking=<optimized out>)
>>>>     at /data/git/yyy/qemu/main-loop.c:504
>>>> #16 0x00000000800148a6 in main_loop () at /data/git/yyy/qemu/vl.c:1923
>>>> #17 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>)
>>>>     at /data/git/yyy/qemu/vl.c:4684
>>>>
>>>> Relevant part of command line:
>>>>
>>>> -drive file=/dev/sda,if=none,id=drive-virtio-disk0,format=raw,serial=ccwzfcp1,cache=none -device virtio-blk-ccw,devno=fe.0.0001,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,scsi=off
>>>
>>> I played around a bit. The main part of this change seems to be calling
>>> wait_serialising_requests() conditionally; reverting this makes the
>>> guest boot again.
>>>
>>> I then tried to find out when wait_serialising_requests() was NOT
>>> called and added fprintfs: well, it was _always_ called. I then added a
>>> fprintf for flags at the beginning of the function: this produced a
>>> segfault no matter whether wait_serialising_requests() was called
>>> conditionally or unconditionally. Weird race?
>>>
>>> Anything further I can do? I guess this patch fixes a bug for someone,
>>> but it means insta-death for my setup...
>>
>> If it happens immediately, perhaps running under valgrind is possible
>> and could give some hints about what happened with blk->bs?
> 
> Just a quick update: This triggers on a qemu built with a not-so-fresh
> gcc 4.7.2 (and it seems to depend on compiler optimizations as well).
> We can't trigger it with newer gccs. No hints from valgrind, sadly.
> Investigation ongoing.
> 
> 

Some updates after looking at Cornelias system. It seem to be related to 
the F18 gcc that is still on that test system.

Problem happens when hw/block/virtio-blk.c is compiled
with -O2 and goes away with -O1 and -O0 (I trimmed that down to
-fexpensive-optimizations) 

The system calls virtio_blk_submit_multireq 26 times and then it messes
up the blk pointer:

(gdb) display blk->name
1: blk->name = 0x80894300 "drive-virtio-disk0"
(gdb) next
419	            if (niov + req->qiov.niov > IOV_MAX) {
1: blk->name = 0x80894300 "drive-virtio-disk0"
(gdb) 
424	            if (req->qiov.size / BDRV_SECTOR_SIZE + nb_sectors > max_xfer_len) {
1: blk->name = 0x80894300 "drive-virtio-disk0"
(gdb) 
419	            if (niov + req->qiov.niov > IOV_MAX) {
1: blk->name = 0x80894300 "drive-virtio-disk0"
(gdb) 
429	            if (sector_num + nb_sectors != req->sector_num) {
1: blk->name = 0x80894300 "drive-virtio-disk0"
(gdb) 
434	                submit_requests(blk, mrb, start, num_reqs, niov);
1: blk->name = 0x80894300 "drive-virtio-disk0"
(gdb) 
413	    for (i = 0; i < mrb->num_reqs; i++) {
1: blk->name = 0x8089ae40 ""


uninlining submit_request makes the problem go away, so might be a
gcc bug in Fedora18. I am now going to look at the disassembly to be
sure.

Christian

Re: [Qemu-devel] [PULL for-2.5 2/4] block: Don't wait serialising for non-COR read requests

[Christian Borntraeger]

On 12/08/2015 01:30 PM, Christian Borntraeger wrote:
> On 12/08/2015 01:00 PM, Cornelia Huck wrote:
>> On Tue, 8 Dec 2015 10:59:54 +0100
>> Kevin Wolf <kwolf@redhat.com> wrote:
>>
>>> Am 07.12.2015 um 17:42 hat Cornelia Huck geschrieben:
>>>> On Mon, 7 Dec 2015 11:02:51 +0100
>>>> Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
>>>>
>>>>> On Thu,  3 Dec 2015 13:00:00 +0800
>>>>> Stefan Hajnoczi <stefanha@redhat.com> wrote:
>>>>>
>>>>>> From: Fam Zheng <famz@redhat.com>
>>>>>>
>>>>>> The assertion problem was noticed in 06c3916b35a, but it wasn't
>>>>>> completely fixed, because even though the req is not marked as
>>>>>> serialising, it still gets serialised by wait_serialising_requests
>>>>>> against other serialising requests, which could lead to the same
>>>>>> assertion failure.
>>>>>>
>>>>>> Fix it by even more explicitly skipping the serialising for this
>>>>>> specific case.
>>>>>>
>>>>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>>>>> Message-id: 1448962590-2842-2-git-send-email-famz@redhat.com
>>>>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>>>>> ---
>>>>>>  block/backup.c        |  2 +-
>>>>>>  block/io.c            | 12 +++++++-----
>>>>>>  include/block/block.h |  4 ++--
>>>>>>  trace-events          |  2 +-
>>>>>>  4 files changed, 11 insertions(+), 9 deletions(-)
>>>>>
>>>>> This one causes segfaults for me:
>>>>>
>>>>> Program received signal SIGSEGV, Segmentation fault.
>>>>> bdrv_is_inserted (bs=0x800000000000) at /data/git/yyy/qemu/block.c:3071
>>>>> 3071	    if (!drv) {
>>>>>
>>>>> (gdb) bt
>>>>> #0  bdrv_is_inserted (bs=0x800000000000) at /data/git/yyy/qemu/block.c:3071
>>>
>>> This looks like some kind of memory corruption that hit blk->bs. It's
>>> most definitely not a valid pointer anyway.
>>>
>>>>> #1  0x0000000080216974 in blk_is_inserted (blk=<optimized out>)
>>>>>     at /data/git/yyy/qemu/block/block-backend.c:986
>>>>> #2  0x00000000802169c6 in blk_is_available (blk=blk@entry=0x3ffb17e7960)
>>>>>     at /data/git/yyy/qemu/block/block-backend.c:991
>>>>> #3  0x0000000080216d12 in blk_check_byte_request (blk=blk@entry=0x3ffb17e7960, 
>>>>>     offset=offset@entry=4928966656, size=16384)
>>>>>     at /data/git/yyy/qemu/block/block-backend.c:558
>>>>> #4  0x0000000080216df2 in blk_check_request (blk=blk@entry=0x3ffb17e7960, 
>>>>>     sector_num=sector_num@entry=9626888, nb_sectors=nb_sectors@entry=32)
>>>>>     at /data/git/yyy/qemu/block/block-backend.c:589
>>>>> #5  0x0000000080217ee8 in blk_aio_readv (blk=0x3ffb17e7960, sector_num=
>>>>>     9626888, iov=0x8098c658, nb_sectors=<optimized out>, cb=
>>>>>     0x80081150 <virtio_blk_rw_complete>, opaque=0x80980620)
>>>>>     at /data/git/yyy/qemu/block/block-backend.c:727
>>>>> #6  0x000000008008186e in submit_requests (niov=<optimized out>, 
>>>>>     num_reqs=<optimized out>, start=<optimized out>, mrb=<optimized out>, 
>>>>>     blk=<optimized out>) at /data/git/yyy/qemu/hw/block/virtio-blk.c:366
>>>>> #7  virtio_blk_submit_multireq (mrb=<optimized out>, blk=<optimized out>)
>>>>>     at /data/git/yyy/qemu/hw/block/virtio-blk.c:444
>>>>> #8  virtio_blk_submit_multireq (blk=0x3ffb17e7960, mrb=0x3ffffffeb58)
>>>>>     at /data/git/yyy/qemu/hw/block/virtio-blk.c:389
>>>>> #9  0x00000000800823ee in virtio_blk_handle_output (vdev=<optimized out>, 
>>>>>     vq=<optimized out>) at /data/git/yyy/qemu/hw/block/virtio-blk.c:615
>>>>> #10 0x00000000801e367e in aio_dispatch (ctx=0x80918520)
>>>>>     at /data/git/yyy/qemu/aio-posix.c:326
>>>>> #11 0x00000000801d28b0 in aio_ctx_dispatch (source=<optimized out>, 
>>>>>     callback=<optimized out>, user_data=<optimized out>)
>>>>>     at /data/git/yyy/qemu/async.c:231
>>>>> #12 0x000003fffd36a05a in g_main_context_dispatch ()
>>>>>    from /lib64/libglib-2.0.so.0
>>>>> #13 0x00000000801e0ffa in glib_pollfds_poll ()
>>>>>     at /data/git/yyy/qemu/main-loop.c:211
>>>>> #14 os_host_main_loop_wait (timeout=<optimized out>)
>>>>>     at /data/git/yyy/qemu/main-loop.c:256
>>>>> #15 main_loop_wait (nonblocking=<optimized out>)
>>>>>     at /data/git/yyy/qemu/main-loop.c:504
>>>>> #16 0x00000000800148a6 in main_loop () at /data/git/yyy/qemu/vl.c:1923
>>>>> #17 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>)
>>>>>     at /data/git/yyy/qemu/vl.c:4684
>>>>>
>>>>> Relevant part of command line:
>>>>>
>>>>> -drive file=/dev/sda,if=none,id=drive-virtio-disk0,format=raw,serial=ccwzfcp1,cache=none -device virtio-blk-ccw,devno=fe.0.0001,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,scsi=off
>>>>
>>>> I played around a bit. The main part of this change seems to be calling
>>>> wait_serialising_requests() conditionally; reverting this makes the
>>>> guest boot again.
>>>>
>>>> I then tried to find out when wait_serialising_requests() was NOT
>>>> called and added fprintfs: well, it was _always_ called. I then added a
>>>> fprintf for flags at the beginning of the function: this produced a
>>>> segfault no matter whether wait_serialising_requests() was called
>>>> conditionally or unconditionally. Weird race?
>>>>
>>>> Anything further I can do? I guess this patch fixes a bug for someone,
>>>> but it means insta-death for my setup...
>>>
>>> If it happens immediately, perhaps running under valgrind is possible
>>> and could give some hints about what happened with blk->bs?
>>
>> Just a quick update: This triggers on a qemu built with a not-so-fresh
>> gcc 4.7.2 (and it seems to depend on compiler optimizations as well).
>> We can't trigger it with newer gccs. No hints from valgrind, sadly.
>> Investigation ongoing.
>>
>>
> 
> Some updates after looking at Cornelias system. It seem to be related to 
> the F18 gcc that is still on that test system.
> 
> Problem happens when hw/block/virtio-blk.c is compiled
> with -O2 and goes away with -O1 and -O0 (I trimmed that down to
> -fexpensive-optimizations) 
> 
> The system calls virtio_blk_submit_multireq 26 times and then it messes
> up the blk pointer:
> 
> (gdb) display blk->name
> 1: blk->name = 0x80894300 "drive-virtio-disk0"
> (gdb) next
> 419	            if (niov + req->qiov.niov > IOV_MAX) {
> 1: blk->name = 0x80894300 "drive-virtio-disk0"
> (gdb) 
> 424	            if (req->qiov.size / BDRV_SECTOR_SIZE + nb_sectors > max_xfer_len) {
> 1: blk->name = 0x80894300 "drive-virtio-disk0"
> (gdb) 
> 419	            if (niov + req->qiov.niov > IOV_MAX) {
> 1: blk->name = 0x80894300 "drive-virtio-disk0"
> (gdb) 
> 429	            if (sector_num + nb_sectors != req->sector_num) {
> 1: blk->name = 0x80894300 "drive-virtio-disk0"
> (gdb) 
> 434	                submit_requests(blk, mrb, start, num_reqs, niov);
> 1: blk->name = 0x80894300 "drive-virtio-disk0"
> (gdb) 
> 413	    for (i = 0; i < mrb->num_reqs; i++) {
> 1: blk->name = 0x8089ae40 ""
> 
> 
> uninlining submit_request makes the problem go away, so might be a
> gcc bug in Fedora18. I am now going to look at the disassembly to be
> sure.

Not a compiler bug. gcc uses a floating point register 8 to spill
the pointer of blk (which is call saved) submit_request will later
on call  qemu_coroutine_enter and after returning from 
qemu_coroutine_enter, the fpr8 contains junk. Not sure yet, what happened.

Christian

Re: [Qemu-devel] [PULL for-2.5 2/4] block: Don't wait serialising for non-COR read requests

[Kevin Wolf]

Am 08.12.2015 um 14:28 hat Christian Borntraeger geschrieben:
> On 12/08/2015 01:30 PM, Christian Borntraeger wrote:
> > On 12/08/2015 01:00 PM, Cornelia Huck wrote:
> >> On Tue, 8 Dec 2015 10:59:54 +0100
> >> Kevin Wolf <kwolf@redhat.com> wrote:
> >>
> >>> Am 07.12.2015 um 17:42 hat Cornelia Huck geschrieben:
> >>>> On Mon, 7 Dec 2015 11:02:51 +0100
> >>>> Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
> >>>>
> >>>>> On Thu,  3 Dec 2015 13:00:00 +0800
> >>>>> Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >>>>>
> >>>>>> From: Fam Zheng <famz@redhat.com>
> >>>>>>
> >>>>>> The assertion problem was noticed in 06c3916b35a, but it wasn't
> >>>>>> completely fixed, because even though the req is not marked as
> >>>>>> serialising, it still gets serialised by wait_serialising_requests
> >>>>>> against other serialising requests, which could lead to the same
> >>>>>> assertion failure.
> >>>>>>
> >>>>>> Fix it by even more explicitly skipping the serialising for this
> >>>>>> specific case.
> >>>>>>
> >>>>>> Signed-off-by: Fam Zheng <famz@redhat.com>
> >>>>>> Message-id: 1448962590-2842-2-git-send-email-famz@redhat.com
> >>>>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> >>>>>> ---
> >>>>>>  block/backup.c        |  2 +-
> >>>>>>  block/io.c            | 12 +++++++-----
> >>>>>>  include/block/block.h |  4 ++--
> >>>>>>  trace-events          |  2 +-
> >>>>>>  4 files changed, 11 insertions(+), 9 deletions(-)
> >>>>>
> >>>>> This one causes segfaults for me:
> >>>>>
> >>>>> Program received signal SIGSEGV, Segmentation fault.
> >>>>> bdrv_is_inserted (bs=0x800000000000) at /data/git/yyy/qemu/block.c:3071
> >>>>> 3071	    if (!drv) {
> >>>>>
> >>>>> (gdb) bt
> >>>>> #0  bdrv_is_inserted (bs=0x800000000000) at /data/git/yyy/qemu/block.c:3071
> >>>
> >>> This looks like some kind of memory corruption that hit blk->bs. It's
> >>> most definitely not a valid pointer anyway.
> >>>
> >>>>> #1  0x0000000080216974 in blk_is_inserted (blk=<optimized out>)
> >>>>>     at /data/git/yyy/qemu/block/block-backend.c:986
> >>>>> #2  0x00000000802169c6 in blk_is_available (blk=blk@entry=0x3ffb17e7960)
> >>>>>     at /data/git/yyy/qemu/block/block-backend.c:991
> >>>>> #3  0x0000000080216d12 in blk_check_byte_request (blk=blk@entry=0x3ffb17e7960, 
> >>>>>     offset=offset@entry=4928966656, size=16384)
> >>>>>     at /data/git/yyy/qemu/block/block-backend.c:558
> >>>>> #4  0x0000000080216df2 in blk_check_request (blk=blk@entry=0x3ffb17e7960, 
> >>>>>     sector_num=sector_num@entry=9626888, nb_sectors=nb_sectors@entry=32)
> >>>>>     at /data/git/yyy/qemu/block/block-backend.c:589
> >>>>> #5  0x0000000080217ee8 in blk_aio_readv (blk=0x3ffb17e7960, sector_num=
> >>>>>     9626888, iov=0x8098c658, nb_sectors=<optimized out>, cb=
> >>>>>     0x80081150 <virtio_blk_rw_complete>, opaque=0x80980620)
> >>>>>     at /data/git/yyy/qemu/block/block-backend.c:727
> >>>>> #6  0x000000008008186e in submit_requests (niov=<optimized out>, 
> >>>>>     num_reqs=<optimized out>, start=<optimized out>, mrb=<optimized out>, 
> >>>>>     blk=<optimized out>) at /data/git/yyy/qemu/hw/block/virtio-blk.c:366
> >>>>> #7  virtio_blk_submit_multireq (mrb=<optimized out>, blk=<optimized out>)
> >>>>>     at /data/git/yyy/qemu/hw/block/virtio-blk.c:444
> >>>>> #8  virtio_blk_submit_multireq (blk=0x3ffb17e7960, mrb=0x3ffffffeb58)
> >>>>>     at /data/git/yyy/qemu/hw/block/virtio-blk.c:389
> >>>>> #9  0x00000000800823ee in virtio_blk_handle_output (vdev=<optimized out>, 
> >>>>>     vq=<optimized out>) at /data/git/yyy/qemu/hw/block/virtio-blk.c:615
> >>>>> #10 0x00000000801e367e in aio_dispatch (ctx=0x80918520)
> >>>>>     at /data/git/yyy/qemu/aio-posix.c:326
> >>>>> #11 0x00000000801d28b0 in aio_ctx_dispatch (source=<optimized out>, 
> >>>>>     callback=<optimized out>, user_data=<optimized out>)
> >>>>>     at /data/git/yyy/qemu/async.c:231
> >>>>> #12 0x000003fffd36a05a in g_main_context_dispatch ()
> >>>>>    from /lib64/libglib-2.0.so.0
> >>>>> #13 0x00000000801e0ffa in glib_pollfds_poll ()
> >>>>>     at /data/git/yyy/qemu/main-loop.c:211
> >>>>> #14 os_host_main_loop_wait (timeout=<optimized out>)
> >>>>>     at /data/git/yyy/qemu/main-loop.c:256
> >>>>> #15 main_loop_wait (nonblocking=<optimized out>)
> >>>>>     at /data/git/yyy/qemu/main-loop.c:504
> >>>>> #16 0x00000000800148a6 in main_loop () at /data/git/yyy/qemu/vl.c:1923
> >>>>> #17 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>)
> >>>>>     at /data/git/yyy/qemu/vl.c:4684
> >>>>>
> >>>>> Relevant part of command line:
> >>>>>
> >>>>> -drive file=/dev/sda,if=none,id=drive-virtio-disk0,format=raw,serial=ccwzfcp1,cache=none -device virtio-blk-ccw,devno=fe.0.0001,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,scsi=off
> >>>>
> >>>> I played around a bit. The main part of this change seems to be calling
> >>>> wait_serialising_requests() conditionally; reverting this makes the
> >>>> guest boot again.
> >>>>
> >>>> I then tried to find out when wait_serialising_requests() was NOT
> >>>> called and added fprintfs: well, it was _always_ called. I then added a
> >>>> fprintf for flags at the beginning of the function: this produced a
> >>>> segfault no matter whether wait_serialising_requests() was called
> >>>> conditionally or unconditionally. Weird race?
> >>>>
> >>>> Anything further I can do? I guess this patch fixes a bug for someone,
> >>>> but it means insta-death for my setup...
> >>>
> >>> If it happens immediately, perhaps running under valgrind is possible
> >>> and could give some hints about what happened with blk->bs?
> >>
> >> Just a quick update: This triggers on a qemu built with a not-so-fresh
> >> gcc 4.7.2 (and it seems to depend on compiler optimizations as well).
> >> We can't trigger it with newer gccs. No hints from valgrind, sadly.
> >> Investigation ongoing.
> >>
> >>
> > 
> > Some updates after looking at Cornelias system. It seem to be related to 
> > the F18 gcc that is still on that test system.
> > 
> > Problem happens when hw/block/virtio-blk.c is compiled
> > with -O2 and goes away with -O1 and -O0 (I trimmed that down to
> > -fexpensive-optimizations) 
> > 
> > The system calls virtio_blk_submit_multireq 26 times and then it messes
> > up the blk pointer:
> > 
> > (gdb) display blk->name
> > 1: blk->name = 0x80894300 "drive-virtio-disk0"
> > (gdb) next
> > 419	            if (niov + req->qiov.niov > IOV_MAX) {
> > 1: blk->name = 0x80894300 "drive-virtio-disk0"
> > (gdb) 
> > 424	            if (req->qiov.size / BDRV_SECTOR_SIZE + nb_sectors > max_xfer_len) {
> > 1: blk->name = 0x80894300 "drive-virtio-disk0"
> > (gdb) 
> > 419	            if (niov + req->qiov.niov > IOV_MAX) {
> > 1: blk->name = 0x80894300 "drive-virtio-disk0"
> > (gdb) 
> > 429	            if (sector_num + nb_sectors != req->sector_num) {
> > 1: blk->name = 0x80894300 "drive-virtio-disk0"
> > (gdb) 
> > 434	                submit_requests(blk, mrb, start, num_reqs, niov);
> > 1: blk->name = 0x80894300 "drive-virtio-disk0"
> > (gdb) 
> > 413	    for (i = 0; i < mrb->num_reqs; i++) {
> > 1: blk->name = 0x8089ae40 ""
> > 
> > 
> > uninlining submit_request makes the problem go away, so might be a
> > gcc bug in Fedora18. I am now going to look at the disassembly to be
> > sure.
> 
> Not a compiler bug. gcc uses a floating point register 8 to spill
> the pointer of blk (which is call saved) submit_request will later
> on call  qemu_coroutine_enter and after returning from 
> qemu_coroutine_enter, the fpr8 contains junk. Not sure yet, what happened.

Coroutines don't save the FPU state, so you're not supposed to use
floating point operations inside coroutines. That the compiler spills
some integer value into a floating point register is a bit nasty...

Kevin

Re: [Qemu-devel] [PULL for-2.5 2/4] block: Don't wait serialising for non-COR read requests

[Christian Borntraeger]

On 12/08/2015 02:45 PM, Kevin Wolf wrote:
> Am 08.12.2015 um 14:28 hat Christian Borntraeger geschrieben:
>> On 12/08/2015 01:30 PM, Christian Borntraeger wrote:
>>> On 12/08/2015 01:00 PM, Cornelia Huck wrote:
>>>> On Tue, 8 Dec 2015 10:59:54 +0100
>>>> Kevin Wolf <kwolf@redhat.com> wrote:
>>>>
>>>>> Am 07.12.2015 um 17:42 hat Cornelia Huck geschrieben:
>>>>>> On Mon, 7 Dec 2015 11:02:51 +0100
>>>>>> Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
>>>>>>
>>>>>>> On Thu,  3 Dec 2015 13:00:00 +0800
>>>>>>> Stefan Hajnoczi <stefanha@redhat.com> wrote:
>>>>>>>
>>>>>>>> From: Fam Zheng <famz@redhat.com>
>>>>>>>>
>>>>>>>> The assertion problem was noticed in 06c3916b35a, but it wasn't
>>>>>>>> completely fixed, because even though the req is not marked as
>>>>>>>> serialising, it still gets serialised by wait_serialising_requests
>>>>>>>> against other serialising requests, which could lead to the same
>>>>>>>> assertion failure.
>>>>>>>>
>>>>>>>> Fix it by even more explicitly skipping the serialising for this
>>>>>>>> specific case.
>>>>>>>>
>>>>>>>> Signed-off-by: Fam Zheng <famz@redhat.com>
>>>>>>>> Message-id: 1448962590-2842-2-git-send-email-famz@redhat.com
>>>>>>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>>>>>>> ---
>>>>>>>>  block/backup.c        |  2 +-
>>>>>>>>  block/io.c            | 12 +++++++-----
>>>>>>>>  include/block/block.h |  4 ++--
>>>>>>>>  trace-events          |  2 +-
>>>>>>>>  4 files changed, 11 insertions(+), 9 deletions(-)
>>>>>>>
>>>>>>> This one causes segfaults for me:
>>>>>>>
>>>>>>> Program received signal SIGSEGV, Segmentation fault.
>>>>>>> bdrv_is_inserted (bs=0x800000000000) at /data/git/yyy/qemu/block.c:3071
>>>>>>> 3071	    if (!drv) {
>>>>>>>
>>>>>>> (gdb) bt
>>>>>>> #0  bdrv_is_inserted (bs=0x800000000000) at /data/git/yyy/qemu/block.c:3071
>>>>>
>>>>> This looks like some kind of memory corruption that hit blk->bs. It's
>>>>> most definitely not a valid pointer anyway.
>>>>>
>>>>>>> #1  0x0000000080216974 in blk_is_inserted (blk=<optimized out>)
>>>>>>>     at /data/git/yyy/qemu/block/block-backend.c:986
>>>>>>> #2  0x00000000802169c6 in blk_is_available (blk=blk@entry=0x3ffb17e7960)
>>>>>>>     at /data/git/yyy/qemu/block/block-backend.c:991
>>>>>>> #3  0x0000000080216d12 in blk_check_byte_request (blk=blk@entry=0x3ffb17e7960, 
>>>>>>>     offset=offset@entry=4928966656, size=16384)
>>>>>>>     at /data/git/yyy/qemu/block/block-backend.c:558
>>>>>>> #4  0x0000000080216df2 in blk_check_request (blk=blk@entry=0x3ffb17e7960, 
>>>>>>>     sector_num=sector_num@entry=9626888, nb_sectors=nb_sectors@entry=32)
>>>>>>>     at /data/git/yyy/qemu/block/block-backend.c:589
>>>>>>> #5  0x0000000080217ee8 in blk_aio_readv (blk=0x3ffb17e7960, sector_num=
>>>>>>>     9626888, iov=0x8098c658, nb_sectors=<optimized out>, cb=
>>>>>>>     0x80081150 <virtio_blk_rw_complete>, opaque=0x80980620)
>>>>>>>     at /data/git/yyy/qemu/block/block-backend.c:727
>>>>>>> #6  0x000000008008186e in submit_requests (niov=<optimized out>, 
>>>>>>>     num_reqs=<optimized out>, start=<optimized out>, mrb=<optimized out>, 
>>>>>>>     blk=<optimized out>) at /data/git/yyy/qemu/hw/block/virtio-blk.c:366
>>>>>>> #7  virtio_blk_submit_multireq (mrb=<optimized out>, blk=<optimized out>)
>>>>>>>     at /data/git/yyy/qemu/hw/block/virtio-blk.c:444
>>>>>>> #8  virtio_blk_submit_multireq (blk=0x3ffb17e7960, mrb=0x3ffffffeb58)
>>>>>>>     at /data/git/yyy/qemu/hw/block/virtio-blk.c:389
>>>>>>> #9  0x00000000800823ee in virtio_blk_handle_output (vdev=<optimized out>, 
>>>>>>>     vq=<optimized out>) at /data/git/yyy/qemu/hw/block/virtio-blk.c:615
>>>>>>> #10 0x00000000801e367e in aio_dispatch (ctx=0x80918520)
>>>>>>>     at /data/git/yyy/qemu/aio-posix.c:326
>>>>>>> #11 0x00000000801d28b0 in aio_ctx_dispatch (source=<optimized out>, 
>>>>>>>     callback=<optimized out>, user_data=<optimized out>)
>>>>>>>     at /data/git/yyy/qemu/async.c:231
>>>>>>> #12 0x000003fffd36a05a in g_main_context_dispatch ()
>>>>>>>    from /lib64/libglib-2.0.so.0
>>>>>>> #13 0x00000000801e0ffa in glib_pollfds_poll ()
>>>>>>>     at /data/git/yyy/qemu/main-loop.c:211
>>>>>>> #14 os_host_main_loop_wait (timeout=<optimized out>)
>>>>>>>     at /data/git/yyy/qemu/main-loop.c:256
>>>>>>> #15 main_loop_wait (nonblocking=<optimized out>)
>>>>>>>     at /data/git/yyy/qemu/main-loop.c:504
>>>>>>> #16 0x00000000800148a6 in main_loop () at /data/git/yyy/qemu/vl.c:1923
>>>>>>> #17 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>)
>>>>>>>     at /data/git/yyy/qemu/vl.c:4684
>>>>>>>
>>>>>>> Relevant part of command line:
>>>>>>>
>>>>>>> -drive file=/dev/sda,if=none,id=drive-virtio-disk0,format=raw,serial=ccwzfcp1,cache=none -device virtio-blk-ccw,devno=fe.0.0001,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,scsi=off
>>>>>>
>>>>>> I played around a bit. The main part of this change seems to be calling
>>>>>> wait_serialising_requests() conditionally; reverting this makes the
>>>>>> guest boot again.
>>>>>>
>>>>>> I then tried to find out when wait_serialising_requests() was NOT
>>>>>> called and added fprintfs: well, it was _always_ called. I then added a
>>>>>> fprintf for flags at the beginning of the function: this produced a
>>>>>> segfault no matter whether wait_serialising_requests() was called
>>>>>> conditionally or unconditionally. Weird race?
>>>>>>
>>>>>> Anything further I can do? I guess this patch fixes a bug for someone,
>>>>>> but it means insta-death for my setup...
>>>>>
>>>>> If it happens immediately, perhaps running under valgrind is possible
>>>>> and could give some hints about what happened with blk->bs?
>>>>
>>>> Just a quick update: This triggers on a qemu built with a not-so-fresh
>>>> gcc 4.7.2 (and it seems to depend on compiler optimizations as well).
>>>> We can't trigger it with newer gccs. No hints from valgrind, sadly.
>>>> Investigation ongoing.
>>>>
>>>>
>>>
>>> Some updates after looking at Cornelias system. It seem to be related to 
>>> the F18 gcc that is still on that test system.
>>>
>>> Problem happens when hw/block/virtio-blk.c is compiled
>>> with -O2 and goes away with -O1 and -O0 (I trimmed that down to
>>> -fexpensive-optimizations) 
>>>
>>> The system calls virtio_blk_submit_multireq 26 times and then it messes
>>> up the blk pointer:
>>>
>>> (gdb) display blk->name
>>> 1: blk->name = 0x80894300 "drive-virtio-disk0"
>>> (gdb) next
>>> 419	            if (niov + req->qiov.niov > IOV_MAX) {
>>> 1: blk->name = 0x80894300 "drive-virtio-disk0"
>>> (gdb) 
>>> 424	            if (req->qiov.size / BDRV_SECTOR_SIZE + nb_sectors > max_xfer_len) {
>>> 1: blk->name = 0x80894300 "drive-virtio-disk0"
>>> (gdb) 
>>> 419	            if (niov + req->qiov.niov > IOV_MAX) {
>>> 1: blk->name = 0x80894300 "drive-virtio-disk0"
>>> (gdb) 
>>> 429	            if (sector_num + nb_sectors != req->sector_num) {
>>> 1: blk->name = 0x80894300 "drive-virtio-disk0"
>>> (gdb) 
>>> 434	                submit_requests(blk, mrb, start, num_reqs, niov);
>>> 1: blk->name = 0x80894300 "drive-virtio-disk0"
>>> (gdb) 
>>> 413	    for (i = 0; i < mrb->num_reqs; i++) {
>>> 1: blk->name = 0x8089ae40 ""
>>>
>>>
>>> uninlining submit_request makes the problem go away, so might be a
>>> gcc bug in Fedora18. I am now going to look at the disassembly to be
>>> sure.
>>
>> Not a compiler bug. gcc uses a floating point register 8 to spill
>> the pointer of blk (which is call saved) submit_request will later
>> on call  qemu_coroutine_enter and after returning from 
>> qemu_coroutine_enter, the fpr8 contains junk. Not sure yet, what happened.
> 
> Coroutines don't save the FPU state, so you're not supposed to use
> floating point operations inside coroutines. That the compiler spills
> some integer value into a floating point register is a bit nasty...

Just checked.  bdrv_aligned_preadv does also use fprs (also for filling
and spilling). Some versions of gcc seem to like that as the LDGR and LGDR
instructions are pretty cheap and move the content from/to fprs in a bitwise
fashion. So this coroutine DOES trash floating point registers.

Without the patch gcc seems to be fine with the 16 gprs and does not
spilling/filling from/to fprs in bdrv_aligned_preadv.

Christian

Re: [Qemu-devel] [PULL for-2.5 2/4] block: Don't wait serialising for non-COR read requests

[Christian Borntraeger]

On 12/08/2015 02:58 PM, Christian Borntraeger wrote:
[...9
>>>
>>> Not a compiler bug. gcc uses a floating point register 8 to spill
>>> the pointer of blk (which is call saved) submit_request will later
>>> on call  qemu_coroutine_enter and after returning from 
>>> qemu_coroutine_enter, the fpr8 contains junk. Not sure yet, what happened.
>>
>> Coroutines don't save the FPU state, so you're not supposed to use
>> floating point operations inside coroutines. That the compiler spills
>> some integer value into a floating point register is a bit nasty...
> 
> Just checked.  bdrv_aligned_preadv does also use fprs (also for filling
> and spilling). Some versions of gcc seem to like that as the LDGR and LGDR
> instructions are pretty cheap and move the content from/to fprs in a bitwise
> fashion. So this coroutine DOES trash floating point registers.
> 
> Without the patch gcc seems to be fine with the 16 gprs and does not
> spilling/filling from/to fprs in bdrv_aligned_preadv.
> 
> Christian

Kevin,

I am wondering. gcc saves/restores f8 in the generated code for the
coroutine and setjmp/longjmp also save/restore the fprs. why do 
coroutines do not save the FPU state (which code does a light weight
switching)

Christian

Re: [Qemu-devel] [PULL for-2.5 2/4] block: Don't wait serialising for non-COR read requests

[Kevin Wolf]

Am 08.12.2015 um 14:58 hat Christian Borntraeger geschrieben:
> On 12/08/2015 02:45 PM, Kevin Wolf wrote:
> > Am 08.12.2015 um 14:28 hat Christian Borntraeger geschrieben:
> >> On 12/08/2015 01:30 PM, Christian Borntraeger wrote:
> >>> On 12/08/2015 01:00 PM, Cornelia Huck wrote:
> >>>> On Tue, 8 Dec 2015 10:59:54 +0100
> >>>> Kevin Wolf <kwolf@redhat.com> wrote:
> >>>>
> >>>>> Am 07.12.2015 um 17:42 hat Cornelia Huck geschrieben:
> >>>>>> On Mon, 7 Dec 2015 11:02:51 +0100
> >>>>>> Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
> >>>>>>
> >>>>>>> On Thu,  3 Dec 2015 13:00:00 +0800
> >>>>>>> Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >>>>>>>
> >>>>>>>> From: Fam Zheng <famz@redhat.com>
> >>>>>>>>
> >>>>>>>> The assertion problem was noticed in 06c3916b35a, but it wasn't
> >>>>>>>> completely fixed, because even though the req is not marked as
> >>>>>>>> serialising, it still gets serialised by wait_serialising_requests
> >>>>>>>> against other serialising requests, which could lead to the same
> >>>>>>>> assertion failure.
> >>>>>>>>
> >>>>>>>> Fix it by even more explicitly skipping the serialising for this
> >>>>>>>> specific case.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Fam Zheng <famz@redhat.com>
> >>>>>>>> Message-id: 1448962590-2842-2-git-send-email-famz@redhat.com
> >>>>>>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> >>>>>>>> ---
> >>>>>>>>  block/backup.c        |  2 +-
> >>>>>>>>  block/io.c            | 12 +++++++-----
> >>>>>>>>  include/block/block.h |  4 ++--
> >>>>>>>>  trace-events          |  2 +-
> >>>>>>>>  4 files changed, 11 insertions(+), 9 deletions(-)
> >>>>>>>
> >>>>>>> This one causes segfaults for me:
> >>>>>>>
> >>>>>>> Program received signal SIGSEGV, Segmentation fault.
> >>>>>>> bdrv_is_inserted (bs=0x800000000000) at /data/git/yyy/qemu/block.c:3071
> >>>>>>> 3071	    if (!drv) {
> >>>>>>>
> >>>>>>> (gdb) bt
> >>>>>>> #0  bdrv_is_inserted (bs=0x800000000000) at /data/git/yyy/qemu/block.c:3071
> >>>>>
> >>>>> This looks like some kind of memory corruption that hit blk->bs. It's
> >>>>> most definitely not a valid pointer anyway.
> >>>>>
> >>>>>>> #1  0x0000000080216974 in blk_is_inserted (blk=<optimized out>)
> >>>>>>>     at /data/git/yyy/qemu/block/block-backend.c:986
> >>>>>>> #2  0x00000000802169c6 in blk_is_available (blk=blk@entry=0x3ffb17e7960)
> >>>>>>>     at /data/git/yyy/qemu/block/block-backend.c:991
> >>>>>>> #3  0x0000000080216d12 in blk_check_byte_request (blk=blk@entry=0x3ffb17e7960, 
> >>>>>>>     offset=offset@entry=4928966656, size=16384)
> >>>>>>>     at /data/git/yyy/qemu/block/block-backend.c:558
> >>>>>>> #4  0x0000000080216df2 in blk_check_request (blk=blk@entry=0x3ffb17e7960, 
> >>>>>>>     sector_num=sector_num@entry=9626888, nb_sectors=nb_sectors@entry=32)
> >>>>>>>     at /data/git/yyy/qemu/block/block-backend.c:589
> >>>>>>> #5  0x0000000080217ee8 in blk_aio_readv (blk=0x3ffb17e7960, sector_num=
> >>>>>>>     9626888, iov=0x8098c658, nb_sectors=<optimized out>, cb=
> >>>>>>>     0x80081150 <virtio_blk_rw_complete>, opaque=0x80980620)
> >>>>>>>     at /data/git/yyy/qemu/block/block-backend.c:727
> >>>>>>> #6  0x000000008008186e in submit_requests (niov=<optimized out>, 
> >>>>>>>     num_reqs=<optimized out>, start=<optimized out>, mrb=<optimized out>, 
> >>>>>>>     blk=<optimized out>) at /data/git/yyy/qemu/hw/block/virtio-blk.c:366
> >>>>>>> #7  virtio_blk_submit_multireq (mrb=<optimized out>, blk=<optimized out>)
> >>>>>>>     at /data/git/yyy/qemu/hw/block/virtio-blk.c:444
> >>>>>>> #8  virtio_blk_submit_multireq (blk=0x3ffb17e7960, mrb=0x3ffffffeb58)
> >>>>>>>     at /data/git/yyy/qemu/hw/block/virtio-blk.c:389
> >>>>>>> #9  0x00000000800823ee in virtio_blk_handle_output (vdev=<optimized out>, 
> >>>>>>>     vq=<optimized out>) at /data/git/yyy/qemu/hw/block/virtio-blk.c:615
> >>>>>>> #10 0x00000000801e367e in aio_dispatch (ctx=0x80918520)
> >>>>>>>     at /data/git/yyy/qemu/aio-posix.c:326
> >>>>>>> #11 0x00000000801d28b0 in aio_ctx_dispatch (source=<optimized out>, 
> >>>>>>>     callback=<optimized out>, user_data=<optimized out>)
> >>>>>>>     at /data/git/yyy/qemu/async.c:231
> >>>>>>> #12 0x000003fffd36a05a in g_main_context_dispatch ()
> >>>>>>>    from /lib64/libglib-2.0.so.0
> >>>>>>> #13 0x00000000801e0ffa in glib_pollfds_poll ()
> >>>>>>>     at /data/git/yyy/qemu/main-loop.c:211
> >>>>>>> #14 os_host_main_loop_wait (timeout=<optimized out>)
> >>>>>>>     at /data/git/yyy/qemu/main-loop.c:256
> >>>>>>> #15 main_loop_wait (nonblocking=<optimized out>)
> >>>>>>>     at /data/git/yyy/qemu/main-loop.c:504
> >>>>>>> #16 0x00000000800148a6 in main_loop () at /data/git/yyy/qemu/vl.c:1923
> >>>>>>> #17 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>)
> >>>>>>>     at /data/git/yyy/qemu/vl.c:4684
> >>>>>>>
> >>>>>>> Relevant part of command line:
> >>>>>>>
> >>>>>>> -drive file=/dev/sda,if=none,id=drive-virtio-disk0,format=raw,serial=ccwzfcp1,cache=none -device virtio-blk-ccw,devno=fe.0.0001,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1,scsi=off
> >>>>>>
> >>>>>> I played around a bit. The main part of this change seems to be calling
> >>>>>> wait_serialising_requests() conditionally; reverting this makes the
> >>>>>> guest boot again.
> >>>>>>
> >>>>>> I then tried to find out when wait_serialising_requests() was NOT
> >>>>>> called and added fprintfs: well, it was _always_ called. I then added a
> >>>>>> fprintf for flags at the beginning of the function: this produced a
> >>>>>> segfault no matter whether wait_serialising_requests() was called
> >>>>>> conditionally or unconditionally. Weird race?
> >>>>>>
> >>>>>> Anything further I can do? I guess this patch fixes a bug for someone,
> >>>>>> but it means insta-death for my setup...
> >>>>>
> >>>>> If it happens immediately, perhaps running under valgrind is possible
> >>>>> and could give some hints about what happened with blk->bs?
> >>>>
> >>>> Just a quick update: This triggers on a qemu built with a not-so-fresh
> >>>> gcc 4.7.2 (and it seems to depend on compiler optimizations as well).
> >>>> We can't trigger it with newer gccs. No hints from valgrind, sadly.
> >>>> Investigation ongoing.
> >>>>
> >>>>
> >>>
> >>> Some updates after looking at Cornelias system. It seem to be related to 
> >>> the F18 gcc that is still on that test system.
> >>>
> >>> Problem happens when hw/block/virtio-blk.c is compiled
> >>> with -O2 and goes away with -O1 and -O0 (I trimmed that down to
> >>> -fexpensive-optimizations) 
> >>>
> >>> The system calls virtio_blk_submit_multireq 26 times and then it messes
> >>> up the blk pointer:
> >>>
> >>> (gdb) display blk->name
> >>> 1: blk->name = 0x80894300 "drive-virtio-disk0"
> >>> (gdb) next
> >>> 419	            if (niov + req->qiov.niov > IOV_MAX) {
> >>> 1: blk->name = 0x80894300 "drive-virtio-disk0"
> >>> (gdb) 
> >>> 424	            if (req->qiov.size / BDRV_SECTOR_SIZE + nb_sectors > max_xfer_len) {
> >>> 1: blk->name = 0x80894300 "drive-virtio-disk0"
> >>> (gdb) 
> >>> 419	            if (niov + req->qiov.niov > IOV_MAX) {
> >>> 1: blk->name = 0x80894300 "drive-virtio-disk0"
> >>> (gdb) 
> >>> 429	            if (sector_num + nb_sectors != req->sector_num) {
> >>> 1: blk->name = 0x80894300 "drive-virtio-disk0"
> >>> (gdb) 
> >>> 434	                submit_requests(blk, mrb, start, num_reqs, niov);
> >>> 1: blk->name = 0x80894300 "drive-virtio-disk0"
> >>> (gdb) 
> >>> 413	    for (i = 0; i < mrb->num_reqs; i++) {
> >>> 1: blk->name = 0x8089ae40 ""
> >>>
> >>>
> >>> uninlining submit_request makes the problem go away, so might be a
> >>> gcc bug in Fedora18. I am now going to look at the disassembly to be
> >>> sure.
> >>
> >> Not a compiler bug. gcc uses a floating point register 8 to spill
> >> the pointer of blk (which is call saved) submit_request will later
> >> on call  qemu_coroutine_enter and after returning from 
> >> qemu_coroutine_enter, the fpr8 contains junk. Not sure yet, what happened.
> > 
> > Coroutines don't save the FPU state, so you're not supposed to use
> > floating point operations inside coroutines. That the compiler spills
> > some integer value into a floating point register is a bit nasty...
> 
> Just checked.  bdrv_aligned_preadv does also use fprs (also for filling
> and spilling). Some versions of gcc seem to like that as the LDGR and LGDR
> instructions are pretty cheap and move the content from/to fprs in a bitwise
> fashion. So this coroutine DOES trash floating point registers.
> 
> Without the patch gcc seems to be fine with the 16 gprs and does not
> spilling/filling from/to fprs in bdrv_aligned_preadv.

Actually, on closer look it seems that the reason why there is no code
for saving the floating point registers in setjmp() on x86 is that they
are caller-save registers anyway, so it doesn't have to. Otherwise the
internet seems to be of the opinion that longjmp() must indeed restore
floating point registers.

So this might be a libc bug on s390 then.

Kevin

Re: [Qemu-devel] [PULL for-2.5 2/4] block: Don't wait serialising for non-COR read requests

[Peter Maydell]

On 8 December 2015 at 13:45, Kevin Wolf <kwolf@redhat.com> wrote:
> Coroutines don't save the FPU state, so you're not supposed to use
> floating point operations inside coroutines. That the compiler spills
> some integer value into a floating point register is a bit nasty...

The compiler will happily use FP registers even for apparently
integer code if it thinks that is a better way to do it (eg on
some CPUs doing memcpy and other kinds of block data move may
go faster via the FPU registers, or it might be faster to spill
an integer register into an FP register rather than spilling it
to memory). As I see you've already determined, it's the
job of setjmp/longjmp to make sure that everything is saved
and restored correctly, fp or otherwise...

thanks
-- PMM

Re: [Qemu-devel] [PULL for-2.5 2/4] block: Don't wait serialising for non-COR read requests

[Christian Borntraeger]

On 12/08/2015 03:10 PM, Kevin Wolf wrote:
[...]
>>>> Not a compiler bug. gcc uses a floating point register 8 to spill
>>>> the pointer of blk (which is call saved) submit_request will later
>>>> on call  qemu_coroutine_enter and after returning from 
>>>> qemu_coroutine_enter, the fpr8 contains junk. Not sure yet, what happened.
>>>
>>> Coroutines don't save the FPU state, so you're not supposed to use
>>> floating point operations inside coroutines. That the compiler spills
>>> some integer value into a floating point register is a bit nasty...
>>
>> Just checked.  bdrv_aligned_preadv does also use fprs (also for filling
>> and spilling). Some versions of gcc seem to like that as the LDGR and LGDR
>> instructions are pretty cheap and move the content from/to fprs in a bitwise
>> fashion. So this coroutine DOES trash floating point registers.
>>
>> Without the patch gcc seems to be fine with the 16 gprs and does not
>> spilling/filling from/to fprs in bdrv_aligned_preadv.
> 
> Actually, on closer look it seems that the reason why there is no code
> for saving the floating point registers in setjmp() on x86 is that they
> are caller-save registers anyway, so it doesn't have to. Otherwise the
> internet seems to be of the opinion that longjmp() must indeed restore
> floating point registers.
> 
> So this might be a libc bug on s390 then.

Fixed with 
https://sourceware.org/ml/libc-alpha/2013-01/msg00853.html

Christian

Re: [Qemu-devel] [PULL for-2.5 2/4] block: Don't wait serialising for non-COR read requests

[Cornelia Huck]

On Tue, 8 Dec 2015 15:24:29 +0100
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 12/08/2015 03:10 PM, Kevin Wolf wrote:
> > So this might be a libc bug on s390 then.
> 
> Fixed with 
> https://sourceware.org/ml/libc-alpha/2013-01/msg00853.html

OK, so I need to upgrade that system; no bug in qemu. Thank you for
looking into this!