[PATCH 0/4] coroutine: Clean up includes

[PATCH 0/4] coroutine: Clean up includes

[Markus Armbruster]

Markus Armbruster (4):
  coroutine: Clean up superfluous inclusion of qemu/coroutine.h
  coroutine: Move coroutine_fn to qemu/osdep.h, trim includes
  coroutine: Clean up superfluous inclusion of qemu/lockable.h
  coroutine: Break inclusion loop

 crypto/block-luks-priv.h      |  1 -
 include/block/aio_task.h      |  2 --
 include/block/block-common.h  |  1 -
 include/block/raw-aio.h       |  1 -
 include/monitor/hmp.h         |  1 -
 include/qemu/coroutine.h      | 21 ++++++++-------------
 include/qemu/lockable.h       |  1 -
 include/qemu/osdep.h          | 16 ++++++++++++++++
 include/qemu/progress_meter.h |  2 +-
 include/qemu/ratelimit.h      |  2 +-
 include/qemu/seqlock.h        |  2 +-
 include/scsi/pr-manager.h     |  1 -
 linux-user/fd-trans.h         |  2 +-
 nbd/nbd-internal.h            |  1 -
 backends/tpm/tpm_emulator.c   |  2 +-
 block/progress_meter.c        |  2 ++
 blockjob.c                    |  1 -
 cpus-common.c                 |  2 +-
 crypto/block-luks.c           |  1 -
 hw/9pfs/codir.c               |  1 -
 hw/9pfs/cofile.c              |  1 -
 hw/9pfs/cofs.c                |  1 -
 hw/9pfs/coxattr.c             |  1 -
 hw/hyperv/hyperv.c            |  2 +-
 hw/usb/ccid-card-emulated.c   |  2 +-
 hw/vfio/platform.c            |  2 +-
 plugins/core.c                |  2 +-
 plugins/loader.c              |  2 +-
 tests/unit/test-coroutine.c   |  2 --
 tests/unit/test-vmstate.c     |  1 -
 ui/spice-display.c            |  2 +-
 util/log.c                    |  2 +-
 util/qemu-coroutine-lock.c    |  1 -
 util/qemu-coroutine-sleep.c   |  1 -
 util/qemu-coroutine.c         |  1 -
 util/qemu-timer.c             |  2 +-
 util/rcu.c                    |  2 +-
 util/vfio-helpers.c           |  2 +-
 util/yank.c                   |  2 +-
 39 files changed, 43 insertions(+), 51 deletions(-)

-- 
2.37.3

[PATCH 1/4] coroutine: Clean up superfluous inclusion of qemu/coroutine.h

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 crypto/block-luks-priv.h    | 1 -
 include/block/raw-aio.h     | 1 -
 include/scsi/pr-manager.h   | 1 -
 nbd/nbd-internal.h          | 1 -
 blockjob.c                  | 1 -
 crypto/block-luks.c         | 1 -
 hw/9pfs/codir.c             | 1 -
 hw/9pfs/cofile.c            | 1 -
 hw/9pfs/cofs.c              | 1 -
 hw/9pfs/coxattr.c           | 1 -
 tests/unit/test-coroutine.c | 1 -
 tests/unit/test-vmstate.c   | 1 -
 util/qemu-coroutine-lock.c  | 1 -
 util/qemu-coroutine-sleep.c | 1 -
 util/qemu-coroutine.c       | 1 -
 15 files changed, 15 deletions(-)

diff --git a/crypto/block-luks-priv.h b/crypto/block-luks-priv.h
index 90a20d432b..dc2dd14e52 100644
--- a/crypto/block-luks-priv.h
+++ b/crypto/block-luks-priv.h
@@ -31,7 +31,6 @@
 #include "crypto/random.h"
 #include "qemu/uuid.h"
 
-#include "qemu/coroutine.h"
 #include "qemu/bitmap.h"
 
 /*
diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h
index 21fc10c4c9..f8cda9df91 100644
--- a/include/block/raw-aio.h
+++ b/include/block/raw-aio.h
@@ -17,7 +17,6 @@
 #define QEMU_RAW_AIO_H
 
 #include "block/aio.h"
-#include "qemu/coroutine.h"
 #include "qemu/iov.h"
 
 /* AIO request types */
diff --git a/include/scsi/pr-manager.h b/include/scsi/pr-manager.h
index e4ecbe00f6..45de28d354 100644
--- a/include/scsi/pr-manager.h
+++ b/include/scsi/pr-manager.h
@@ -5,7 +5,6 @@
 #include "qapi/visitor.h"
 #include "qom/object_interfaces.h"
 #include "block/aio.h"
-#include "qemu/coroutine.h"
 
 #define TYPE_PR_MANAGER "pr-manager"
 
diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
index 1b2141ab4b..df42fef706 100644
--- a/nbd/nbd-internal.h
+++ b/nbd/nbd-internal.h
@@ -13,7 +13,6 @@
 #include "sysemu/block-backend.h"
 #include "io/channel-tls.h"
 
-#include "qemu/coroutine.h"
 #include "qemu/iov.h"
 
 #ifndef _WIN32
diff --git a/blockjob.c b/blockjob.c
index f51d4e18f3..c3d3e14a92 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -32,7 +32,6 @@
 #include "qapi/error.h"
 #include "qapi/qapi-events-block-core.h"
 #include "qapi/qmp/qerror.h"
-#include "qemu/coroutine.h"
 #include "qemu/main-loop.h"
 #include "qemu/timer.h"
 
diff --git a/crypto/block-luks.c b/crypto/block-luks.c
index df2b4105d6..2ee679a2fa 100644
--- a/crypto/block-luks.c
+++ b/crypto/block-luks.c
@@ -32,7 +32,6 @@
 #include "crypto/random.h"
 #include "qemu/uuid.h"
 
