qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PULL for-8.1 0/1] Block patches
@ 2023-07-17 14:37 Stefan Hajnoczi
  2023-07-17 14:37 ` [PULL for-8.1 1/1] block/nvme: invoke blk_io_plug_call() outside q->lock Stefan Hajnoczi
  2023-07-17 19:12 ` [PULL for-8.1 0/1] Block patches Richard Henderson
  0 siblings, 2 replies; 3+ messages in thread
From: Stefan Hajnoczi @ 2023-07-17 14:37 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Richard Henderson,
	qemu-block, Fam Zheng, Philippe Mathieu-Daudé

The following changes since commit ed8ad9728a9c0eec34db9dff61dfa2f1dd625637:

  Merge tag 'pull-tpm-2023-07-14-1' of https://github.com/stefanberger/qemu-tpm into staging (2023-07-15 14:54:04 +0100)

are available in the Git repository at:

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

for you to fetch changes up to 66547f416a61e0cb711dc76821890242432ba193:

  block/nvme: invoke blk_io_plug_call() outside q->lock (2023-07-17 09:17:41 -0400)

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

Fix the hang in the nvme:// block driver during startup.

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

Stefan Hajnoczi (1):
  block/nvme: invoke blk_io_plug_call() outside q->lock

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

-- 
2.40.1



^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PULL for-8.1 1/1] block/nvme: invoke blk_io_plug_call() outside q->lock
  2023-07-17 14:37 [PULL for-8.1 0/1] Block patches Stefan Hajnoczi
@ 2023-07-17 14:37 ` Stefan Hajnoczi
  2023-07-17 19:12 ` [PULL for-8.1 0/1] Block patches Richard Henderson
  1 sibling, 0 replies; 3+ messages in thread
From: Stefan Hajnoczi @ 2023-07-17 14:37 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Hanna Reitz, Stefan Hajnoczi, Richard Henderson,
	qemu-block, Fam Zheng, Philippe Mathieu-Daudé,
	Lukáš Doktor

blk_io_plug_call() is invoked outside a blk_io_plug()/blk_io_unplug()
section while opening the NVMe drive from:

  nvme_file_open() ->
  nvme_init() ->
  nvme_identify() ->
  nvme_admin_cmd_sync() ->
  nvme_submit_command() ->
  blk_io_plug_call()

blk_io_plug_call() immediately invokes the given callback when the
current thread is not plugged, as is the case during nvme_file_open().

Unfortunately, nvme_submit_command() calls blk_io_plug_call() with
q->lock still held:

    ...
    q->sq.tail = (q->sq.tail + 1) % NVME_QUEUE_SIZE;
    q->need_kick++;
    blk_io_plug_call(nvme_unplug_fn, q);
    qemu_mutex_unlock(&q->lock);
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^

nvme_unplug_fn() deadlocks trying to acquire q->lock because the lock is
already acquired by the same thread. The symptom is that QEMU hangs
during startup while opening the NVMe drive.

Fix this by moving the blk_io_plug_call() outside q->lock. This is safe
because no other thread runs code related to this queue and
blk_io_plug_call()'s internal state is immune to thread safety issues
since it is thread-local.

Reported-by: Lukáš Doktor <ldoktor@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Lukas Doktor <ldoktor@redhat.com>
Message-id: 20230712191628.252806-1-stefanha@redhat.com
Fixes: f2e590002bd6 ("block/nvme: convert to blk_io_plug_call() API")
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/nvme.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/nvme.c b/block/nvme.c
index 7ca85bc44a..b6e95f0b7e 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -501,8 +501,9 @@ static void nvme_submit_command(NVMeQueuePair *q, NVMeRequest *req,
            q->sq.tail * NVME_SQ_ENTRY_BYTES, cmd, sizeof(*cmd));
     q->sq.tail = (q->sq.tail + 1) % NVME_QUEUE_SIZE;
     q->need_kick++;
+    qemu_mutex_unlock(&q->lock);
+
     blk_io_plug_call(nvme_unplug_fn, q);
-    qemu_mutex_unlock(&q->lock);
 }
 
 static void nvme_admin_cmd_sync_cb(void *opaque, int ret)
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PULL for-8.1 0/1] Block patches
  2023-07-17 14:37 [PULL for-8.1 0/1] Block patches Stefan Hajnoczi
  2023-07-17 14:37 ` [PULL for-8.1 1/1] block/nvme: invoke blk_io_plug_call() outside q->lock Stefan Hajnoczi
@ 2023-07-17 19:12 ` Richard Henderson
  1 sibling, 0 replies; 3+ messages in thread
From: Richard Henderson @ 2023-07-17 19:12 UTC (permalink / raw)
  To: Stefan Hajnoczi, qemu-devel
  Cc: Kevin Wolf, Hanna Reitz, Richard Henderson, qemu-block, Fam Zheng,
	Philippe Mathieu-Daudé

On 7/17/23 15:37, Stefan Hajnoczi wrote:
> The following changes since commit ed8ad9728a9c0eec34db9dff61dfa2f1dd625637:
> 
>    Merge tag 'pull-tpm-2023-07-14-1' ofhttps://github.com/stefanberger/qemu-tpm  into staging (2023-07-15 14:54:04 +0100)
> 
> are available in the Git repository at:
> 
>    https://gitlab.com/stefanha/qemu.git  tags/block-pull-request
> 
> for you to fetch changes up to 66547f416a61e0cb711dc76821890242432ba193:
> 
>    block/nvme: invoke blk_io_plug_call() outside q->lock (2023-07-17 09:17:41 -0400)
> 
> ----------------------------------------------------------------
> Pull request
> 
> Fix the hang in the nvme:// block driver during startup.

Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/8.1 as appropriate.


r~



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-07-17 19:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-17 14:37 [PULL for-8.1 0/1] Block patches Stefan Hajnoczi
2023-07-17 14:37 ` [PULL for-8.1 1/1] block/nvme: invoke blk_io_plug_call() outside q->lock Stefan Hajnoczi
2023-07-17 19:12 ` [PULL for-8.1 0/1] Block patches Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).