1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH v2 0/4] coroutine-lock: polymorphic CoQueue

Paolo Bonzini posted 4 patches 7 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180116142316.30486-1-pbonzini@redhat.com
Test checkpatch passed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test docker-quick@centos6 passed
Test ppc passed
Test s390x passed
There is a newer version of this series
block/curl.c                | 20 +++---------
fsdev/qemu-fsdev-throttle.c |  4 +--
include/qemu/compiler.h     | 40 ++++++++++++++++++++++++
include/qemu/coroutine.h    | 25 ++++++++++-----
include/qemu/lockable.h     | 75 +++++++++++++++++++++++++++++++++++++++++++++
include/qemu/thread.h       |  5 ++-
include/qemu/typedefs.h     |  4 +++
util/qemu-coroutine-lock.c  | 22 ++++++++-----
8 files changed, 159 insertions(+), 36 deletions(-)
create mode 100644 include/qemu/lockable.h
[Qemu-devel] [PATCH v2 0/4] coroutine-lock: polymorphic CoQueue
Posted by Paolo Bonzini 7 years, 9 months ago 
There are cases in which a queued coroutine must be restarted from
non-coroutine context (with qemu_co_enter_next).  In this cases,
qemu_co_enter_next also needs to be thread-safe, but it cannot use a
CoMutex and so cannot qemu_co_queue_wait.  This happens in curl (which
right now is rolling its own list of Coroutines) and will happen in
Fam's NVMe driver as well.

This series extracts the idea of a polymorphic lockable object
from my "scoped lock guard" proposal, and applies it to CoQueue.
The implementation of QemuLockable is similar to C11 _Generic, but
redone using the preprocessor and GCC builtins for compatibility.

In general, while a bit on the esoteric side, the functionality used
to emulate _Generic is fairly old in GCC, and the builtins are already
used by include/qemu/atomic.h; the series was tested with Fedora 27 (boot
Damn Small Linux via http) and CentOS 6 (compiled only).

Paolo

v1->v2: fix typos and copyright year

Paolo Bonzini (4):
  lockable: add QemuLockable
  coroutine-lock: convert CoQueue to use QemuLockable
  coroutine-lock: make qemu_co_enter_next thread-safe
  curl: convert to CoQueue

 block/curl.c                | 20 +++---------
 fsdev/qemu-fsdev-throttle.c |  4 +--
 include/qemu/compiler.h     | 40 ++++++++++++++++++++++++
 include/qemu/coroutine.h    | 25 ++++++++++-----
 include/qemu/lockable.h     | 75 +++++++++++++++++++++++++++++++++++++++++++++
 include/qemu/thread.h       |  5 ++-
 include/qemu/typedefs.h     |  4 +++
 util/qemu-coroutine-lock.c  | 22 ++++++++-----
 8 files changed, 159 insertions(+), 36 deletions(-)
 create mode 100644 include/qemu/lockable.h

-- 
2.14.3
Re: [Qemu-devel] [PATCH v2 0/4] coroutine-lock: polymorphic CoQueue
Posted by Fam Zheng 7 years, 9 months ago 
On Tue, Jan 16, 2018 at 10:23 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> There are cases in which a queued coroutine must be restarted from
> non-coroutine context (with qemu_co_enter_next).  In this cases,
> qemu_co_enter_next also needs to be thread-safe, but it cannot use a
> CoMutex and so cannot qemu_co_queue_wait.  This happens in curl (which
> right now is rolling its own list of Coroutines) and will happen in
> Fam's NVMe driver as well.
>
> This series extracts the idea of a polymorphic lockable object
> from my "scoped lock guard" proposal, and applies it to CoQueue.
> The implementation of QemuLockable is similar to C11 _Generic, but
> redone using the preprocessor and GCC builtins for compatibility.
>
> In general, while a bit on the esoteric side, the functionality used
> to emulate _Generic is fairly old in GCC, and the builtins are already
> used by include/qemu/atomic.h; the series was tested with Fedora 27 (boot
> Damn Small Linux via http) and CentOS 6 (compiled only).

I'm seeing this crash with the series:

(gdb) bt
#0  0x00007ff76204d66b in raise () at /lib64/libc.so.6
#1  0x00007ff76204f381 in abort () at /lib64/libc.so.6
#2  0x00007ff7620458fa in __assert_fail_base () at /lib64/libc.so.6
#3  0x00007ff762045972 in  () at /lib64/libc.so.6
#4  0x000055eaab249c68 in qemu_co_mutex_unlock (mutex=0x7ff750bf7b40)
at /stor/work/qemu/util/qemu-coroutine-lock.c:320
#5  0x000055eaab249da3 in qemu_lockable_unlock (x=0x7ff750bf7b40) at
/stor/work/qemu/include/qemu/lockable.h:72
#6  0x000055eaab249da3 in qemu_co_queue_wait_impl
(queue=0x55eaaef41a08, lock=lock@entry=0x7ff750bf7b40) at
/stor/work/qemu/util/qemu-coroutine-lock.c:49
#7  0x000055eaab19f2b9 in handle_dependencies
(bs=bs@entry=0x55eaad9c6620, guest_offset=guest_offset@entry=1597440,
cur_bytes=cur_bytes@entry=0x7ff750bf7ba0, m=m@entry=0x7ff7
50bf7c58) at /stor/work/qemu/block/qcow2-cluster.c:1067
#8  0x000055eaab1a1b85 in qcow2_alloc_cluster_offset
(bs=bs@entry=0x55eaad9c6620, offset=offset@entry=1597440,
bytes=bytes@entry=0x7ff750bf7c4c, host_offset=host_offset@entry
=0x7ff750bf7c50, m=m@entry=0x7ff750bf7c58) at
/stor/work/qemu/block/qcow2-cluster.c:1497
#9  0x000055eaab19411e in qcow2_co_pwritev (bs=0x55eaad9c6620,
offset=1597440, bytes=8192, qiov=0x55eaaedb4880, flags=<optimized
out>) at /stor/work/qemu/block/qcow2.c:1896
#10 0x000055eaab1c2962 in bdrv_driver_pwritev
(bs=bs@entry=0x55eaad9c6620, offset=offset@entry=1597440,
bytes=bytes@entry=8192, qiov=qiov@entry=0x55eaaedb4880, flags=flags@en
try=0) at /stor/work/qemu/block/io.c:976
#11 0x000055eaab1c3985 in bdrv_aligned_pwritev
(child=child@entry=0x55eaad92bd00, req=req@entry=0x7ff750bf7e70,
offset=offset@entry=1597440, bytes=bytes@entry=8192, align=ali
gn@entry=1, qiov=qiov@entry=0x55eaaedb4880, flags=0) at
/stor/work/qemu/block/io.c:1534
#12 0x000055eaab1c4ca5 in bdrv_co_pwritev (child=0x55eaad92bd00,
offset=offset@entry=1597440, bytes=bytes@entry=8192,
qiov=qiov@entry=0x55eaaedb4880, flags=flags@entry=0)
   at /stor/work/qemu/block/io.c:1785
#13 0x000055eaab1b4f06 in blk_co_pwritev (blk=0x55eaad9c63c0,
offset=1597440, bytes=8192, qiov=0x55eaaedb4880, flags=0) at
/stor/work/qemu/block/block-backend.c:1135
#14 0x000055eaab1b4fff in blk_aio_write_entry (opaque=0x55eaaefc5eb0)
at /stor/work/qemu/block/block-backend.c:1326
#15 0x000055eaab24a77a in coroutine_trampoline (i0=<optimized out>,
i1=<optimized out>) at /stor/work/qemu/util/coroutine-ucontext.c:79
#16 0x00007ff762066bc0 in __start_context () at /lib64/libc.so.6
#17 0x00007ffdf69102d0 in  ()
#18 0x0000000000000000 in  ()


It's late today so I'll take a closer look tomorrow.

Fam
Re: [Qemu-devel] [PATCH v2 0/4] coroutine-lock: polymorphic CoQueue
Posted by Paolo Bonzini 7 years, 9 months ago 

