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
