[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 1/6] blkdebug: refactor removal of a suspended request
From: |
Eric Blake |
Subject: |
Re: [PATCH v4 1/6] blkdebug: refactor removal of a suspended request |
Date: |
Fri, 4 Jun 2021 11:16:10 -0500 |
User-agent: |
NeoMutt/20210205 |
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.
>
> Co-developed-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/blkdebug.c | 28 +++++++++++++++++-----------
> 1 file changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/block/blkdebug.c b/block/blkdebug.c
> index 2c0b9b0ee8..8f19d991fa 100644
> --- a/block/blkdebug.c
> -static int blkdebug_debug_resume(BlockDriverState *bs, const char *tag)
> +static int resume_req_by_tag(BDRVBlkdebugState *s, const char *tag, bool all)
> {
> - BDRVBlkdebugState *s = bs->opaque;
> - BlkdebugSuspendedReq *r, *next;
> + BlkdebugSuspendedReq *r;
>
> - 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 */
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[PATCH v4 3/6] blkdebug: track all actions, Emanuele Giuseppe Esposito, 2021/06/04
[PATCH v4 4/6] blkdebug: do not suspend in the middle of QLIST_FOREACH_SAFE, Emanuele Giuseppe Esposito, 2021/06/04
[PATCH v4 5/6] block/blkdebug: remove new_state field and instead use a local variable, Emanuele Giuseppe Esposito, 2021/06/04
[PATCH v4 6/6] blkdebug: protect rules and suspended_reqs with a lock, Emanuele Giuseppe Esposito, 2021/06/04