----- Original Message -----
> From: "Fam Zheng" <famz@redhat.com>
> To: "Paolo Bonzini" <pbonzini@redhat.com>
> Cc: "QEMU Developers" <qemu-devel@nongnu.org>
> Sent: Thursday, January 25, 2018 4:05:27 PM
> Subject: Re: [Qemu-devel] [PATCH v2 0/4] coroutine-lock: polymorphic CoQueue
> 
> On Tue, Jan 16, 2018 at 10:23 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > There are cases in which a queued coroutine must be restarted from
> > non-coroutine context (with qemu_co_enter_next).  In this cases,
> > qemu_co_enter_next also needs to be thread-safe, but it cannot use a
> > CoMutex and so cannot qemu_co_queue_wait.  This happens in curl (which
> > right now is rolling its own list of Coroutines) and will happen in
> > Fam's NVMe driver as well.
> >
> > This series extracts the idea of a polymorphic lockable object
> > from my "scoped lock guard" proposal, and applies it to CoQueue.
> > The implementation of QemuLockable is similar to C11 _Generic, but
> > redone using the preprocessor and GCC builtins for compatibility.
> >
> > In general, while a bit on the esoteric side, the functionality used
> > to emulate _Generic is fairly old in GCC, and the builtins are already
> > used by include/qemu/atomic.h; the series was tested with Fedora 27 (boot
> > Damn Small Linux via http) and CentOS 6 (compiled only).
> 
> I'm seeing this crash with the series:
> 
> (gdb) bt
> #0  0x00007ff76204d66b in raise () at /lib64/libc.so.6
> #1  0x00007ff76204f381 in abort () at /lib64/libc.so.6
> #2  0x00007ff7620458fa in __assert_fail_base () at /lib64/libc.so.6
> #3  0x00007ff762045972 in  () at /lib64/libc.so.6
> #4  0x000055eaab249c68 in qemu_co_mutex_unlock (mutex=0x7ff750bf7b40)
> at /stor/work/qemu/util/qemu-coroutine-lock.c:320
> #5  0x000055eaab249da3 in qemu_lockable_unlock (x=0x7ff750bf7b40) at
> /stor/work/qemu/include/qemu/lockable.h:72
> #6  0x000055eaab249da3 in qemu_co_queue_wait_impl
> (queue=0x55eaaef41a08, lock=lock@entry=0x7ff750bf7b40) at
> /stor/work/qemu/util/qemu-coroutine-lock.c:49
> #7  0x000055eaab19f2b9 in handle_dependencies
> (bs=bs@entry=0x55eaad9c6620, guest_offset=guest_offset@entry=1597440,
> cur_bytes=cur_bytes@entry=0x7ff750bf7ba0, m=m@entry=0x7ff7
> 50bf7c58) at /stor/work/qemu/block/qcow2-cluster.c:1067
> #8  0x000055eaab1a1b85 in qcow2_alloc_cluster_offset
> (bs=bs@entry=0x55eaad9c6620, offset=offset@entry=1597440,
> bytes=bytes@entry=0x7ff750bf7c4c, host_offset=host_offset@entry
> =0x7ff750bf7c50, m=m@entry=0x7ff750bf7c58) at
> /stor/work/qemu/block/qcow2-cluster.c:1497
> #9  0x000055eaab19411e in qcow2_co_pwritev (bs=0x55eaad9c6620,
> offset=1597440, bytes=8192, qiov=0x55eaaedb4880, flags=<optimized
> out>) at /stor/work/qemu/block/qcow2.c:1896
> #10 0x000055eaab1c2962 in bdrv_driver_pwritev
> (bs=bs@entry=0x55eaad9c6620, offset=offset@entry=1597440,
> bytes=bytes@entry=8192, qiov=qiov@entry=0x55eaaedb4880, flags=flags@en
> try=0) at /stor/work/qemu/block/io.c:976
> #11 0x000055eaab1c3985 in bdrv_aligned_pwritev
> (child=child@entry=0x55eaad92bd00, req=req@entry=0x7ff750bf7e70,
> offset=offset@entry=1597440, bytes=bytes@entry=8192, align=ali
> gn@entry=1, qiov=qiov@entry=0x55eaaedb4880, flags=0) at
> /stor/work/qemu/block/io.c:1534
> #12 0x000055eaab1c4ca5 in bdrv_co_pwritev (child=0x55eaad92bd00,
> offset=offset@entry=1597440, bytes=bytes@entry=8192,
> qiov=qiov@entry=0x55eaaedb4880, flags=flags@entry=0)
>    at /stor/work/qemu/block/io.c:1785
> #13 0x000055eaab1b4f06 in blk_co_pwritev (blk=0x55eaad9c63c0,
> offset=1597440, bytes=8192, qiov=0x55eaaedb4880, flags=0) at
> /stor/work/qemu/block/block-backend.c:1135
> #14 0x000055eaab1b4fff in blk_aio_write_entry (opaque=0x55eaaefc5eb0)
> at /stor/work/qemu/block/block-backend.c:1326
> #15 0x000055eaab24a77a in coroutine_trampoline (i0=<optimized out>,
> i1=<optimized out>) at /stor/work/qemu/util/coroutine-ucontext.c:79
> #16 0x00007ff762066bc0 in __start_context () at /lib64/libc.so.6
> #17 0x00007ffdf69102d0 in  ()
> #18 0x0000000000000000 in  ()
> 
> It's late today so I'll take a closer look tomorrow.

Ouch.  /me bangs the head against the wall and runs writing testcases.
Sorry.

Paolo
Re: [Qemu-devel] [PATCH v2 0/4] coroutine-lock: polymorphic CoQueue
Posted by no-reply@patchew.org 7 years, 9 months ago 
Hi,

This series failed docker-build@min-glib build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

