[PULL 0/1] Block patches

[PULL 0/1] Block patches

[Stefan Hajnoczi]

The following changes since commit a3607def89f9cd68c1b994e1030527df33aa91d0:

  Update version for v6.2.0-rc4 release (2021-12-07 17:51:38 -0800)

are available in the Git repository at:

  https://gitlab.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to cf4fbc3030c974fff726756a7ceef8386cdf500b:

  block/nvme: fix infinite loop in nvme_free_req_queue_cb() (2021-12-09 09:19:49 +0000)

----------------------------------------------------------------
Pull request

An infinite loop fix for the userspace NVMe driver.

----------------------------------------------------------------

Stefan Hajnoczi (1):
  block/nvme: fix infinite loop in nvme_free_req_queue_cb()

 block/nvme.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

-- 
2.33.1

[PULL 1/1] block/nvme: fix infinite loop in nvme_free_req_queue_cb()

[Stefan Hajnoczi]

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
    }

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 <stefanha@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20211208152246.244585-1-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@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 */
     }
     qemu_mutex_unlock(&q->lock);
 }
-- 
2.33.1

Re: [PULL 0/1] Block patches

[Peter Maydell]

On Thu, 9 Dec 2021 at 15:21, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> The following changes since commit a3607def89f9cd68c1b994e1030527df33aa91d0:
>
>   Update version for v6.2.0-rc4 release (2021-12-07 17:51:38 -0800)
>
> are available in the Git repository at:
>
>   https://gitlab.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to cf4fbc3030c974fff726756a7ceef8386cdf500b:
>
>   block/nvme: fix infinite loop in nvme_free_req_queue_cb() (2021-12-09 09:19:49 +0000)
>
> ----------------------------------------------------------------
> Pull request
>
> An infinite loop fix for the userspace NVMe driver.
>
> ----------------------------------------------------------------

I'm not running the release cycle this time around, but: it's
already rc4, pull requests by this point need a clear justification
in the cover letter for why they're really release critical.

-- PMM

Re: [PULL 0/1] Block patches

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1185 bytes --]

On Thu, Dec 09, 2021 at 03:46:29PM +0000, Peter Maydell wrote:
> On Thu, 9 Dec 2021 at 15:21, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > The following changes since commit a3607def89f9cd68c1b994e1030527df33aa91d0:
> >
> >   Update version for v6.2.0-rc4 release (2021-12-07 17:51:38 -0800)
> >
> > are available in the Git repository at:
> >
> >   https://gitlab.com/stefanha/qemu.git tags/block-pull-request
> >
> > for you to fetch changes up to cf4fbc3030c974fff726756a7ceef8386cdf500b:
> >
> >   block/nvme: fix infinite loop in nvme_free_req_queue_cb() (2021-12-09 09:19:49 +0000)
> >
> > ----------------------------------------------------------------
> > Pull request
> >
> > An infinite loop fix for the userspace NVMe driver.
> >
> > ----------------------------------------------------------------
> 
> I'm not running the release cycle this time around, but: it's
> already rc4, pull requests by this point need a clear justification
> in the cover letter for why they're really release critical.

It's late, this isn't a show-stopper (block/nvme.c is not widely used).
Let's leave it for the next release cycle and -stable.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PULL 0/1] Block patches

[Richard Henderson]

On 12/9/21 8:34 AM, Stefan Hajnoczi wrote:
>> I'm not running the release cycle this time around, but: it's
>> already rc4, pull requests by this point need a clear justification
>> in the cover letter for why they're really release critical.
> 
> It's late, this isn't a show-stopper (block/nvme.c is not widely used).
> Let's leave it for the next release cycle and -stable.

Good.

Unless you want to re-issue with Cc: qemu-stable included in the patch, this can be the 
first PR of the next devel cycle, since it's already here.  :-)


r~

Re: [PULL 0/1] Block patches

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 755 bytes --]

On Thu, Dec 09, 2021 at 08:53:25AM -0800, Richard Henderson wrote:
> On 12/9/21 8:34 AM, Stefan Hajnoczi wrote:
> > > I'm not running the release cycle this time around, but: it's
> > > already rc4, pull requests by this point need a clear justification
> > > in the cover letter for why they're really release critical.
> > 
> > It's late, this isn't a show-stopper (block/nvme.c is not widely used).
> > Let's leave it for the next release cycle and -stable.
> 
> Good.
> 
> Unless you want to re-issue with Cc: qemu-stable included in the patch, this
> can be the first PR of the next devel cycle, since it's already here.  :-)

Thank you! qemu-stable can merge it separately. I won't add a Cc: tag to
the commit description.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PULL 0/1] Block patches

[Richard Henderson]

On 12/9/21 7:21 AM, Stefan Hajnoczi wrote:
> The following changes since commit a3607def89f9cd68c1b994e1030527df33aa91d0:
> 
>    Update version for v6.2.0-rc4 release (2021-12-07 17:51:38 -0800)
> 
> are available in the Git repository at:
> 
>    https://gitlab.com/stefanha/qemu.git tags/block-pull-request
> 
> for you to fetch changes up to cf4fbc3030c974fff726756a7ceef8386cdf500b:
> 
>    block/nvme: fix infinite loop in nvme_free_req_queue_cb() (2021-12-09 09:19:49 +0000)
> 
> ----------------------------------------------------------------
> Pull request
> 
> An infinite loop fix for the userspace NVMe driver.
> 
> ----------------------------------------------------------------
> 
> Stefan Hajnoczi (1):
>    block/nvme: fix infinite loop in nvme_free_req_queue_cb()
> 
>   block/nvme.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions
Applied, as the beginning of the 7.0 development tree.


r~