On 12/8/21 16:22, Stefan Hajnoczi wrote:
> When the request free list is exhausted the coroutine waits on
> q->free_req_queue for the next free request. Whenever a request is
> completed a BH is scheduled to invoke nvme_free_req_queue_cb() and wake
> up waiting coroutines.
>
> 1. nvme_get_free_req() waits for a free request:
>
> while (q->free_req_head == -1) {
> ...
> trace_nvme_free_req_queue_wait(q->s, q->index);
> qemu_co_queue_wait(&q->free_req_queue, &q->lock);
> ...
> }
>
> 2. nvme_free_req_queue_cb() wakes up the coroutine:
>
> while (qemu_co_enter_next(&q->free_req_queue, &q->lock)) {
> ^--- infinite loop when free_req_head == -1
Ouch.
> }
>
> nvme_free_req_queue_cb() and the coroutine form an infinite loop when
> q->free_req_head == -1. Fix this by checking q->free_req_head in
> nvme_free_req_queue_cb(). If the free request list is exhausted, don't
> wake waiting coroutines. Eventually an in-flight request will complete
> and the BH will be scheduled again, guaranteeing forward progress.
>
> Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
> ---
> block/nvme.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/block/nvme.c b/block/nvme.c
> index e4f336d79c..fa360b9b3c 100644
> --- a/block/nvme.c
> +++ b/block/nvme.c
> @@ -206,8 +206,9 @@ static void nvme_free_req_queue_cb(void *opaque)
> NVMeQueuePair *q = opaque;
>
> qemu_mutex_lock(&q->lock);
> - while (qemu_co_enter_next(&q->free_req_queue, &q->lock)) {
> - /* Retry all pending requests */
> + while (q->free_req_head != -1 &&
> + qemu_co_enter_next(&q->free_req_queue, &q->lock)) {
> + /* Retry waiting requests */
Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
> }
> qemu_mutex_unlock(&q->lock);
> }
>