Type: series
Message-id: 20180116142316.30486-1-pbonzini@redhat.com
Subject: [Qemu-devel] [PATCH v2 0/4] coroutine-lock: polymorphic CoQueue

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=8
time make docker-test-build@min-glib
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
ceb8579b71 curl: convert to CoQueue
deb8f34b74 coroutine-lock: make qemu_co_enter_next thread-safe
5a30f36fba coroutine-lock: convert CoQueue to use QemuLockable
215b4ef03d lockable: add QemuLockable

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into '/var/tmp/patchew-tester-tmp-m03t9cw_/src/dtc'...
Submodule path 'dtc': checked out 'e54388015af1fb4bf04d0bca99caba1074d9cc42'
  BUILD   min-glib
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 407, in <module>
    sys.exit(main())
  File "./tests/docker/docker.py", line 404, in main
    return args.cmdobj.run(args, argv)
  File "./tests/docker/docker.py", line 287, in run
    dkr = Docker()
  File "./tests/docker/docker.py", line 133, in __init__
    self._command = _guess_docker_command()
  File "./tests/docker/docker.py", line 58, in _guess_docker_command
    commands_txt)
Exception: Cannot find working docker command. Tried:
  docker
  sudo -n docker
make: *** [tests/docker/Makefile.include:37: docker-image-min-glib] Error 1

real	0m0.649s
user	0m0.049s
sys	0m0.029s
=== OUTPUT END ===

Test command exited with code: 2


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Re: [Qemu-devel] [PATCH v2 0/4] coroutine-lock: polymorphic CoQueue
Posted by Fam Zheng 7 years, 9 months ago 

On 01/17/2018 02:04 PM, no-reply@patchew.org wrote:
>    BUILD   min-glib
> Traceback (most recent call last):
>    File "./tests/docker/docker.py", line 407, in <module>
>      sys.exit(main())
>    File "./tests/docker/docker.py", line 404, in main
>      return args.cmdobj.run(args, argv)
>    File "./tests/docker/docker.py", line 287, in run
>      dkr = Docker()
>    File "./tests/docker/docker.py", line 133, in __init__
>      self._command = _guess_docker_command()
>    File "./tests/docker/docker.py", line 58, in _guess_docker_command
>      commands_txt)
> Exception: Cannot find working docker command. Tried:
>    docker
>    sudo -n docker

Patchew.org was misconfigured to run this docker tests on a host that 
has no docker. Fixed now. Sorry for the noise.

Fam
Re: [Qemu-devel] [PATCH v2 0/4] coroutine-lock: polymorphic CoQueue
Posted by Fam Zheng 7 years, 9 months ago 
On Tue, Jan 16, 2018 at 10:23 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> There are cases in which a queued coroutine must be restarted from
> non-coroutine context (with qemu_co_enter_next).  In this cases,
> qemu_co_enter_next also needs to be thread-safe, but it cannot use a
> CoMutex and so cannot qemu_co_queue_wait.  This happens in curl (which
> right now is rolling its own list of Coroutines) and will happen in
> Fam's NVMe driver as well.
>
> This series extracts the idea of a polymorphic lockable object
> from my "scoped lock guard" proposal, and applies it to CoQueue.
> The implementation of QemuLockable is similar to C11 _Generic, but
> redone using the preprocessor and GCC builtins for compatibility.
>
> In general, while a bit on the esoteric side, the functionality used
> to emulate _Generic is fairly old in GCC, and the builtins are already
> used by include/qemu/atomic.h; the series was tested with Fedora 27 (boot
> Damn Small Linux via http) and CentOS 6 (compiled only).
>
> Paolo
>
> v1->v2: fix typos and copyright year

Reviewed-by: Fam Zheng <famz@redhat.com>

Should I include this series in my pull request for the NVMe driver?
Re: [Qemu-devel] [PATCH v2 0/4] coroutine-lock: polymorphic CoQueue
Posted by Paolo Bonzini 7 years, 9 months ago 
On 24/01/2018 04:58, Fam Zheng wrote:
> On Tue, Jan 16, 2018 at 10:23 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> There are cases in which a queued coroutine must be restarted from
>> non-coroutine context (with qemu_co_enter_next).  In this cases,
>> qemu_co_enter_next also needs to be thread-safe, but it cannot use a
>> CoMutex and so cannot qemu_co_queue_wait.  This happens in curl (which
>> right now is rolling its own list of Coroutines) and will happen in
>> Fam's NVMe driver as well.
>>
>> This series extracts the idea of a polymorphic lockable object
>> from my "scoped lock guard" proposal, and applies it to CoQueue.
>> The implementation of QemuLockable is similar to C11 _Generic, but
>> redone using the preprocessor and GCC builtins for compatibility.
>>
>> In general, while a bit on the esoteric side, the functionality used
>> to emulate _Generic is fairly old in GCC, and the builtins are already
>> used by include/qemu/atomic.h; the series was tested with Fedora 27 (boot
>> Damn Small Linux via http) and CentOS 6 (compiled only).
>>
>> Paolo
>>
>> v1->v2: fix typos and copyright year
> 
> Reviewed-by: Fam Zheng <famz@redhat.com>
> 
> Should I include this series in my pull request for the NVMe driver?

Yes, please.  This is within block layer area, since CoQueue and
block/curl.c are still the only users.

Paolo

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.