Re: [PATCH 04/13] block: Remove drained_end_counter

[Vladimir Sementsov-Ogievskiy]

On 11/8/22 15:37, Kevin Wolf wrote:
> drained_end_counter is unused now, nobody changes its value any more. It
> can be removed.
>
> In cases where we had two almost identical functions that only differed
> in whether the caller passes drained_end_counter, or whether they would
> poll for a local drained_end_counter to reach 0, these become a single
> function.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

[..]

>   
>   /* Recursively call BlockDriver.bdrv_drain_begin/end callbacks */

Not about this patch, but what is recursive in bdrv_drain_invoke() ?

> -static void bdrv_drain_invoke(BlockDriverState *bs, bool begin,
> -                              int *drained_end_counter)
> +static void bdrv_drain_invoke(BlockDriverState *bs, bool begin)
>   {
>       if (!bs->drv || (begin && !bs->drv->bdrv_drain_begin) ||
>               (!begin && !bs->drv->bdrv_drain_end)) {

[..]

>   
>   /**
>    * This function does not poll, nor must any of its recursively called
> - * functions.  The *drained_end_counter pointee will be incremented
> - * once 

Seems that is wrong already after previous commit.. Not critical

> for every background operation scheduled, and decremented once
> - * the operation settles.  Therefore, the pointer must remain valid
> - * until the pointee reaches 0.  That implies that whoever sets up the
> - * pointee has to poll until it is 0.
> - *
> - * We use atomic operations to access *drained_end_counter, because
> - * (1) when called from bdrv_set_aio_context_ignore(), the subgraph of
> - *     @bs may contain nodes in different AioContexts,
> - * (2) bdrv_drain_all_end() uses the same counter for all nodes,
> - *     regardless of which AioContext they are in.
> + * functions.
>    */
>   static void bdrv_do_drained_end(BlockDriverState *bs, bool recursive,
> -                                BdrvChild *parent, bool ignore_bds_parents,
> -                                int *drained_end_counter)
> +                                BdrvChild *parent, bool ignore_bds_parents)
>   {
>       BdrvChild *child;
>       int old_quiesce_counter;
>   
> -    assert(drained_end_counter != NULL);
> -

[..]

--
Best regards,
Vladimir