Re: [PATCH for-5.0 v2 2/3] block: Increase BB.in_flight for coroutine interfaces

[Vladimir Sementsov-Ogievskiy]

06.04.2020 20:14, Kevin Wolf wrote:
> External callers of blk_co_*() don't currently increase the
> BlockBackend.in_flight counter, but calls from blk_aio_*() do, so there
> is an inconsistency whether the counter has been increased or not.
>
> This patch moves the actual operations to static functions that can
> later know they will always be called with in_flight increased exactly
> once, even for external callers using the blk_co_*() coroutine
> interfaces.
>
> If the public blk_co_*() interface is unused, remove it.
>
> Signed-off-by: Kevin Wolf <address@hidden>


Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>


Still, did you consider instead just move inc/dec to _co_ functions, like


@@ -1154,6 +1154,7 @@ int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t 
offset,
     int ret;
     BlockDriverState *bs;

+    blk_inc_in_flight(blk);
     blk_wait_while_drained(blk);

     /* Call blk_bs() only after waiting, the graph may have changed */
@@ -1175,6 +1176,7 @@ int coroutine_fn blk_co_preadv(BlockBackend *blk, int64_t 
offset,

     ret = bdrv_co_preadv(blk->root, offset, bytes, qiov, flags);
     bdrv_dec_in_flight(bs);
+    blk_dec_in_flight(blk);
     return ret;
 }

@@ -1337,7 +1339,6 @@ static void blk_aio_complete(BlkAioEmAIOCB *acb)
 {
     if (acb->has_returned) {
         acb->common.cb(acb->common.opaque, acb->rwco.ret);
-        blk_dec_in_flight(acb->rwco.blk);
         qemu_aio_unref(acb);
     }
 }
@@ -1357,7 +1358,6 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, 
int64_t offset, int bytes,
     BlkAioEmAIOCB *acb;
     Coroutine *co;

-    blk_inc_in_flight(blk);
     acb = blk_aio_get(&blk_aio_em_aiocb_info, blk, cb, opaque);
     acb->rwco = (BlkRwCo) {
         .blk    = blk,
@@ -1388,9 +1388,7 @@ static void blk_aio_read_entry(void *opaque)
     QEMUIOVector *qiov = rwco->iobuf;

     if (rwco->blk->quiesce_counter) {
-        blk_dec_in_flight(rwco->blk);
         blk_wait_while_drained(rwco->blk);
-        blk_inc_in_flight(rwco->blk);
     }

     assert(qiov->size == acb->bytes);



(and same for write, ioctl, flush, discard). It seems more consistent.. Should 
it work?


--
Best regards,
Vladimir