On 07/06/2021 11:23, Paolo Bonzini wrote:
On 04/06/21 18:16, Eric Blake wrote:
On Fri, Jun 04, 2021 at 12:07:36PM +0200, Emanuele Giuseppe Esposito
wrote:
Extract to a separate function. Do not rely on FOREACH_SAFE, which is
only "safe" if the *current* node is removed---not if another node is
removed. Instead, just walk the entire list from the beginning when
asked to resume all suspended requests with a given tag.
- QLIST_FOREACH_SAFE(r, &s->suspended_reqs, next, next) {
+retry:
+ QLIST_FOREACH(r, &s->suspended_reqs, next) {
if (!strcmp(r->tag, tag)) {
+ QLIST_REMOVE(r, next);
Isn't the whole point of using QLIST_FOREACH_SAFE the ability to call
QLIST_REMOVE on an element in that list while still iterating?
qemu_coroutine_enter(r->co);
+ if (all) {
+ goto retry;
+ }
return 0;
Oh, I see - you abandon the iteration in all control flow paths, so
the simpler loop is still okay. Still, this confused me enough on
first read that it may be worth a comment, maybe:
/* No need for _SAFE, because iteration stops on first hit */
This is a bit confusing too because it sounds like not using _SAFE is an
optimization, but it's actually wrong (see commit message).
What about:
/* No need for _SAFE, since a different coroutine can remove another
node (not the current one) in this list, and when the current one is
removed the iteration starts back from beginning anyways. */
Alternatively, no comment at all.
Thank you,
Emanuele
