From f8a33650d23f00f77dc9ebb22a15722cc6a852b5 Mon Sep 17 00:00:00 2001 From: Konrad Sztyber Date: Thu, 20 Apr 2023 08:49:53 +0200 Subject: [PATCH] bdev: retry IOs on ENOMEM when pushing bounce data/md Signed-off-by: Konrad Sztyber Change-Id: Ia7634b570eb7d04c22003337a46630d152171157 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17764 Reviewed-by: Shuhei Matsumoto Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Community-CI: Mellanox Build Bot Reviewed-by: Aleksey Marchuk Reviewed-by: Jim Harris --- lib/bdev/bdev.c | 47 ++++++++++++++++++++++++----- test/unit/lib/bdev/bdev.c/bdev_ut.c | 31 +++++++++++++++++++ 2 files changed, 71 insertions(+), 7 deletions(-) diff --git a/lib/bdev/bdev.c b/lib/bdev/bdev.c index b6758a1c9..9ee0092c3 100644 --- a/lib/bdev/bdev.c +++ b/lib/bdev/bdev.c @@ -371,6 +371,8 @@ enum bdev_io_retry_state { BDEV_IO_RETRY_STATE_PULL, BDEV_IO_RETRY_STATE_PULL_MD, BDEV_IO_RETRY_STATE_SUBMIT, + BDEV_IO_RETRY_STATE_PUSH, + BDEV_IO_RETRY_STATE_PUSH_MD, }; #define __bdev_to_io_dev(bdev) (((char *)bdev) + 1) @@ -380,6 +382,8 @@ enum bdev_io_retry_state { static inline void bdev_io_complete(void *ctx); static inline void bdev_io_complete_unsubmitted(struct spdk_bdev_io *bdev_io); +static void bdev_io_push_bounce_md_buf(struct spdk_bdev_io *bdev_io); +static void bdev_io_push_bounce_data(struct spdk_bdev_io *bdev_io); static void bdev_write_zero_buffer_done(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg); static int bdev_write_zero_buffer(struct spdk_bdev_io *bdev_io); @@ -1482,6 +1486,12 @@ bdev_ch_retry_io(struct spdk_bdev_channel *bdev_ch) case BDEV_IO_RETRY_STATE_PULL_MD: bdev_io_pull_md_buf(bdev_io); break; + case BDEV_IO_RETRY_STATE_PUSH: + bdev_io_push_bounce_data(bdev_io); + break; + case BDEV_IO_RETRY_STATE_PUSH_MD: + bdev_io_push_bounce_md_buf(bdev_io); + break; default: assert(0 && "invalid retry state"); break; @@ -1557,6 +1567,11 @@ _bdev_io_push_bounce_md_buf_done(void *ctx, int rc) TAILQ_REMOVE(&ch->io_memory_domain, bdev_io, internal.link); bdev_io_decrement_outstanding(ch, ch->shared_resource); + + if (spdk_unlikely(!TAILQ_EMPTY(&ch->shared_resource->nomem_io))) { + bdev_ch_retry_io(ch); + } + bdev_io->internal.data_transfer_cpl(bdev_io, rc); } @@ -1589,8 +1604,11 @@ bdev_io_push_bounce_md_buf(struct spdk_bdev_io *bdev_io) } TAILQ_REMOVE(&ch->io_memory_domain, bdev_io, internal.link); bdev_io_decrement_outstanding(ch, ch->shared_resource); - SPDK_ERRLOG("Failed to push md to memory domain %s\n", - spdk_memory_domain_get_dma_device_id(bdev_io->internal.memory_domain)); + if (rc != -ENOMEM) { + SPDK_ERRLOG("Failed to push md to memory domain %s\n", + spdk_memory_domain_get_dma_device_id( + bdev_io->internal.memory_domain)); + } } else { memcpy(bdev_io->internal.orig_md_iov.iov_base, bdev_io->u.bdev.md_buf, bdev_io->internal.orig_md_iov.iov_len); @@ -1598,8 +1616,12 @@ bdev_io_push_bounce_md_buf(struct spdk_bdev_io *bdev_io) } } - assert(bdev_io->internal.data_transfer_cpl); - bdev_io->internal.data_transfer_cpl(bdev_io, rc); + if (spdk_unlikely(rc == -ENOMEM)) { + bdev_queue_nomem_io_head(ch->shared_resource, bdev_io, BDEV_IO_RETRY_STATE_PUSH_MD); + } else { + assert(bdev_io->internal.data_transfer_cpl); + bdev_io->internal.data_transfer_cpl(bdev_io, rc); + } } static inline void @@ -1632,6 +1654,10 @@ _bdev_io_push_bounce_data_buffer_done(void *ctx, int status) TAILQ_REMOVE(&ch->io_memory_domain, bdev_io, internal.link); bdev_io_decrement_outstanding(ch, ch->shared_resource); + if (spdk_unlikely(!TAILQ_EMPTY(&ch->shared_resource->nomem_io))) { + bdev_ch_retry_io(ch); + } + bdev_io_push_bounce_data_buffer_done(ctx, status); } @@ -1662,8 +1688,11 @@ bdev_io_push_bounce_data(struct spdk_bdev_io *bdev_io) TAILQ_REMOVE(&ch->io_memory_domain, bdev_io, internal.link); bdev_io_decrement_outstanding(ch, ch->shared_resource); - SPDK_ERRLOG("Failed to push data to memory domain %s\n", - spdk_memory_domain_get_dma_device_id(bdev_io->internal.memory_domain)); + if (rc != -ENOMEM) { + SPDK_ERRLOG("Failed to push data to memory domain %s\n", + spdk_memory_domain_get_dma_device_id( + bdev_io->internal.memory_domain)); + } } else { spdk_copy_buf_to_iovs(bdev_io->internal.orig_iovs, bdev_io->internal.orig_iovcnt, @@ -1672,7 +1701,11 @@ bdev_io_push_bounce_data(struct spdk_bdev_io *bdev_io) } } - bdev_io_push_bounce_data_buffer_done(bdev_io, rc); + if (spdk_unlikely(rc == -ENOMEM)) { + bdev_queue_nomem_io_head(ch->shared_resource, bdev_io, BDEV_IO_RETRY_STATE_PUSH); + } else { + bdev_io_push_bounce_data_buffer_done(bdev_io, rc); + } } static inline void diff --git a/test/unit/lib/bdev/bdev.c/bdev_ut.c b/test/unit/lib/bdev/bdev.c/bdev_ut.c index 2216fc37b..d1452f859 100644 --- a/test/unit/lib/bdev/bdev.c/bdev_ut.c +++ b/test/unit/lib/bdev/bdev.c/bdev_ut.c @@ -5895,6 +5895,37 @@ bdev_io_ext_bounce_buffer(void) stub_complete_io(1); CU_ASSERT(g_io_done == true); + /* Verify the request is queued after receiving ENOMEM from push */ + g_io_done = false; + expected_io = ut_alloc_expected_io(SPDK_BDEV_IO_TYPE_READ, 32, 14, 1); + ut_expected_io_set_iov(expected_io, 0, iov.iov_base, iov.iov_len); + TAILQ_INSERT_TAIL(&g_bdev_ut_channel->expected_io, expected_io, link); + + MOCK_SET(spdk_memory_domain_push_data, -ENOMEM); + rc = spdk_bdev_readv_blocks_ext(desc, io_ch, &iov, 1, 32, 14, io_done, NULL, &ext_io_opts); + CU_ASSERT(rc == 0); + CU_ASSERT(g_io_done == false); + CU_ASSERT(g_bdev_ut_channel->outstanding_io_count == 1); + + aux_io = ut_alloc_expected_io(SPDK_BDEV_IO_TYPE_WRITE, 32, 14, 1); + ut_expected_io_set_iov(aux_io, 0, iov.iov_base, iov.iov_len); + TAILQ_INSERT_TAIL(&g_bdev_ut_channel->expected_io, aux_io, link); + rc = spdk_bdev_writev_blocks(desc, io_ch, &iov, 1, 32, 14, io_done, NULL); + CU_ASSERT(rc == 0); + CU_ASSERT(g_bdev_ut_channel->outstanding_io_count == 2); + + stub_complete_io(1); + /* The IO isn't done yet, it's still waiting on push */ + CU_ASSERT(g_io_done == false); + CU_ASSERT(g_bdev_ut_channel->outstanding_io_count == 1); + MOCK_CLEAR(spdk_memory_domain_push_data); + g_memory_domain_push_data_called = false; + /* Completing the second IO should also trigger push on the first one */ + stub_complete_io(1); + CU_ASSERT(g_io_done == true); + CU_ASSERT(g_memory_domain_push_data_called == true); + CU_ASSERT(g_bdev_ut_channel->outstanding_io_count == 0); + spdk_put_io_channel(io_ch); spdk_bdev_close(desc); free_bdev(bdev);