From f0ec7bc6e7152264f64b0e77e6bd5cf1d0a58419 Mon Sep 17 00:00:00 2001 From: Tomasz Zawadzki Date: Thu, 5 Jul 2018 03:32:48 -0400 Subject: [PATCH] scsi/bdev: use spdk_bdev_queue_io_wait() New function was added in bdev layer to allow handling spdk_bdev_io buffer exhaustion. This patch adds that functionality to scsi bdev. Change-Id: Ia6a5be871ae09a4d1166991925f0a44f3b355bdd Signed-off-by: Tomasz Zawadzki Reviewed-on: https://review.gerrithub.io/417032 Tested-by: SPDK Automated Test System Reviewed-by: Jim Harris Reviewed-by: Ben Walker --- include/spdk/scsi.h | 2 + lib/scsi/scsi_bdev.c | 102 +++++++++++++++--- test/unit/lib/scsi/scsi_bdev.c/scsi_bdev_ut.c | 49 +++++++-- 3 files changed, 133 insertions(+), 20 deletions(-) diff --git a/include/spdk/scsi.h b/include/spdk/scsi.h index 308bd3a773..33bada6d7d 100644 --- a/include/spdk/scsi.h +++ b/include/spdk/scsi.h @@ -41,6 +41,7 @@ #include "spdk/stdinc.h" +#include "spdk/bdev.h" #include "spdk/queue.h" #ifdef __cplusplus @@ -140,6 +141,7 @@ struct spdk_scsi_task { TAILQ_ENTRY(spdk_scsi_task) scsi_link; uint32_t abort_id; + struct spdk_bdev_io_wait_entry bdev_io_wait; }; struct spdk_scsi_port; diff --git a/lib/scsi/scsi_bdev.c b/lib/scsi/scsi_bdev.c index 60cb776b95..289d862685 100644 --- a/lib/scsi/scsi_bdev.c +++ b/lib/scsi/scsi_bdev.c @@ -60,6 +60,8 @@ #define INQUIRY_OFFSET(field) offsetof(struct spdk_scsi_cdb_inquiry_data, field) + \ sizeof(((struct spdk_scsi_cdb_inquiry_data *)0x0)->field) +static void spdk_bdev_scsi_process_block_resubmit(void *arg); + static int spdk_hex2bin(char ch) { @@ -1294,6 +1296,24 @@ spdk_bdev_scsi_task_complete_mgmt(struct spdk_bdev_io *bdev_io, bool success, spdk_scsi_lun_complete_mgmt_task(task->lun, task); } +static void +spdk_bdev_scsi_queue_io(struct spdk_scsi_task *task, spdk_bdev_io_wait_cb cb_fn, void *cb_arg) +{ + struct spdk_scsi_lun *lun = task->lun; + struct spdk_bdev *bdev = lun->bdev; + struct spdk_io_channel *ch = lun->io_channel; + int rc; + + task->bdev_io_wait.bdev = bdev; + task->bdev_io_wait.cb_fn = cb_fn; + task->bdev_io_wait.cb_arg = cb_arg; + + rc = spdk_bdev_queue_io_wait(bdev, ch, &task->bdev_io_wait); + if (rc != 0) { + assert(false); + } +} + static int spdk_bdev_scsi_read(struct spdk_bdev *bdev, struct spdk_bdev_desc *bdev_desc, struct spdk_io_channel *bdev_ch, struct spdk_scsi_task *task, @@ -1317,7 +1337,12 @@ spdk_bdev_scsi_read(struct spdk_bdev *bdev, struct spdk_bdev_desc *bdev_desc, rc = spdk_bdev_readv(bdev_desc, bdev_ch, task->iovs, task->iovcnt, offset, nbytes, spdk_bdev_scsi_task_complete_cmd, task); + if (rc) { + if (rc == -ENOMEM) { + spdk_bdev_scsi_queue_io(task, spdk_bdev_scsi_process_block_resubmit, task); + return SPDK_SCSI_TASK_PENDING; + } SPDK_ERRLOG("spdk_bdev_readv() failed\n"); spdk_scsi_task_set_status(task, SPDK_SCSI_STATUS_CHECK_CONDITION, SPDK_SCSI_SENSE_NO_SENSE, @@ -1365,6 +1390,10 @@ spdk_bdev_scsi_write(struct spdk_bdev *bdev, struct spdk_bdev_desc *bdev_desc, task); if (rc) { + if (rc == -ENOMEM) { + spdk_bdev_scsi_queue_io(task, spdk_bdev_scsi_process_block_resubmit, task); + return SPDK_SCSI_TASK_PENDING; + } SPDK_ERRLOG("spdk_bdev_writev failed\n"); spdk_scsi_task_set_status(task, SPDK_SCSI_STATUS_CHECK_CONDITION, SPDK_SCSI_SENSE_NO_SENSE, @@ -1408,6 +1437,10 @@ spdk_bdev_scsi_sync(struct spdk_bdev *bdev, struct spdk_bdev_desc *bdev_desc, spdk_bdev_scsi_task_complete_cmd, task); if (rc) { + if (rc == -ENOMEM) { + spdk_bdev_scsi_queue_io(task, spdk_bdev_scsi_process_block_resubmit, task); + return SPDK_SCSI_TASK_PENDING; + } SPDK_ERRLOG("spdk_bdev_flush_blocks() failed\n"); spdk_scsi_task_set_status(task, SPDK_SCSI_STATUS_CHECK_CONDITION, SPDK_SCSI_SENSE_NO_SENSE, @@ -1482,6 +1515,9 @@ struct spdk_bdev_scsi_unmap_ctx { uint32_t count; }; +static int spdk_bdev_scsi_unmap(struct spdk_bdev *bdev, struct spdk_bdev_desc *bdev_desc, + struct spdk_io_channel *bdev_ch, struct spdk_scsi_task *task, struct spdk_bdev_scsi_unmap_ctx *ctx); + static void spdk_bdev_scsi_task_complete_unmap_cmd(struct spdk_bdev_io *bdev_io, bool success, void *cb_arg) @@ -1539,29 +1575,41 @@ __copy_desc(struct spdk_bdev_scsi_unmap_ctx *ctx, uint8_t *data, size_t data_len return desc_count; } +static void +spdk_bdev_scsi_unmap_resubmit(void *arg) +{ + struct spdk_bdev_scsi_unmap_ctx *ctx = arg; + struct spdk_scsi_task *task = ctx->task; + struct spdk_scsi_lun *lun = task->lun; + + spdk_bdev_scsi_unmap(lun->bdev, lun->bdev_desc, lun->io_channel, task, ctx); +} + static int spdk_bdev_scsi_unmap(struct spdk_bdev *bdev, struct spdk_bdev_desc *bdev_desc, - struct spdk_io_channel *bdev_ch, struct spdk_scsi_task *task) + struct spdk_io_channel *bdev_ch, struct spdk_scsi_task *task, struct spdk_bdev_scsi_unmap_ctx *ctx) { uint8_t *data; - struct spdk_bdev_scsi_unmap_ctx *ctx; int desc_count, i; int data_len; int rc; assert(task->status == SPDK_SCSI_STATUS_GOOD); - ctx = calloc(1, sizeof(*ctx)); - if (!ctx) { - spdk_scsi_task_set_status(task, SPDK_SCSI_STATUS_CHECK_CONDITION, - SPDK_SCSI_SENSE_NO_SENSE, - SPDK_SCSI_ASC_NO_ADDITIONAL_SENSE, - SPDK_SCSI_ASCQ_CAUSE_NOT_REPORTABLE); - return SPDK_SCSI_TASK_COMPLETE; + if (ctx == NULL) { + ctx = calloc(1, sizeof(*ctx)); + if (!ctx) { + spdk_scsi_task_set_status(task, SPDK_SCSI_STATUS_CHECK_CONDITION, + SPDK_SCSI_SENSE_NO_SENSE, + SPDK_SCSI_ASC_NO_ADDITIONAL_SENSE, + SPDK_SCSI_ASCQ_CAUSE_NOT_REPORTABLE); + return SPDK_SCSI_TASK_COMPLETE; + } + + ctx->task = task; + ctx->count = 0; } - ctx->task = task; - ctx->count = 0; if (task->iovcnt == 1) { data = (uint8_t *)task->iovs[0].iov_base; @@ -1584,7 +1632,7 @@ spdk_bdev_scsi_unmap(struct spdk_bdev *bdev, struct spdk_bdev_desc *bdev_desc, return SPDK_SCSI_TASK_COMPLETE; } - for (i = 0; i < desc_count; i++) { + for (i = ctx->count; i < desc_count; i++) { struct spdk_scsi_unmap_bdesc *desc; uint64_t offset_blocks; uint64_t num_blocks; @@ -1603,6 +1651,12 @@ spdk_bdev_scsi_unmap(struct spdk_bdev *bdev, struct spdk_bdev_desc *bdev_desc, spdk_bdev_scsi_task_complete_unmap_cmd, ctx); if (rc) { + if (rc == -ENOMEM) { + spdk_bdev_scsi_queue_io(task, spdk_bdev_scsi_unmap_resubmit, ctx); + /* Unmap was not yet submitted to bdev */ + ctx->count--; + return SPDK_SCSI_TASK_PENDING; + } SPDK_ERRLOG("SCSI Unmapping failed\n"); spdk_scsi_task_set_status(task, SPDK_SCSI_STATUS_CHECK_CONDITION, SPDK_SCSI_SENSE_NO_SENSE, @@ -1737,7 +1791,7 @@ spdk_bdev_scsi_process_block(struct spdk_scsi_task *task) break; case SPDK_SBC_UNMAP: - return spdk_bdev_scsi_unmap(bdev, lun->bdev_desc, lun->io_channel, task); + return spdk_bdev_scsi_unmap(bdev, lun->bdev_desc, lun->io_channel, task, NULL); default: return SPDK_SCSI_TASK_UNKNOWN; @@ -1746,6 +1800,14 @@ spdk_bdev_scsi_process_block(struct spdk_scsi_task *task) return SPDK_SCSI_TASK_COMPLETE; } +static void +spdk_bdev_scsi_process_block_resubmit(void *arg) +{ + struct spdk_scsi_task *task = arg; + + spdk_bdev_scsi_process_block(task); +} + static int spdk_bdev_scsi_check_len(struct spdk_scsi_task *task, int len, int min_len) { @@ -2033,10 +2095,22 @@ spdk_bdev_scsi_execute(struct spdk_scsi_task *task) return rc; } +static void +spdk_bdev_scsi_reset_resubmit(void *arg) +{ + struct spdk_scsi_task *task = arg; + + spdk_bdev_scsi_reset(task); +} + void spdk_bdev_scsi_reset(struct spdk_scsi_task *task) { struct spdk_scsi_lun *lun = task->lun; + int rc; - spdk_bdev_reset(lun->bdev_desc, lun->io_channel, spdk_bdev_scsi_task_complete_mgmt, task); + rc = spdk_bdev_reset(lun->bdev_desc, lun->io_channel, spdk_bdev_scsi_task_complete_mgmt, task); + if (rc == -ENOMEM) { + spdk_bdev_scsi_queue_io(task, spdk_bdev_scsi_reset_resubmit, task); + } } diff --git a/test/unit/lib/scsi/scsi_bdev.c/scsi_bdev_ut.c b/test/unit/lib/scsi/scsi_bdev.c/scsi_bdev_ut.c index 16c0564215..ef5349e9c7 100644 --- a/test/unit/lib/scsi/scsi_bdev.c/scsi_bdev_ut.c +++ b/test/unit/lib/scsi/scsi_bdev.c/scsi_bdev_ut.c @@ -47,6 +47,9 @@ static uint64_t g_test_bdev_num_blocks; TAILQ_HEAD(, spdk_bdev_io) g_bdev_io_queue; int g_scsi_cb_called = 0; +TAILQ_HEAD(, spdk_bdev_io_wait_entry) g_io_wait_queue; +bool g_bdev_io_pool_full = false; + void * spdk_dma_malloc(size_t size, size_t align, uint64_t *phys_addr) { @@ -191,12 +194,21 @@ static void ut_bdev_io_flush(void) { struct spdk_bdev_io *bdev_io; + struct spdk_bdev_io_wait_entry *entry; - while (!TAILQ_EMPTY(&g_bdev_io_queue)) { - bdev_io = TAILQ_FIRST(&g_bdev_io_queue); - TAILQ_REMOVE(&g_bdev_io_queue, bdev_io, internal.link); - bdev_io->internal.cb(bdev_io, true, bdev_io->internal.caller_ctx); - free(bdev_io); + while (!TAILQ_EMPTY(&g_bdev_io_queue) || !TAILQ_EMPTY(&g_io_wait_queue)) { + while (!TAILQ_EMPTY(&g_bdev_io_queue)) { + bdev_io = TAILQ_FIRST(&g_bdev_io_queue); + TAILQ_REMOVE(&g_bdev_io_queue, bdev_io, internal.link); + bdev_io->internal.cb(bdev_io, true, bdev_io->internal.caller_ctx); + free(bdev_io); + } + + while (!TAILQ_EMPTY(&g_io_wait_queue)) { + entry = TAILQ_FIRST(&g_io_wait_queue); + TAILQ_REMOVE(&g_io_wait_queue, entry, link); + entry->cb_fn(entry->cb_arg); + } } } @@ -205,6 +217,11 @@ _spdk_bdev_io_op(spdk_bdev_io_completion_cb cb, void *cb_arg) { struct spdk_bdev_io *bdev_io; + if (g_bdev_io_pool_full) { + g_bdev_io_pool_full = false; + return -ENOMEM; + } + bdev_io = calloc(1, sizeof(*bdev_io)); SPDK_CU_ASSERT_FATAL(bdev_io != NULL); bdev_io->internal.status = SPDK_BDEV_IO_STATUS_SUCCESS; @@ -256,6 +273,14 @@ spdk_bdev_flush_blocks(struct spdk_bdev_desc *desc, struct spdk_io_channel *ch, return _spdk_bdev_io_op(cb, cb_arg); } +int +spdk_bdev_queue_io_wait(struct spdk_bdev *bdev, struct spdk_io_channel *ch, + struct spdk_bdev_io_wait_entry *entry) +{ + TAILQ_INSERT_TAIL(&g_io_wait_queue, entry, link); + return 0; +} + /* * This test specifically tests a mode select 6 command from the * Windows SCSI compliance test that caused SPDK to crash. @@ -805,7 +830,7 @@ xfer_len_test(void) } static void -xfer_test(void) +_xfer_test(bool bdev_io_pool_full) { struct spdk_bdev bdev; struct spdk_scsi_lun lun; @@ -828,6 +853,7 @@ xfer_test(void) to_be64(&cdb[2], 0); /* LBA */ to_be32(&cdb[10], 1); /* transfer length */ task.transfer_len = 1 * 512; + g_bdev_io_pool_full = bdev_io_pool_full; rc = spdk_bdev_scsi_execute(&task); CU_ASSERT(rc == SPDK_SCSI_TASK_PENDING); CU_ASSERT(task.status == 0xFF); @@ -847,6 +873,7 @@ xfer_test(void) to_be64(&cdb[2], 0); /* LBA */ to_be32(&cdb[10], 1); /* transfer length */ task.transfer_len = 1 * 512; + g_bdev_io_pool_full = bdev_io_pool_full; rc = spdk_bdev_scsi_execute(&task); CU_ASSERT(rc == SPDK_SCSI_TASK_PENDING); CU_ASSERT(task.status == 0xFF); @@ -872,6 +899,7 @@ xfer_test(void) to_be32(&data[32], 3); /* 3 blocks */ spdk_scsi_task_set_data(&task, data, sizeof(data)); task.status = SPDK_SCSI_STATUS_GOOD; + g_bdev_io_pool_full = bdev_io_pool_full; rc = spdk_bdev_scsi_execute(&task); CU_ASSERT(rc == SPDK_SCSI_TASK_PENDING); CU_ASSERT(task.status == SPDK_SCSI_STATUS_GOOD); @@ -890,6 +918,7 @@ xfer_test(void) cdb[0] = 0x91; /* SYNCHRONIZE CACHE (16) */ to_be64(&cdb[2], 0); /* LBA */ to_be32(&cdb[10], 1); /* 1 blocks */ + g_bdev_io_pool_full = bdev_io_pool_full; rc = spdk_bdev_scsi_execute(&task); CU_ASSERT(rc == SPDK_SCSI_TASK_PENDING); CU_ASSERT(task.status == 0xFF); @@ -901,6 +930,13 @@ xfer_test(void) ut_put_task(&task); } +static void +xfer_test(void) +{ + _xfer_test(false); + _xfer_test(true); +} + int main(int argc, char **argv) { @@ -908,6 +944,7 @@ main(int argc, char **argv) unsigned int num_failures; TAILQ_INIT(&g_bdev_io_queue); + TAILQ_INIT(&g_io_wait_queue); if (CU_initialize_registry() != CUE_SUCCESS) { return CU_get_error();