-#include "qemu/coroutine.h"
 #include "qemu/bitmap.h"
 
 /*
diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c
index 93ba44fb75..7ba63be489 100644
--- a/hw/9pfs/codir.c
+++ b/hw/9pfs/codir.c
@@ -19,7 +19,6 @@
 #include "qemu/osdep.h"
 #include "fsdev/qemu-fsdev.h"
 #include "qemu/thread.h"
-#include "qemu/coroutine.h"
 #include "qemu/main-loop.h"
 #include "coth.h"
 #include "9p-xattr.h"
diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
index 20f93a90e7..9c5344039e 100644
--- a/hw/9pfs/cofile.c
+++ b/hw/9pfs/cofile.c
@@ -19,7 +19,6 @@
 #include "qemu/osdep.h"
 #include "fsdev/qemu-fsdev.h"
 #include "qemu/thread.h"
-#include "qemu/coroutine.h"
 #include "qemu/main-loop.h"
 #include "coth.h"
 
diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c
index 9d0adc2e78..67e3ae5c5c 100644
--- a/hw/9pfs/cofs.c
+++ b/hw/9pfs/cofs.c
@@ -19,7 +19,6 @@
 #include "qemu/osdep.h"
 #include "fsdev/qemu-fsdev.h"
 #include "qemu/thread.h"
-#include "qemu/coroutine.h"
 #include "qemu/main-loop.h"
 #include "coth.h"
 
diff --git a/hw/9pfs/coxattr.c b/hw/9pfs/coxattr.c
index dbcd09e0fd..cd0f8488ac 100644
--- a/hw/9pfs/coxattr.c
+++ b/hw/9pfs/coxattr.c
@@ -19,7 +19,6 @@
 #include "qemu/osdep.h"
 #include "fsdev/qemu-fsdev.h"
 #include "qemu/thread.h"
-#include "qemu/coroutine.h"
 #include "qemu/main-loop.h"
 #include "coth.h"
 
diff --git a/tests/unit/test-coroutine.c b/tests/unit/test-coroutine.c
index e16b80c245..513800d3db 100644
--- a/tests/unit/test-coroutine.c
+++ b/tests/unit/test-coroutine.c
@@ -12,7 +12,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu/coroutine.h"
 #include "qemu/coroutine_int.h"
 #include "qemu/lockable.h"
 
diff --git a/tests/unit/test-vmstate.c b/tests/unit/test-vmstate.c
index 541bb4f63e..79357b29ca 100644
--- a/tests/unit/test-vmstate.c
+++ b/tests/unit/test-vmstate.c
@@ -29,7 +29,6 @@
 #include "migration/qemu-file-types.h"
 #include "../migration/qemu-file.h"
 #include "../migration/savevm.h"
-#include "qemu/coroutine.h"
 #include "qemu/module.h"
 #include "io/channel-file.h"
 
diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.c
index 45c6b57374..58f3f77181 100644
--- a/util/qemu-coroutine-lock.c
+++ b/util/qemu-coroutine-lock.c
@@ -27,7 +27,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu/coroutine.h"
 #include "qemu/coroutine_int.h"
 #include "qemu/processor.h"
 #include "qemu/queue.h"
diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c
index 571ab521ff..af59f9af98 100644
--- a/util/qemu-coroutine-sleep.c
+++ b/util/qemu-coroutine-sleep.c
@@ -12,7 +12,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu/coroutine.h"
 #include "qemu/coroutine_int.h"
 #include "qemu/timer.h"
 #include "block/aio.h"
diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c
index 356b746f0b..8494523692 100644
--- a/util/qemu-coroutine.c
+++ b/util/qemu-coroutine.c
@@ -16,7 +16,6 @@
 #include "trace.h"
 #include "qemu/thread.h"
 #include "qemu/atomic.h"
-#include "qemu/coroutine.h"
 #include "qemu/coroutine_int.h"
 #include "qemu/coroutine-tls.h"
 #include "block/aio.h"
-- 
2.37.3

Re: [PATCH 1/4] coroutine: Clean up superfluous inclusion of qemu/coroutine.h

[Stefan Hajnoczi]

Probably because block layer, aio.h, and coroutine_int.h header files
already include "qemu/coroutine.h"?

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [PATCH 1/4] coroutine: Clean up superfluous inclusion of qemu/coroutine.h

[Markus Armbruster]

Stefan Hajnoczi <stefanha@gmail.com> writes:

> Probably because block layer, aio.h, and coroutine_int.h header files
> already include "qemu/coroutine.h"?

Mostly, but not always.  For instance, crypto/block-luks-priv.h compiles
fine without it, and doesn't include it after this patch.

> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Thanks!

[PATCH 2/4] coroutine: Move coroutine_fn to qemu/osdep.h, trim includes

[Markus Armbruster]

block/block-hmp-cmds.h and qemu/co-shared-resource.h use coroutine_fn
without including qemu/coroutine.h.  They compile only if it's already
included from elsewhere.

I could fix that, but pulling in qemu/coroutine.h and everything it
includes just for a macro that expands into nothing feels silly.
Instead, move the macro to qemu/osdep.h.

Inclusions of qemu/coroutine.h just for coroutine_fn become
superfluous.  Drop them.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/block/aio_task.h     |  2 --
 include/block/block-common.h |  1 -
 include/monitor/hmp.h        |  1 -
 include/qemu/coroutine.h     | 18 +++++++-----------
 include/qemu/osdep.h         | 16 ++++++++++++++++
 5 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/include/block/aio_task.h b/include/block/aio_task.h
index 50bc1e1817..18a9c41f4e 100644
--- a/include/block/aio_task.h
+++ b/include/block/aio_task.h
@@ -25,8 +25,6 @@
 #ifndef BLOCK_AIO_TASK_H
 #define BLOCK_AIO_TASK_H
 
-#include "qemu/coroutine.h"
-
 typedef struct AioTaskPool AioTaskPool;
 typedef struct AioTask AioTask;
 typedef int coroutine_fn (*AioTaskFunc)(AioTask *task);
diff --git a/include/block/block-common.h b/include/block/block-common.h
index 297704c1e9..0f6b8422bd 100644
--- a/include/block/block-common.h
+++ b/include/block/block-common.h
@@ -27,7 +27,6 @@
 #include "block/aio.h"
 #include "block/aio-wait.h"
 #include "qemu/iov.h"
-#include "qemu/coroutine.h"
 #include "block/accounting.h"
 #include "block/dirty-bitmap.h"
 #include "block/blockjob.h"
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index dfbc0c9a2f..c92f69da8b 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -15,7 +15,6 @@
 #define HMP_H
 
 #include "qemu/readline.h"
-#include "qemu/coroutine.h"
 #include "qapi/qapi-types-common.h"
 
 bool hmp_handle_error(Monitor *mon, Error *err);
diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index 89650a2d7f..2496a4f4ef 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -26,23 +26,19 @@
  * waiting for events to complete.
  *
  * These functions are re-entrant and may be used outside the global mutex.
- */
-
-/**
- * Mark a function that executes in coroutine context
  *
- * Functions that execute in coroutine context cannot be called directly from
- * normal functions.  In the future it would be nice to enable compiler or
- * static checker support for catching such errors.  This annotation might make
- * it possible and in the meantime it serves as documentation.
- *
- * For example:
+ * Functions that execute in coroutine context cannot be called
+ * directly from normal functions.  Use @coroutine_fn to mark such
+ * functions.  For example:
  *
  *   static void coroutine_fn foo(void) {
  *       ....
  *   }
+ *
+ * In the future it would be nice to have the compiler or a static
+ * checker catch misuse of such functions.  This annotation might make
+ * it possible and in the meantime it serves as documentation.
  */
