| 1 | The following changes since commit 61c265f0660ee476985808c8aa7915617c44fd53: | 1 | The following changes since commit ed8ad9728a9c0eec34db9dff61dfa2f1dd625637: |
|---|---|---|---|
| 2 | 2 | ||
| 3 | Merge remote-tracking branch 'remotes/dgilbert/tags/pull-migration-20200313a' into staging (2020-03-13 10:33:04 +0000) | 3 | Merge tag 'pull-tpm-2023-07-14-1' of https://github.com/stefanberger/qemu-tpm into staging (2023-07-15 14:54:04 +0100) |
| 4 | 4 | ||
| 5 | are available in the Git repository at: | 5 | are available in the Git repository at: |
| 6 | 6 | ||
| 7 | https://github.com/stefanha/qemu.git tags/block-pull-request | 7 | https://gitlab.com/stefanha/qemu.git tags/block-pull-request |
| 8 | 8 | ||
| 9 | for you to fetch changes up to 4ab78b19189a81038e744728ed949d09aa477550: | 9 | for you to fetch changes up to 66547f416a61e0cb711dc76821890242432ba193: |
| 10 | 10 | ||
| 11 | block/io: fix bdrv_co_do_copy_on_readv (2020-03-16 11:46:11 +0000) | 11 | block/nvme: invoke blk_io_plug_call() outside q->lock (2023-07-17 09:17:41 -0400) |
| 12 | 12 | ||
| 13 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
| 14 | Pull request | 14 | Pull request |
| 15 | 15 | ||
| 16 | Fix the hang in the nvme:// block driver during startup. | ||
| 17 | |||
| 16 | ---------------------------------------------------------------- | 18 | ---------------------------------------------------------------- |
| 17 | 19 | ||
| 18 | Vladimir Sementsov-Ogievskiy (1): | 20 | Stefan Hajnoczi (1): |
| 19 | block/io: fix bdrv_co_do_copy_on_readv | 21 | block/nvme: invoke blk_io_plug_call() outside q->lock |
| 20 | 22 | ||
| 21 | block/io.c | 2 +- | 23 | block/nvme.c | 3 ++- |
| 22 | 1 file changed, 1 insertion(+), 1 deletion(-) | 24 | 1 file changed, 2 insertions(+), 1 deletion(-) |
| 23 | 25 | ||
| 24 | -- | 26 | -- |
| 25 | 2.24.1 | 27 | 2.40.1 |
| 26 | diff view generated by jsdifflib |
| 1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 1 | blk_io_plug_call() is invoked outside a blk_io_plug()/blk_io_unplug() |
|---|---|---|---|
| 2 | section while opening the NVMe drive from: | ||
| 2 | 3 | ||
| 3 | Prior to 1143ec5ebf4 it was OK to qemu_iovec_from_buf() from aligned-up | 4 | nvme_file_open() -> |
| 4 | buffer to original qiov, as qemu_iovec_from_buf() will stop at qiov end | 5 | nvme_init() -> |
| 5 | anyway. | 6 | nvme_identify() -> |
| 7 | nvme_admin_cmd_sync() -> | ||
| 8 | nvme_submit_command() -> | ||
| 9 | blk_io_plug_call() | ||
| 6 | 10 | ||
| 7 | But after 1143ec5ebf4 we assume that bdrv_co_do_copy_on_readv works on | 11 | blk_io_plug_call() immediately invokes the given callback when the |
| 8 | part of original qiov, defined by qiov_offset and bytes. So we must not | 12 | current thread is not plugged, as is the case during nvme_file_open(). |
| 9 | touch qiov behind qiov_offset+bytes bound. Fix it. | ||
| 10 | 13 | ||
| 11 | Cc: qemu-stable@nongnu.org # v4.2 | 14 | Unfortunately, nvme_submit_command() calls blk_io_plug_call() with |
| 12 | Fixes: 1143ec5ebf4 | 15 | q->lock still held: |
| 13 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 16 | |
| 14 | Reviewed-by: John Snow <jsnow@redhat.com> | 17 | ... |
| 15 | Message-id: 20200312081949.5350-1-vsementsov@virtuozzo.com | 18 | q->sq.tail = (q->sq.tail + 1) % NVME_QUEUE_SIZE; |
| 19 | q->need_kick++; | ||
| 20 | blk_io_plug_call(nvme_unplug_fn, q); | ||
| 21 | qemu_mutex_unlock(&q->lock); | ||
| 22 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| 23 | |||
| 24 | nvme_unplug_fn() deadlocks trying to acquire q->lock because the lock is | ||
| 25 | already acquired by the same thread. The symptom is that QEMU hangs | ||
| 26 | during startup while opening the NVMe drive. | ||
| 27 | |||
| 28 | Fix this by moving the blk_io_plug_call() outside q->lock. This is safe | ||
| 29 | because no other thread runs code related to this queue and | ||
| 30 | blk_io_plug_call()'s internal state is immune to thread safety issues | ||
| 31 | since it is thread-local. | ||
| 32 | |||
| 33 | Reported-by: Lukáš Doktor <ldoktor@redhat.com> | ||
| 34 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
| 35 | Tested-by: Lukas Doktor <ldoktor@redhat.com> | ||
| 36 | Message-id: 20230712191628.252806-1-stefanha@redhat.com | ||
| 37 | Fixes: f2e590002bd6 ("block/nvme: convert to blk_io_plug_call() API") | ||
| 16 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 38 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
| 17 | --- | 39 | --- |
| 18 | block/io.c | 2 +- | 40 | block/nvme.c | 3 ++- |
| 19 | 1 file changed, 1 insertion(+), 1 deletion(-) | 41 | 1 file changed, 2 insertions(+), 1 deletion(-) |
| 20 | 42 | ||
| 21 | diff --git a/block/io.c b/block/io.c | 43 | diff --git a/block/nvme.c b/block/nvme.c |
| 22 | index XXXXXXX..XXXXXXX 100644 | 44 | index XXXXXXX..XXXXXXX 100644 |
| 23 | --- a/block/io.c | 45 | --- a/block/nvme.c |
| 24 | +++ b/block/io.c | 46 | +++ b/block/nvme.c |
| 25 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child, | 47 | @@ -XXX,XX +XXX,XX @@ static void nvme_submit_command(NVMeQueuePair *q, NVMeRequest *req, |
| 26 | if (!(flags & BDRV_REQ_PREFETCH)) { | 48 | q->sq.tail * NVME_SQ_ENTRY_BYTES, cmd, sizeof(*cmd)); |
| 27 | qemu_iovec_from_buf(qiov, qiov_offset + progress, | 49 | q->sq.tail = (q->sq.tail + 1) % NVME_QUEUE_SIZE; |
| 28 | bounce_buffer + skip_bytes, | 50 | q->need_kick++; |
| 29 | - pnum - skip_bytes); | 51 | + qemu_mutex_unlock(&q->lock); |
| 30 | + MIN(pnum - skip_bytes, bytes - progress)); | 52 | + |
| 31 | } | 53 | blk_io_plug_call(nvme_unplug_fn, q); |
| 32 | } else if (!(flags & BDRV_REQ_PREFETCH)) { | 54 | - qemu_mutex_unlock(&q->lock); |
| 33 | /* Read directly into the destination */ | 55 | } |
| 56 | |||
| 57 | static void nvme_admin_cmd_sync_cb(void *opaque, int ret) | ||
| 34 | -- | 58 | -- |
| 35 | 2.24.1 | 59 | 2.40.1 |
| 36 | 60 | ||
| 61 | diff view generated by jsdifflib |