From 56e12b00711b40d1e2ff45d4147193f45fdd9af0 Mon Sep 17 00:00:00 2001 From: Shuhei Matsumoto Date: Fri, 15 Feb 2019 09:03:59 +0900 Subject: [PATCH] scsi: Use spdk_bdev_readv_blocks instead of spdk_bdev_readv This is in a effort to consolidate SCSI read and write I/O for the upcoming transparent DIF support. Previously conversion of bytes and blocks are done both in SCSI layer and BDEV layer. After the patch series, conversion is consolidated into SCSI layer. About conversion from bytes to blocks, we don't expose bdev API spdk_bdev_bytes_to_blocks and but create private helper function _bytes_to_blocks because we will use not block size but data block size when we support transparent DIF feature. Change-Id: I37169c673479c92e027e2507a0e54a1e414b43e1 Signed-off-by: Shuhei Matsumoto Reviewed-on: https://review.gerrithub.io/c/444778 Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Darek Stojaczyk --- lib/scsi/scsi_bdev.c | 73 +++++++++++++------ test/unit/lib/scsi/scsi_bdev.c/scsi_bdev_ut.c | 33 +++++++-- 2 files changed, 76 insertions(+), 30 deletions(-) diff --git a/lib/scsi/scsi_bdev.c b/lib/scsi/scsi_bdev.c index 9765d478f..ed1a20267 100644 --- a/lib/scsi/scsi_bdev.c +++ b/lib/scsi/scsi_bdev.c @@ -1315,36 +1315,38 @@ spdk_bdev_scsi_queue_io(struct spdk_scsi_task *task, spdk_bdev_io_wait_cb cb_fn, } } +static uint64_t +_bytes_to_blocks(uint32_t block_size, uint64_t offset_bytes, uint64_t *offset_blocks, + uint64_t num_bytes, uint64_t *num_blocks) +{ + uint8_t shift_cnt; + + /* Avoid expensive div operations if possible. These spdk_u32 functions are very cheap. */ + if (spdk_likely(spdk_u32_is_pow2(block_size))) { + shift_cnt = spdk_u32log2(block_size); + *offset_blocks = offset_bytes >> shift_cnt; + *num_blocks = num_bytes >> shift_cnt; + return (offset_bytes - (*offset_blocks << shift_cnt)) | + (num_bytes - (*num_blocks << shift_cnt)); + } else { + *offset_blocks = offset_bytes / block_size; + *num_blocks = num_bytes / block_size; + return (offset_bytes % block_size) | (num_bytes % block_size); + } +} + 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, uint64_t lba) { - uint64_t blen; - uint64_t offset; - uint64_t nbytes; + uint64_t offset_blocks, num_blocks; int rc; - blen = spdk_bdev_get_block_size(bdev); - - lba += (task->offset / blen); - offset = lba * blen; - nbytes = task->length; - - SPDK_DEBUGLOG(SPDK_LOG_SCSI, - "Read: lba=%"PRIu64", len=%"PRIu64"\n", - lba, (uint64_t)task->length / blen); - - 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"); + if (_bytes_to_blocks(spdk_bdev_get_block_size(bdev), task->offset, &offset_blocks, + task->length, &num_blocks) != 0) { + SPDK_ERRLOG("task's offset %" PRIu64 " or length %" PRIu32 " is not block multiple\n", + task->offset, task->length); spdk_scsi_task_set_status(task, SPDK_SCSI_STATUS_CHECK_CONDITION, SPDK_SCSI_SENSE_NO_SENSE, SPDK_SCSI_ASC_NO_ADDITIONAL_SENSE, @@ -1352,7 +1354,30 @@ spdk_bdev_scsi_read(struct spdk_bdev *bdev, struct spdk_bdev_desc *bdev_desc, return SPDK_SCSI_TASK_COMPLETE; } - task->data_transferred = nbytes; + offset_blocks += lba; + + SPDK_DEBUGLOG(SPDK_LOG_SCSI, + "Read: lba=%"PRIu64", len=%"PRIu64"\n", + offset_blocks, num_blocks); + + rc = spdk_bdev_readv_blocks(bdev_desc, bdev_ch, task->iovs, task->iovcnt, + offset_blocks, num_blocks, + 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_blocks() failed\n"); + 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; + } + + task->data_transferred = task->length; return SPDK_SCSI_TASK_PENDING; } 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 918151ba1..dbabb2600 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 @@ -194,9 +194,10 @@ _spdk_bdev_io_op(spdk_bdev_io_completion_cb cb, void *cb_arg) } int -spdk_bdev_readv(struct spdk_bdev_desc *desc, struct spdk_io_channel *ch, - struct iovec *iov, int iovcnt, uint64_t offset, uint64_t nbytes, - spdk_bdev_io_completion_cb cb, void *cb_arg) +spdk_bdev_readv_blocks(struct spdk_bdev_desc *desc, struct spdk_io_channel *ch, + struct iovec *iov, int iovcnt, + uint64_t offset_blocks, uint64_t num_blocks, + spdk_bdev_io_completion_cb cb, void *cb_arg) { return _spdk_bdev_io_op(cb, cb_arg); } @@ -639,7 +640,7 @@ task_complete_test(void) static void lba_range_test(void) { - struct spdk_bdev bdev; + struct spdk_bdev bdev = { .blocklen = 512 }; struct spdk_scsi_lun lun; struct spdk_scsi_task task; uint8_t cdb[16]; @@ -663,6 +664,8 @@ lba_range_test(void) to_be64(&cdb[2], 0); /* LBA */ to_be32(&cdb[10], 1); /* transfer length */ task.transfer_len = 1 * 512; + task.offset = 0; + task.length = 1 * 512; rc = spdk_bdev_scsi_execute(&task); CU_ASSERT(rc == SPDK_SCSI_TASK_PENDING); CU_ASSERT(task.status == 0xFF); @@ -676,6 +679,8 @@ lba_range_test(void) to_be64(&cdb[2], 4); /* LBA */ to_be32(&cdb[10], 1); /* transfer length */ task.transfer_len = 1 * 512; + task.offset = 0; + task.length = 1 * 512; rc = spdk_bdev_scsi_execute(&task); CU_ASSERT(rc == SPDK_SCSI_TASK_COMPLETE); CU_ASSERT(task.status == SPDK_SCSI_STATUS_CHECK_CONDITION); @@ -687,6 +692,8 @@ lba_range_test(void) to_be32(&cdb[10], 4); /* transfer length */ task.transfer_len = 4 * 512; task.status = 0xFF; + task.offset = 0; + task.length = 1 * 512; rc = spdk_bdev_scsi_execute(&task); CU_ASSERT(rc == SPDK_SCSI_TASK_PENDING); CU_ASSERT(task.status == 0xFF); @@ -700,6 +707,8 @@ lba_range_test(void) to_be64(&cdb[2], 0); /* LBA */ to_be32(&cdb[10], 5); /* transfer length */ task.transfer_len = 5 * 512; + task.offset = 0; + task.length = 1 * 512; rc = spdk_bdev_scsi_execute(&task); CU_ASSERT(rc == SPDK_SCSI_TASK_COMPLETE); CU_ASSERT(task.status == SPDK_SCSI_STATUS_CHECK_CONDITION); @@ -712,7 +721,7 @@ lba_range_test(void) static void xfer_len_test(void) { - struct spdk_bdev bdev; + struct spdk_bdev bdev = { .blocklen = 512 }; struct spdk_scsi_lun lun; struct spdk_scsi_task task; uint8_t cdb[16]; @@ -736,6 +745,8 @@ xfer_len_test(void) to_be64(&cdb[2], 0); /* LBA */ to_be32(&cdb[10], 1); /* transfer length */ task.transfer_len = 1 * 512; + task.offset = 0; + task.length = 1 * 512; rc = spdk_bdev_scsi_execute(&task); CU_ASSERT(rc == SPDK_SCSI_TASK_PENDING); CU_ASSERT(task.status == 0xFF); @@ -750,6 +761,8 @@ xfer_len_test(void) to_be32(&cdb[10], SPDK_WORK_BLOCK_SIZE / 512); /* transfer length */ task.transfer_len = SPDK_WORK_BLOCK_SIZE; task.status = 0xFF; + task.offset = 0; + task.length = 1 * 512; rc = spdk_bdev_scsi_execute(&task); CU_ASSERT(rc == SPDK_SCSI_TASK_PENDING); CU_ASSERT(task.status == 0xFF); @@ -763,6 +776,8 @@ xfer_len_test(void) to_be64(&cdb[2], 0); /* LBA */ to_be32(&cdb[10], SPDK_WORK_BLOCK_SIZE / 512 + 1); /* transfer length */ task.transfer_len = SPDK_WORK_BLOCK_SIZE + 512; + task.offset = 0; + task.length = 1 * 512; rc = spdk_bdev_scsi_execute(&task); CU_ASSERT(rc == SPDK_SCSI_TASK_COMPLETE); CU_ASSERT(task.status == SPDK_SCSI_STATUS_CHECK_CONDITION); @@ -774,6 +789,8 @@ xfer_len_test(void) to_be64(&cdb[2], 0); /* LBA */ to_be32(&cdb[10], 0); /* transfer length */ task.transfer_len = 0; + task.offset = 0; + task.length = 0; rc = spdk_bdev_scsi_execute(&task); CU_ASSERT(rc == SPDK_SCSI_TASK_COMPLETE); CU_ASSERT(task.status == SPDK_SCSI_STATUS_GOOD); @@ -784,6 +801,8 @@ xfer_len_test(void) to_be64(&cdb[2], g_test_bdev_num_blocks); /* LBA */ to_be32(&cdb[10], 0); /* transfer length */ task.transfer_len = 0; + task.offset = 0; + task.length = 0; rc = spdk_bdev_scsi_execute(&task); CU_ASSERT(rc == SPDK_SCSI_TASK_COMPLETE); CU_ASSERT(task.status == SPDK_SCSI_STATUS_CHECK_CONDITION); @@ -796,7 +815,7 @@ xfer_len_test(void) static void _xfer_test(bool bdev_io_pool_full) { - struct spdk_bdev bdev; + struct spdk_bdev bdev = { .blocklen = 512 }; struct spdk_scsi_lun lun; struct spdk_scsi_task task; uint8_t cdb[16]; @@ -819,6 +838,8 @@ _xfer_test(bool bdev_io_pool_full) to_be64(&cdb[2], 0); /* LBA */ to_be32(&cdb[10], 1); /* transfer length */ task.transfer_len = 1 * 512; + task.offset = 0; + task.length = 1 * 512; g_bdev_io_pool_full = bdev_io_pool_full; rc = spdk_bdev_scsi_execute(&task); CU_ASSERT(rc == SPDK_SCSI_TASK_PENDING);