-#define coroutine_fn
 
 typedef struct Coroutine Coroutine;
 
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index b9c4307779..8e97e5d79a 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -157,6 +157,22 @@ extern "C" {
 
 #include "qemu/typedefs.h"
 
+/**
+ * Mark a function that executes in coroutine context
+ *
+ * Functions that execute in coroutine context cannot be called directly from
+ * normal functions.  In the future it would be nice to enable compiler or
+ * static checker support for catching such errors.  This annotation might make
+ * it possible and in the meantime it serves as documentation.
+ *
+ * For example:
+ *
+ *   static void coroutine_fn foo(void) {
+ *       ....
+ *   }
+ */
+#define coroutine_fn
+
 /*
  * For mingw, as of v6.0.0, the function implementing the assert macro is
  * not marked as noreturn, so the compiler cannot delete code following an
-- 
2.37.3

Re: [PATCH 2/4] coroutine: Move coroutine_fn to qemu/osdep.h, trim includes

[Stefan Hajnoczi]

On Thu, 8 Dec 2022 at 09:25, Markus Armbruster <armbru@redhat.com> wrote:
>
> block/block-hmp-cmds.h and qemu/co-shared-resource.h use coroutine_fn
> without including qemu/coroutine.h.  They compile only if it's already
> included from elsewhere.
>
> I could fix that, but pulling in qemu/coroutine.h and everything it
> includes just for a macro that expands into nothing feels silly.
> Instead, move the macro to qemu/osdep.h.
>
> Inclusions of qemu/coroutine.h just for coroutine_fn become
> superfluous.  Drop them.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/block/aio_task.h     |  2 --
>  include/block/block-common.h |  1 -
>  include/monitor/hmp.h        |  1 -
>  include/qemu/coroutine.h     | 18 +++++++-----------
>  include/qemu/osdep.h         | 16 ++++++++++++++++
>  5 files changed, 23 insertions(+), 15 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[PATCH 3/4] coroutine: Clean up superfluous inclusion of qemu/lockable.h

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qemu/progress_meter.h | 2 +-
 block/progress_meter.c        | 2 ++
 tests/unit/test-coroutine.c   | 1 -
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/qemu/progress_meter.h b/include/qemu/progress_meter.h
index dadf822bbf..0f2c0a32d2 100644
--- a/include/qemu/progress_meter.h
+++ b/include/qemu/progress_meter.h
@@ -27,7 +27,7 @@
 #ifndef QEMU_PROGRESS_METER_H
 #define QEMU_PROGRESS_METER_H
 
-#include "qemu/lockable.h"
+#include "qemu/thread.h"
 
 typedef struct ProgressMeter {
     /**
diff --git a/block/progress_meter.c b/block/progress_meter.c
index aa2e60248c..31a170a2cd 100644
--- a/block/progress_meter.c
+++ b/block/progress_meter.c
@@ -23,7 +23,9 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
+
 #include "qemu/osdep.h"
+#include "qemu/coroutine.h"
 #include "qemu/progress_meter.h"
 
 void progress_init(ProgressMeter *pm)
diff --git a/tests/unit/test-coroutine.c b/tests/unit/test-coroutine.c
index 513800d3db..b0d21d673a 100644
--- a/tests/unit/test-coroutine.c
+++ b/tests/unit/test-coroutine.c
@@ -13,7 +13,6 @@
 
 #include "qemu/osdep.h"
 #include "qemu/coroutine_int.h"
-#include "qemu/lockable.h"
 
 /*
  * Check that qemu_in_coroutine() works
-- 
2.37.3

Re: [PATCH 3/4] coroutine: Clean up superfluous inclusion of qemu/lockable.h

[Stefan Hajnoczi]

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[PATCH 4/4] coroutine: Break inclusion loop

[Markus Armbruster]

qemu/coroutine.h and qemu/lockable.h include each other.  Neither
header actually needs the other one.

Drop #include "qemu/coroutine.h" from qemu/lockable.h to break the
loop.  All users of lockable.h actually need qemu/coroutine.h, so
adjust their #include directives.

I'm not dropping the #include "qemu/lockable" from qemu/coroutine.h,
because that would require adding it back next to #include
"qemu/coroutine.h" all over the place.  It's in an unusual place,
though.  Move it to the usual place, next to the other #include
directives.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qemu/coroutine.h    | 3 +--
 include/qemu/lockable.h     | 1 -
 include/qemu/ratelimit.h    | 2 +-
 include/qemu/seqlock.h      | 2 +-
 linux-user/fd-trans.h       | 2 +-
 backends/tpm/tpm_emulator.c | 2 +-
 cpus-common.c               | 2 +-
 hw/hyperv/hyperv.c          | 2 +-
 hw/usb/ccid-card-emulated.c | 2 +-
 hw/vfio/platform.c          | 2 +-
 plugins/core.c              | 2 +-
 plugins/loader.c            | 2 +-
 ui/spice-display.c          | 2 +-
 util/log.c                  | 2 +-
 util/qemu-timer.c           | 2 +-
 util/rcu.c                  | 2 +-
 util/vfio-helpers.c         | 2 +-
 util/yank.c                 | 2 +-
 18 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h
index 2496a4f4ef..2504d4757f 100644
--- a/include/qemu/coroutine.h
+++ b/include/qemu/coroutine.h
@@ -15,6 +15,7 @@
 #ifndef QEMU_COROUTINE_H
 #define QEMU_COROUTINE_H
 
+#include "qemu/lockable.h"
 #include "qemu/queue.h"
 #include "qemu/timer.h"
 
@@ -376,8 +377,6 @@ void qemu_coroutine_inc_pool_size(unsigned int additional_pool_size);
  */
 void qemu_coroutine_dec_pool_size(unsigned int additional_pool_size);
 
-#include "qemu/lockable.h"
-
 /**
  * Sends a (part of) iovec down a socket, yielding when the socket is full, or
  * Receives data into a (part of) iovec from a socket,
diff --git a/include/qemu/lockable.h b/include/qemu/lockable.h
index 86db7cb04c..7d6cdeb750 100644
--- a/include/qemu/lockable.h
+++ b/include/qemu/lockable.h
@@ -13,7 +13,6 @@
 #ifndef QEMU_LOCKABLE_H
 #define QEMU_LOCKABLE_H
 
-#include "qemu/coroutine.h"
 #include "qemu/thread.h"
 
 typedef void QemuLockUnlockFunc(void *);
diff --git a/include/qemu/ratelimit.h b/include/qemu/ratelimit.h
index 48bf59e857..4e07e1e2a4 100644
--- a/include/qemu/ratelimit.h
+++ b/include/qemu/ratelimit.h
@@ -14,7 +14,7 @@
 #ifndef QEMU_RATELIMIT_H
 #define QEMU_RATELIMIT_H
 
-#include "qemu/lockable.h"
+#include "qemu/coroutine.h"
 #include "qemu/timer.h"
 
 typedef struct {
diff --git a/include/qemu/seqlock.h b/include/qemu/seqlock.h
index ecb7d2c864..79a0af625b 100644
--- a/include/qemu/seqlock.h
+++ b/include/qemu/seqlock.h
@@ -16,7 +16,7 @@
 
 #include "qemu/atomic.h"
 #include "qemu/thread.h"
-#include "qemu/lockable.h"
+#include "qemu/coroutine.h"
 
 typedef struct QemuSeqLock QemuSeqLock;
 
diff --git a/linux-user/fd-trans.h b/linux-user/fd-trans.h
index 1b9fa2041c..e662d644bc 100644
--- a/linux-user/fd-trans.h
+++ b/linux-user/fd-trans.h
@@ -16,7 +16,7 @@
 #ifndef FD_TRANS_H
 #define FD_TRANS_H
 
-#include "qemu/lockable.h"
+#include "qemu/coroutine.h"
 
 typedef abi_long (*TargetFdDataFunc)(void *, size_t);
 typedef abi_long (*TargetFdAddrFunc)(void *, abi_ulong, socklen_t);
diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
index 49cc3d749d..f364b089ad 100644
--- a/backends/tpm/tpm_emulator.c
+++ b/backends/tpm/tpm_emulator.c
@@ -30,7 +30,7 @@
 #include "qemu/error-report.h"
 #include "qemu/module.h"
 #include "qemu/sockets.h"
-#include "qemu/lockable.h"
+#include "qemu/coroutine.h"
 #include "io/channel-socket.h"
 #include "sysemu/runstate.h"
 #include "sysemu/tpm_backend.h"
diff --git a/cpus-common.c b/cpus-common.c
index 793364dc0e..1b1f8f75c9 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -22,7 +22,7 @@
 #include "exec/cpu-common.h"
 #include "hw/core/cpu.h"
 #include "sysemu/cpus.h"
-#include "qemu/lockable.h"
+#include "qemu/coroutine.h"
 
 static QemuMutex qemu_cpu_list_lock;
 static QemuCond exclusive_cond;
diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
index 57b402b956..41f3e9f1ca 100644
--- a/hw/hyperv/hyperv.c
+++ b/hw/hyperv/hyperv.c
@@ -15,7 +15,7 @@
 #include "sysemu/kvm.h"
 #include "qemu/bitops.h"
 #include "qemu/error-report.h"
-#include "qemu/lockable.h"
+#include "qemu/coroutine.h"
 #include "qemu/queue.h"
 #include "qemu/rcu.h"
 #include "qemu/rcu_queue.h"
diff --git a/hw/usb/ccid-card-emulated.c b/hw/usb/ccid-card-emulated.c
index ee41a81801..6962143c29 100644
--- a/hw/usb/ccid-card-emulated.c
+++ b/hw/usb/ccid-card-emulated.c
@@ -30,7 +30,7 @@
 #include <libcacard.h>
 
 #include "qemu/thread.h"
-#include "qemu/lockable.h"
+#include "qemu/coroutine.h"
 #include "qemu/main-loop.h"
 #include "qemu/module.h"
 #include "ccid.h"
diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 5af73f9287..e252edc04a 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -22,7 +22,7 @@
 #include "hw/vfio/vfio-platform.h"
 #include "migration/vmstate.h"
 #include "qemu/error-report.h"
-#include "qemu/lockable.h"
+#include "qemu/coroutine.h"
 #include "qemu/main-loop.h"
 #include "qemu/module.h"
 #include "qemu/range.h"
diff --git a/plugins/core.c b/plugins/core.c
index ccb770a485..313a631ecf 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -15,7 +15,7 @@
 #include "qemu/error-report.h"
 #include "qemu/config-file.h"
 #include "qapi/error.h"
-#include "qemu/lockable.h"
+#include "qemu/coroutine.h"
 #include "qemu/option.h"
 #include "qemu/rcu_queue.h"
 #include "qemu/xxhash.h"
diff --git a/plugins/loader.c b/plugins/loader.c
index 88c30bde2d..0e0a19730e 100644
--- a/plugins/loader.c
+++ b/plugins/loader.c
@@ -19,7 +19,7 @@
 #include "qemu/error-report.h"
 #include "qemu/config-file.h"
 #include "qapi/error.h"
-#include "qemu/lockable.h"
+#include "qemu/coroutine.h"
 #include "qemu/option.h"
 #include "qemu/rcu_queue.h"
 #include "qemu/qht.h"
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 494168e7fe..8e2245309e 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -18,7 +18,7 @@
 #include "qemu/osdep.h"
 #include "ui/qemu-spice.h"
 #include "qemu/timer.h"
-#include "qemu/lockable.h"
+#include "qemu/coroutine.h"
 #include "qemu/main-loop.h"
 #include "qemu/option.h"
 #include "qemu/queue.h"
diff --git a/util/log.c b/util/log.c
index c2198badf2..157c25f0b5 100644
--- a/util/log.c
+++ b/util/log.c
@@ -25,7 +25,7 @@
 #include "qemu/cutils.h"
 #include "trace/control.h"
 #include "qemu/thread.h"
-#include "qemu/lockable.h"
+#include "qemu/coroutine.h"
 #include "qemu/rcu.h"
 #ifdef CONFIG_LINUX
 #include <sys/syscall.h>
diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index 6a0de33dd2..6d5d21cb92 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -25,7 +25,7 @@
 #include "qemu/osdep.h"
 #include "qemu/main-loop.h"
 #include "qemu/timer.h"
-#include "qemu/lockable.h"
+#include "qemu/coroutine.h"
 #include "sysemu/cpu-timers.h"
 #include "sysemu/replay.h"
 #include "sysemu/cpus.h"
diff --git a/util/rcu.c b/util/rcu.c
index b6d6c71cff..013700bb5c 100644
--- a/util/rcu.c
+++ b/util/rcu.c
@@ -31,7 +31,7 @@
 #include "qemu/atomic.h"
 #include "qemu/thread.h"
 #include "qemu/main-loop.h"
-#include "qemu/lockable.h"
+#include "qemu/coroutine.h"
 #if defined(CONFIG_MALLOC_TRIM)
 #include <malloc.h>
 #endif
diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index 0d1520caac..8584db8416 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -22,7 +22,7 @@
 #include "standard-headers/linux/pci_regs.h"
 #include "qemu/event_notifier.h"
 #include "qemu/vfio-helpers.h"
-#include "qemu/lockable.h"
+#include "qemu/coroutine.h"
 #include "trace.h"
 
 #define QEMU_VFIO_DEBUG 0
diff --git a/util/yank.c b/util/yank.c
index abf47c346d..1fd3e550c1 100644
--- a/util/yank.c
+++ b/util/yank.c
@@ -11,7 +11,7 @@
 #include "qapi/error.h"
 #include "qemu/thread.h"
 #include "qemu/queue.h"
-#include "qemu/lockable.h"
+#include "qemu/coroutine.h"
 #include "qapi/qapi-commands-yank.h"
 #include "qapi/qapi-visit-yank.h"
 #include "qapi/clone-visitor.h"
-- 
2.37.3

Re: [PATCH 4/4] coroutine: Break inclusion loop

[Stefan Hajnoczi]

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [PATCH 4/4] coroutine: Break inclusion loop

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 2713 bytes --]

dropped qemu-devel by mistake.

Paolo


Il lun 12 dic 2022, 23:16 Paolo Bonzini <pbonzini@redhat.com> ha scritto:

> On 12/8/22 15:23, Markus Armbruster wrote:
> > qemu/coroutine.h and qemu/lockable.h include each other.  Neither
> > header actually needs the other one.
>
> qemu/lockable.h wants qemu/coroutine.h because of the reference to
> qemu_co_mutex_lock/unlock in the QEMU_MAKE_LOCKABLE macro.  Said
> reference only happens when the macro is used, so strictly speaking
> only code that uses of qemu/lockable.h's functionality needs to
> include qemu/coroutine.h.  The order doesn't matter.
>
> qemu/coroutine.h similarly wants qemu/lockable.h only for a macro: it
> uses QemuLockable for the prototype of qemu_co_queue_wait_impl, but
> QemuLockable is defined in qemu/typedefs.h.  On the other hand, the
> qemu_co_queue_wait macro needs QEMU_MAKE_LOCKABLE.  Again, the order
> does not matter but callers of qemu_co_queue_wait appreciate it if
> both files are included.
>
> So, this is why the inclusion loop works.  This patch makes some
> files include qemu/coroutine.h even if they only need qemu/lockable.h
> for QEMU_LOCK_GUARD of a "regular" QemuMutex; for example, linux-user/
> does not use coroutines, so I'd like to avoid that it includes
> qemu/coroutine.h.
>
> One way is to just keep the cycle.  Another is to break the cycle is
> as follows:
>
> 1) qemu/coroutine.h keeps including qemu/lockable.h
>
> 2) qemu/lockable.h is modified as follows to omit the reference to CoMutex:
>
> diff --git a/include/qemu/lockable.h b/include/qemu/lockable.h
> index 86db7cb04c9c..db59656538a4 100644
> --- a/include/qemu/lockable.h
> +++ b/include/qemu/lockable.h
> @@ -71,9 +71,11 @@ qemu_null_lockable(void *x)
>                void *: qemu_null_lockable(x),                             \
>                QemuMutex *: qemu_make_lockable(x, QML_OBJ_(x, mutex)),    \
>                QemuRecMutex *: qemu_make_lockable(x, QML_OBJ_(x,
> rec_mutex)), \
> -             CoMutex *: qemu_make_lockable(x, QML_OBJ_(x, co_mutex)),   \
> +             QEMU_MAKE_CO_MUTEX_LOCKABLE(x)                             \
>                QemuSpin *: qemu_make_lockable(x, QML_OBJ_(x, spin)))
>
> +#define QEMU_MAKE_CO_MUTEX_LOCKABLE(x)
> +
>   /**
>    * QEMU_MAKE_LOCKABLE_NONNULL - Make a polymorphic QemuLockable
>    *
>
> 3) the following hack is added in qemu/coroutine.h, right after including
> qemu/lockable.h:
>
> #undef QEMU_MAKE_CO_MUTEX_LOCKABLE(x)
> #define QEMU_MAKE_CO_MUTEX_LOCKABLE(x) \
>               CoMutex *: qemu_make_lockable(x, QML_OBJ_(x, co_mutex)),
>
>
> Neither is particularly pretty, so I vote for leaving things as is with
> a comment above the two #include directives.
>
> Paolo
>

[-- Attachment #2: Type: text/html, Size: 3347 bytes --]

Re: [PATCH 4/4] coroutine: Break inclusion loop

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> dropped qemu-devel by mistake.
>
> Paolo
>
>
> Il lun 12 dic 2022, 23:16 Paolo Bonzini <pbonzini@redhat.com> ha scritto:
>
>> On 12/8/22 15:23, Markus Armbruster wrote:
>> > qemu/coroutine.h and qemu/lockable.h include each other.  Neither
>> > header actually needs the other one.
>>
>> qemu/lockable.h wants qemu/coroutine.h because of the reference to
>> qemu_co_mutex_lock/unlock in the QEMU_MAKE_LOCKABLE macro.  Said
>> reference only happens when the macro is used, so strictly speaking
>> only code that uses of qemu/lockable.h's functionality needs to
>> include qemu/coroutine.h.  The order doesn't matter.
>>
>> qemu/coroutine.h similarly wants qemu/lockable.h only for a macro: it
>> uses QemuLockable for the prototype of qemu_co_queue_wait_impl, but
>> QemuLockable is defined in qemu/typedefs.h.  On the other hand, the
>> qemu_co_queue_wait macro needs QEMU_MAKE_LOCKABLE.  Again, the order
>> does not matter but callers of qemu_co_queue_wait appreciate it if
>> both files are included.
>>
>> So, this is why the inclusion loop works.  This patch makes some
>> files include qemu/coroutine.h even if they only need qemu/lockable.h
>> for QEMU_LOCK_GUARD of a "regular" QemuMutex; for example, linux-user/
>> does not use coroutines, so I'd like to avoid that it includes
>> qemu/coroutine.h.

They include it even before the patch, via lockable.h.

My patch actually enables *not* including coroutine.h: with it applied,
including lockable.h no longer gets you coroutine.h as well.

If you include lockable.h and make use of certain macros, the compile
fails, and you fix it by including coroutine.h instead like pretty much
everything else.  Is this really too objectionable to be committed?

>> One way is to just keep the cycle.  Another is to break the cycle is
>> as follows:
>>
>> 1) qemu/coroutine.h keeps including qemu/lockable.h

As in my patch.

>> 2) qemu/lockable.h is modified as follows to omit the reference to CoMutex:
>>
>> diff --git a/include/qemu/lockable.h b/include/qemu/lockable.h
>> index 86db7cb04c9c..db59656538a4 100644
>> --- a/include/qemu/lockable.h
>> +++ b/include/qemu/lockable.h
>> @@ -71,9 +71,11 @@ qemu_null_lockable(void *x)
>>                void *: qemu_null_lockable(x),                             \
>>                QemuMutex *: qemu_make_lockable(x, QML_OBJ_(x, mutex)),    \
>>                QemuRecMutex *: qemu_make_lockable(x, QML_OBJ_(x,
>> rec_mutex)), \
>> -             CoMutex *: qemu_make_lockable(x, QML_OBJ_(x, co_mutex)),   \
>> +             QEMU_MAKE_CO_MUTEX_LOCKABLE(x)                             \
>>                QemuSpin *: qemu_make_lockable(x, QML_OBJ_(x, spin)))
>>
>> +#define QEMU_MAKE_CO_MUTEX_LOCKABLE(x)
>> +
>>   /**
>>    * QEMU_MAKE_LOCKABLE_NONNULL - Make a polymorphic QemuLockable
>>    *
>>
>> 3) the following hack is added in qemu/coroutine.h, right after including
>> qemu/lockable.h:
>>
>> #undef QEMU_MAKE_CO_MUTEX_LOCKABLE(x)
>> #define QEMU_MAKE_CO_MUTEX_LOCKABLE(x) \
>>               CoMutex *: qemu_make_lockable(x, QML_OBJ_(x, co_mutex)),

Let me see...  if I include just lockable.h and make use of certain
(generic) macro(s), the macro(s) won't have a case for QemuMutex *.
Using them with a QemuMutex * argument won't compile.

>> Neither is particularly pretty, so I vote for leaving things as is with
>> a comment above the two #include directives.

Inlusion loops are landmines.  Evidence: the compilation failure Phil
ran in, leading to his

    Subject: [PATCH-for-8.0] coroutine: Add missing <qemu/atomic.h> include
    Message-Id: <20221125175532.48858-1-philmd@linaro.org>

Your macro hack I find too off-putting :)

>>> Drop #include "qemu/coroutine.h" from qemu/lockable.h to break the
>>> loop.  All users of lockable.h actually need qemu/coroutine.h, so
>>> adjust their #include directives.
>>> 
>>> I'm not dropping the #include "qemu/lockable.h" from qemu/coroutine.h,
>>> because that would require adding it back next to #include
>>> "qemu/coroutine.h" all over the place.  It's in an unusual place,
>>> though.  Move it to the usual place, next to the other #include
>>> directives.
>>> 
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH 4/4] coroutine: Break inclusion loop

[Paolo Bonzini]

On 12/15/22 07:49, Markus Armbruster wrote:
>> linux-user/ does not use coroutines, so I'd like to avoid that it
>> includes qemu/coroutine.h.
> 
> They include it even before the patch, via lockable.h.

They do but there's a difference between "including lockable.h and 
implictly getting coroutine.h due to dependencies" and "including 
coroutine.h when you really wanted QEMU_LOCK_GUARD()".

> My patch actually enables*not*  including coroutine.h: with it applied,
> including lockable.h no longer gets you coroutine.h as well.
> 
> If you include lockable.h and make use of certain macros, the compile
> fails, and you fix it by including coroutine.h instead like pretty much
> everything else.  Is this really too objectionable to be committed?

s/certain macros/all macros/.  All you can do is 
qemu_lockable_lock/unlock, which is the less common usage of 
qemu/lockable.h:

- qemu_lockable_lock/unlock: used in include/qemu/seqlock.h, 
tests/unit/test-coroutine.c, util/qemu-coroutine-lock.c

- QEMU_LOCK_GUARD and WITH_QEMU_LOCK_GUARD are used in 49 files.

>>> 1) qemu/coroutine.h keeps including qemu/lockable.h
>
> As in my patch.

That's where the similarity ends. :)

>>> 2) qemu/lockable.h is modified as follows to omit the reference to CoMutex:
>>>
>>> diff --git a/include/qemu/lockable.h b/include/qemu/lockable.h
>>> index 86db7cb04c9c..db59656538a4 100644
>>> --- a/include/qemu/lockable.h
>>> +++ b/include/qemu/lockable.h
>>> @@ -71,9 +71,11 @@ qemu_null_lockable(void *x)
>>>                 void *: qemu_null_lockable(x),                             \
>>>                 QemuMutex *: qemu_make_lockable(x, QML_OBJ_(x, mutex)),    \
>>>                 QemuRecMutex *: qemu_make_lockable(x, QML_OBJ_(x,
>>> rec_mutex)), \
>>> -             CoMutex *: qemu_make_lockable(x, QML_OBJ_(x, co_mutex)),   \
>>> +             QEMU_MAKE_CO_MUTEX_LOCKABLE(x)                             \
>>>                 QemuSpin *: qemu_make_lockable(x, QML_OBJ_(x, spin)))
>>>
>>> +#define QEMU_MAKE_CO_MUTEX_LOCKABLE(x)
>>> +
>>>    /**
>>>     * QEMU_MAKE_LOCKABLE_NONNULL - Make a polymorphic QemuLockable
>>>     *
>>>
>>> 3) the following hack is added in qemu/coroutine.h, right after including
>>> qemu/lockable.h:
>>>
>>> #undef QEMU_MAKE_CO_MUTEX_LOCKABLE(x)
>>> #define QEMU_MAKE_CO_MUTEX_LOCKABLE(x) \
>>>                CoMutex *: qemu_make_lockable(x, QML_OBJ_(x, co_mutex)),
>
> Let me see...  if I include just lockable.h and make use of certain
> (generic) macro(s), the macro(s) won't have a case for QemuMutex *.
> Using them with a QemuMutex * argument won't compile.

s/QemuMutex/CoMutex/.  That is, you get the CoMutex case only if you 
include qemu/coroutine.h.  Which you probably did anyway, because 
CoMutexes are almost always embedded (not used as pointers).  In fact I 
suspect the above trick also makes it possible to remove CoMutex from 
qemu/typedefs.h.

Furthermore, using qemu_lockable_lock/unlock with CoMutexes still works, 
even if you don't include qemu/coroutine.h, just like in your patch.

>>> Neither is particularly pretty, so I vote for leaving things as is with
>>> a comment above the two #include directives.
> Inlusion loops are landmines.  Evidence: the compilation failure Phil
> ran in, leading to his
> 
>      Subject: [PATCH-for-8.0] coroutine: Add missing <qemu/atomic.h> include
>      Message-Id:<20221125175532.48858-1-philmd@linaro.org>
> 
> Your macro hack I find too off-putting 😄

I think the macro is much better than nerfing qemu/lockable.h.

Another alternative is to add a new header qemu/lockable-protos.h and 
move qemu_co_mutex_{lock,unlock} there (possibly other prototypes as 
well).  Then include it from both qemu/lockable.h and qemu/coroutine.h.

Paolo

Re: [PATCH 4/4] coroutine: Break inclusion loop

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 12/15/22 07:49, Markus Armbruster wrote:
>>> linux-user/ does not use coroutines, so I'd like to avoid that it
>>> includes qemu/coroutine.h.
>> They include it even before the patch, via lockable.h.
>
> They do but there's a difference between "including lockable.h and implictly getting coroutine.h due to dependencies" and "including 
> coroutine.h when you really wanted QEMU_LOCK_GUARD()".
>
>> My patch actually enables*not*  including coroutine.h: with it applied,
>> including lockable.h no longer gets you coroutine.h as well.
>> If you include lockable.h and make use of certain macros, the compile
>> fails, and you fix it by including coroutine.h instead like pretty much
>> everything else.  Is this really too objectionable to be committed?
>
> s/certain macros/all macros/.  All you can do is qemu_lockable_lock/unlock, which is the less common usage of 
> qemu/lockable.h:
>
> - qemu_lockable_lock/unlock: used in include/qemu/seqlock.h, tests/unit/test-coroutine.c, util/qemu-coroutine-lock.c
>
> - QEMU_LOCK_GUARD and WITH_QEMU_LOCK_GUARD are used in 49 files.
>
>>>> 1) qemu/coroutine.h keeps including qemu/lockable.h
>>
>> As in my patch.
>
> That's where the similarity ends. :)
>
>>>> 2) qemu/lockable.h is modified as follows to omit the reference to CoMutex:
>>>>
>>>> diff --git a/include/qemu/lockable.h b/include/qemu/lockable.h
>>>> index 86db7cb04c9c..db59656538a4 100644
>>>> --- a/include/qemu/lockable.h
>>>> +++ b/include/qemu/lockable.h
>>>> @@ -71,9 +71,11 @@ qemu_null_lockable(void *x)
>>>>                 void *: qemu_null_lockable(x),                             \
>>>>                 QemuMutex *: qemu_make_lockable(x, QML_OBJ_(x, mutex)),    \
>>>>                 QemuRecMutex *: qemu_make_lockable(x, QML_OBJ_(x,
>>>> rec_mutex)), \
>>>> -             CoMutex *: qemu_make_lockable(x, QML_OBJ_(x, co_mutex)),   \
>>>> +             QEMU_MAKE_CO_MUTEX_LOCKABLE(x)                             \
>>>>                 QemuSpin *: qemu_make_lockable(x, QML_OBJ_(x, spin)))
>>>>
>>>> +#define QEMU_MAKE_CO_MUTEX_LOCKABLE(x)
>>>> +
>>>>    /**
>>>>     * QEMU_MAKE_LOCKABLE_NONNULL - Make a polymorphic QemuLockable
>>>>     *
>>>>
>>>> 3) the following hack is added in qemu/coroutine.h, right after including
>>>> qemu/lockable.h:
>>>>
>>>> #undef QEMU_MAKE_CO_MUTEX_LOCKABLE(x)
>>>> #define QEMU_MAKE_CO_MUTEX_LOCKABLE(x) \
>>>>                CoMutex *: qemu_make_lockable(x, QML_OBJ_(x, co_mutex)),
>>
>> Let me see...  if I include just lockable.h and make use of certain
>> (generic) macro(s), the macro(s) won't have a case for QemuMutex *.
>> Using them with a QemuMutex * argument won't compile.
>
> s/QemuMutex/CoMutex/.  That is, you get the CoMutex case only if you include qemu/coroutine.h.  Which you probably did anyway, because 
> CoMutexes are almost always embedded (not used as pointers).  In fact I suspect the above trick also makes it possible to remove CoMutex from 
> qemu/typedefs.h.
>
> Furthermore, using qemu_lockable_lock/unlock with CoMutexes still works, even if you don't include qemu/coroutine.h, just like in your patch.
>
>>>> Neither is particularly pretty, so I vote for leaving things as is with
>>>> a comment above the two #include directives.
>>
>> Inlusion loops are landmines.  Evidence: the compilation failure Phil
>> ran in, leading to his
>>      Subject: [PATCH-for-8.0] coroutine: Add missing <qemu/atomic.h> include
>>      Message-Id:<20221125175532.48858-1-philmd@linaro.org>
>> Your macro hack I find too off-putting 😄
>
> I think the macro is much better than nerfing qemu/lockable.h.

The core of the problem is that lockable.h wants to provide _Generic()
with a CoMutex case if CoMutex exists.

The solution you proposed approximates this as follows.  lockable.h
makes the case configurable, default off.  coroutine.h configures it to
on, and includes lockable.h.  Works as long as we include only
coroutine.h, or coroutine.h before lockable.h.  Falls apart if we
include lockable.h, and only then coroutine.h.

For a robust solution, we'd need to enable lockable.h to detect "this
compilation unit may use coroutines".  Could CONFIG_USB_ONLY be pressed
into service?

> Another alternative is to add a new header qemu/lockable-protos.h and move qemu_co_mutex_{lock,unlock} there (possibly other prototypes as 
> well).  Then include it from both qemu/lockable.h and qemu/coroutine.h.

Only from lockable.h, because coroutine.h gets it via lockable.h, right?

Lazy^Wpragmatic solution: move the coroutine.h parts lockable.h needs to
lockable.h.  As far as I can tell:

    typedef CoMutex (unless we keep it in typedefs.h)
    qemu_co_mutex_lock()
    qemu_co_mutex_unlock()

Could throw in

    qemu_co_mutex_init()
    qemu_co_mutex_assert_locked()

to avoid splitting the co_mutex interface.

If keeping this in lockable.h bothers you, we could create comutex.h for
it.

Can't see what adding more to the new header would buy us (other than
arguments on how to name it).  Happy to be enlightened there, of course.

Re: [PATCH 4/4] coroutine: Break inclusion loop

[Philippe Mathieu-Daudé]

On 13/12/22 01:34, Paolo Bonzini wrote:
>     On 12/8/22 15:23, Markus Armbruster wrote:
>      > qemu/coroutine.h and qemu/lockable.h include each other.  Neither
>      > header actually needs the other one.
> 
>     qemu/lockable.h wants qemu/coroutine.h because of the reference to
>     qemu_co_mutex_lock/unlock in the QEMU_MAKE_LOCKABLE macro.  Said
>     reference only happens when the macro is used, so strictly speaking
>     only code that uses of qemu/lockable.h's functionality needs to
>     include qemu/coroutine.h.  The order doesn't matter.

[*]

>     qemu/coroutine.h similarly wants qemu/lockable.h only for a macro: it
>     uses QemuLockable for the prototype of qemu_co_queue_wait_impl, but
>     QemuLockable is defined in qemu/typedefs.h.  On the other hand, the
>     qemu_co_queue_wait macro needs QEMU_MAKE_LOCKABLE.  Again, the order
>     does not matter but callers of qemu_co_queue_wait appreciate it if
>     both files are included.
> 
>     So, this is why the inclusion loop works.  This patch makes some
>     files include qemu/coroutine.h even if they only need qemu/lockable.h
>     for QEMU_LOCK_GUARD of a "regular" QemuMutex; for example, linux-user/
>     does not use coroutines, so I'd like to avoid that it includes
>     qemu/coroutine.h.
> 
>     One way is to just keep the cycle.  Another is to break the cycle is
>     as follows:
> 
>     1) qemu/coroutine.h keeps including qemu/lockable.h
> 
>     2) qemu/lockable.h is modified as follows to omit the reference to
>     CoMutex:
> 
>     diff --git a/include/qemu/lockable.h b/include/qemu/lockable.h
>     index 86db7cb04c9c..db59656538a4 100644
>     --- a/include/qemu/lockable.h
>     +++ b/include/qemu/lockable.h
>     @@ -71,9 +71,11 @@ qemu_null_lockable(void *x)
>                     void *: qemu_null_lockable(x),                     
>             \
>                     QemuMutex *: qemu_make_lockable(x, QML_OBJ_(x,
>     mutex)),    \
>                     QemuRecMutex *: qemu_make_lockable(x, QML_OBJ_(x,
>     rec_mutex)), \
>     -             CoMutex *: qemu_make_lockable(x, QML_OBJ_(x,
>     co_mutex)),   \
>     +             QEMU_MAKE_CO_MUTEX_LOCKABLE(x)                       
>           \

Interesting, I ended doing something similar today because this line is
the single sysemu-specific part of this file (user emulation shouldn't
have access to qemu/coroutine.h). So back to [*], the order seems to
matter for user-mode.

>                     QemuSpin *: qemu_make_lockable(x, QML_OBJ_(x, spin)))
> 
>     +#define QEMU_MAKE_CO_MUTEX_LOCKABLE(x)
>     +
>        /**
>         * QEMU_MAKE_LOCKABLE_NONNULL - Make a polymorphic QemuLockable
>         *
> 
>     3) the following hack is added in qemu/coroutine.h, right after
>     including qemu/lockable.h:
> 
>     #undef QEMU_MAKE_CO_MUTEX_LOCKABLE(x)
>     #define QEMU_MAKE_CO_MUTEX_LOCKABLE(x) \
>                    CoMutex *: qemu_make_lockable(x, QML_OBJ_(x, co_mutex)),
> 
> 
>     Neither is particularly pretty, so I vote for leaving things as is with
>     a comment above the two #include directives.
> 
>     Paolo
>