bdev: retry IOs on ENOMEM when pushing bounce data/md

Signed-off-by: Konrad Sztyber <konrad.sztyber@intel.com>
Change-Id: Ia7634b570eb7d04c22003337a46630d152171157
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/17764
Reviewed-by: Shuhei Matsumoto <smatsumoto@nvidia.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Aleksey Marchuk <alexeymar@nvidia.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>

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);
+				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));
+						    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)
 		}
 	}
 
+	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);
+			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));
+					    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)
 		}
 	}
 
+	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

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);