Re: [Qemu-devel] [PATCH v3 19/29] block: Allow wait_serialising_requests() at any point

["Benoît Canet" <benoit.canet@irqsave.net>]

Le Friday 17 Jan 2014 à 15:15:09 (+0100), Kevin Wolf a écrit :
> We can only have a single wait_serialising_requests() call per request
> because otherwise we can run into deadlocks where requests are waiting
> for each other. The same is true when wait_serialising_requests() is not
> at the very beginning of a request, so that other requests can be issued
> between the start of the tracking and wait_serialising_requests().
> 
> Fix this by changing wait_serialising_requests() to ignore requests that
> are already (directly or indirectly) waiting for the calling request.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
>  block.c                   | 13 ++++++++++---
>  include/block/block_int.h |  2 ++
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/block.c b/block.c
> index e72966a..55e8c69 100644
> --- a/block.c
> +++ b/block.c
> @@ -2148,9 +2148,16 @@ static void coroutine_fn wait_serialising_requests(BdrvTrackedRequest *self)
>                   */
>                  assert(qemu_coroutine_self() != req->co);
>  
> -                qemu_co_queue_wait(&req->wait_queue);
> -                retry = true;
> -                break;
> +                /* If the request is already (indirectly) waiting for us, or
> +                 * will wait for us as soon as it wakes up, then just go on
> +                 * (instead of producing a deadlock in the former case). */
> +                if (!req->waiting_for) {
> +                    self->waiting_for = req;
> +                    qemu_co_queue_wait(&req->wait_queue);
> +                    self->waiting_for = NULL;
> +                    retry = true;
> +                    break;
> +                }
>              }
>          }
>      } while (retry);
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index ccd2c68..fdf0e0b 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -68,6 +68,8 @@ typedef struct BdrvTrackedRequest {
>      QLIST_ENTRY(BdrvTrackedRequest) list;
>      Coroutine *co; /* owner, used for deadlock detection */
>      CoQueue wait_queue; /* coroutines blocked on this request */
> +
> +    struct BdrvTrackedRequest *waiting_for;
>  } BdrvTrackedRequest;
>  
>  struct BlockDriver {
> -- 
> 1.8.1.4
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>