[Qemu-devel] [PATCH v3 0/5] coroutine-lock: polymorphic CoQueue

Paolo Bonzini posted 5 patches 6 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180125175949.7780-1-pbonzini@redhat.com
Test checkpatch failed
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     | 88 +++++++++++++++++++++++++++++++++++++++++++++
include/qemu/thread.h       |  5 ++-
include/qemu/typedefs.h     |  4 +++
tests/test-coroutine.c      | 75 ++++++++++++++++++++++++++++++++++++--
util/qemu-coroutine-lock.c  | 22 ++++++++----
9 files changed, 245 insertions(+), 38 deletions(-)
create mode 100644 include/qemu/lockable.h
[Qemu-devel] [PATCH v3 0/5] coroutine-lock: polymorphic CoQueue
Posted by Paolo Bonzini 6 years, 2 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

v2->v3: add tests, fix brown paper bag bug, avoid -Waddress errors

Paolo Bonzini (5):
  test-coroutine: add simple CoMutex test
  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     | 88 +++++++++++++++++++++++++++++++++++++++++++++
 include/qemu/thread.h       |  5 ++-
 include/qemu/typedefs.h     |  4 +++
 tests/test-coroutine.c      | 75 ++++++++++++++++++++++++++++++++++++--
 util/qemu-coroutine-lock.c  | 22 ++++++++----
 9 files changed, 245 insertions(+), 38 deletions(-)
 create mode 100644 include/qemu/lockable.h

-- 
2.14.3


Re: [Qemu-devel] [PATCH v3 0/5] coroutine-lock: polymorphic CoQueue
Posted by no-reply@patchew.org 6 years, 2 months ago
Hi,

This series seems to have some coding style problems. See output below for
more information:

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

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   a3f9362af5..2077fef91d  master     -> master
 * [new tag]               patchew/20180125175949.7780-1-pbonzini@redhat.com -> patchew/20180125175949.7780-1-pbonzini@redhat.com
Switched to a new branch 'test'
31cc3c8bd1 curl: convert to CoQueue
b9796ab014 coroutine-lock: make qemu_co_enter_next thread-safe
0a41dcd114 coroutine-lock: convert CoQueue to use QemuLockable
70398398ad lockable: add QemuLockable
695a0dcaa5 test-coroutine: add simple CoMutex test

=== OUTPUT BEGIN ===
Checking PATCH 1/5: test-coroutine: add simple CoMutex test...
ERROR: do not initialise statics to 0 or NULL
#29: FILE: tests/test-coroutine.c:198:
+static bool locked = false;

total: 1 errors, 0 warnings, 74 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/5: lockable: add QemuLockable...
WARNING: line over 80 characters
#58: FILE: include/qemu/compiler.h:144:
+#define QEMU_GENERIC2(x, a0, ...) QEMU_GENERIC_IF(x, a0, QEMU_GENERIC1(x, __VA_ARGS__))

WARNING: line over 80 characters
#59: FILE: include/qemu/compiler.h:145:
+#define QEMU_GENERIC3(x, a0, ...) QEMU_GENERIC_IF(x, a0, QEMU_GENERIC2(x, __VA_ARGS__))

WARNING: line over 80 characters
#60: FILE: include/qemu/compiler.h:146:
+#define QEMU_GENERIC4(x, a0, ...) QEMU_GENERIC_IF(x, a0, QEMU_GENERIC3(x, __VA_ARGS__))

WARNING: line over 80 characters
#61: FILE: include/qemu/compiler.h:147:
+#define QEMU_GENERIC5(x, a0, ...) QEMU_GENERIC_IF(x, a0, QEMU_GENERIC4(x, __VA_ARGS__))

WARNING: line over 80 characters
#62: FILE: include/qemu/compiler.h:148:
+#define QEMU_GENERIC6(x, a0, ...) QEMU_GENERIC_IF(x, a0, QEMU_GENERIC5(x, __VA_ARGS__))

WARNING: line over 80 characters
#63: FILE: include/qemu/compiler.h:149:
+#define QEMU_GENERIC7(x, a0, ...) QEMU_GENERIC_IF(x, a0, QEMU_GENERIC6(x, __VA_ARGS__))

WARNING: line over 80 characters
#64: FILE: include/qemu/compiler.h:150:
+#define QEMU_GENERIC8(x, a0, ...) QEMU_GENERIC_IF(x, a0, QEMU_GENERIC7(x, __VA_ARGS__))

WARNING: line over 80 characters
#65: FILE: include/qemu/compiler.h:151:
+#define QEMU_GENERIC9(x, a0, ...) QEMU_GENERIC_IF(x, a0, QEMU_GENERIC8(x, __VA_ARGS__))

WARNING: line over 80 characters
#66: FILE: include/qemu/compiler.h:152:
+#define QEMU_GENERIC10(x, a0, ...) QEMU_GENERIC_IF(x, a0, QEMU_GENERIC9(x, __VA_ARGS__))

total: 0 errors, 9 warnings, 234 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 3/5: coroutine-lock: convert CoQueue to use QemuLockable...
Checking PATCH 4/5: coroutine-lock: make qemu_co_enter_next thread-safe...
Checking PATCH 5/5: curl: convert to CoQueue...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
Re: [Qemu-devel] [PATCH v3 0/5] coroutine-lock: polymorphic CoQueue
Posted by Eric Blake 6 years, 2 months ago
On 01/25/2018 11:59 AM, Paolo Bonzini 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
> 
> v2->v3: add tests, fix brown paper bag bug, avoid -Waddress errors

I spotted a few typos, but did not rigorously test it enough to leave
R-b.  At any rate, writing polymorphic code in C is an interesting
exercise, so it was fun learning from this thread.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org