Re: [PATCH 01/13] qed: Don't yield in bdrv_qed_co_drain_begin()

[Stefan Hajnoczi <stefanha@redhat.com>]

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

On Tue, Nov 08, 2022 at 01:37:26PM +0100, Kevin Wolf wrote:
> @@ -310,9 +309,20 @@ static void coroutine_fn qed_need_check_timer_entry(void *opaque)
>      (void) ret;
>  }
>  
> +static void coroutine_fn qed_need_check_timer_entry(void *opaque)
> +{
> +    BDRVQEDState *s = opaque;
> +
> +    qed_need_check_timer(opaque);
> +    bdrv_dec_in_flight(s->bs);
> +}
> +
>  static void qed_need_check_timer_cb(void *opaque)
>  {
> +    BDRVQEDState *s = opaque;
>      Coroutine *co = qemu_coroutine_create(qed_need_check_timer_entry, opaque);
> +
> +    bdrv_inc_in_flight(s->bs);
>      qemu_coroutine_enter(co);
>  }
>  
> @@ -363,8 +373,12 @@ static void coroutine_fn bdrv_qed_co_drain_begin(BlockDriverState *bs)
>       * header is flushed.
>       */
>      if (s->need_check_timer && timer_pending(s->need_check_timer)) {
> +        Coroutine *co;
> +
>          qed_cancel_need_check_timer(s);
> -        qed_need_check_timer_entry(s);
> +        co = qemu_coroutine_create(qed_need_check_timer_entry, s);
> +        bdrv_inc_in_flight(bs);

Please include comments that indicate where inc/dec are paired. This is
like pairing memory barriers where it can be very hard to know after the
code has been written (and modified).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]