Re: [Qemu-devel] [PATCH 1/2] block: clean up bdrv_drain_all() throttling comments

[Alex Bligh]

On 25 Sep 2013, at 13:15, Stefan Hajnoczi wrote:

> Since cc0681c45430a1f1a4c2d06e9499b7775afc9a18 ("block: Enable the new
> throttling code in the block layer.") bdrv_drain_all() no longer spins.
> The code used to look as follows:
> 
>  do {
>      busy = qemu_aio_wait();
> 
>      /* FIXME: We do not have timer support here, so this is effectively
>       * a busy wait.
>       */
>      QTAILQ_FOREACH(bs, &bdrv_states, list) {
>          while (qemu_co_enter_next(&bs->throttled_reqs)) {
>              busy = true;
>          }
>      }
>  } while (busy);
> 
> Note that throttle requests are kicked but I/O throttling limits are
> still in effect.  The loop spins until the vm_clock time allows the
> request to make progress and complete.
> 
> The new throttling code introduced bdrv_start_throttled_reqs().  This
> function not only kicks throttled requests but also temporarily disables
> throttling so requests can run.
> 
> The outdated FIXME comment can be removed.  Also drop the busy = true
> assignment since we overwrite it immediately afterwards.
> 
> Signed-off-by: Stefan Hajnoczi <address@hidden>

Signed-off-by: Alex Bligh <address@hidden>

(if I meant 'Reviewed-By:' please change it!)

> ---
> block.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/block.c b/block.c
> index ea4956d..5b276fb 100644
> --- a/block.c
> +++ b/block.c
> @@ -1558,13 +1558,8 @@ void bdrv_drain_all(void)
>     BlockDriverState *bs;
> 
>     while (busy) {
> -        /* FIXME: We do not have timer support here, so this is effectively
> -         * a busy wait.
> -         */
>         QTAILQ_FOREACH(bs, &bdrv_states, list) {
> -            if (bdrv_start_throttled_reqs(bs)) {
> -                busy = true;
> -            }
> +            bdrv_start_throttled_reqs(bs);
>         }
> 
>         busy = bdrv_requests_pending_all();
> -- 
> 1.8.3.1
> 
> 
> 

-- 
Alex Bligh