From 55f76cfb0cdba9a52357117495ecccba6c8cffcc Mon Sep 17 00:00:00 2001 From: Peter Lieven Date: Wed, 21 Nov 2012 17:02:59 +0100 Subject: [PATCH] SCSI add support for iovectors If an application passes buffers to libiscsi for reading they are currently added to a linked list one by one. This leads to a malloc for each buffer object plus O(n) walks trough the list to add the buffer to then end of the list. Additionally the buffer read routine takes up to O(n) iterations to find the right buffer for a request. This patch introduces an scsi_iovector struct to pass buffers to an scsi task. Adding a new buffer is in O(1) and finding the right buffer to also. Malloc requirements are in O(log(n)). Additionally the scsi_iovector struct is itended to be binary compatible to an QEMUIOVector allowing to pass this structure directly to the library. Initial tests have been made booting an Ubuntu LTS 12.04.1 Desktop server up to the login prompt. The following observations have been made with regards to scsi_malloc calls: original implementation: ~11.500 mallocs using iovector instead of list: ~ 7.500 mallocs passing the iovector directly: 0 mallocs To enable this feature in qemu for testing the following patch might be used: diff --git a/block/iscsi.c b/block/iscsi.c index a6a819d..2809c15 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -390,11 +390,16 @@ iscsi_aio_readv(BlockDriverState *bs, int64_t sector_num, return NULL; } +#if defined(LIBISCSI_FEATURE_IOVECTOR) + assert(sizeof(struct QEMUIOVector) == sizeof(struct scsi_iovector)); + scsi_iovector_assign(acb->task, (struct scsi_iovector*) acb->qiov); +#else for (i = 0; i < acb->qiov->niov; i++) { scsi_task_add_data_in_buffer(acb->task, acb->qiov->iov[i].iov_len, acb->qiov->iov[i].iov_base); } +#endif iscsi_set_events(iscsilun); --- Signed-off-by: Peter Lieven --- include/iscsi.h | 3 + include/scsi-lowlevel.h | 20 ++++++- lib/init.c | 2 + lib/libiscsi.def | 1 + lib/libiscsi.syms | 1 + lib/scsi-lowlevel.c | 129 ++++++++++++++++++++++++++-------------- 6 files changed, 112 insertions(+), 44 deletions(-) diff --git a/include/iscsi.h b/include/iscsi.h index 3aadafd..b893ce7 100644 --- a/include/iscsi.h +++ b/include/iscsi.h @@ -33,6 +33,9 @@ extern "C" { struct iscsi_context; struct sockaddr; +/* FEATURES */ +#define LIBISCSI_FEATURE_IOVECTOR (1) + #define MAX_STRING_SIZE (255) /* diff --git a/include/scsi-lowlevel.h b/include/scsi-lowlevel.h index 0e1a78e..7c173c7 100644 --- a/include/scsi-lowlevel.h +++ b/include/scsi-lowlevel.h @@ -197,6 +197,20 @@ enum scsi_residual { SCSI_RESIDUAL_OVERFLOW }; +#if !defined(_SYS_UIO_H) && !defined(CONFIG_IOVEC) +struct iovec { + void *iov_base; + size_t iov_len; +}; +#endif + +struct scsi_iovector { + struct iovec *iov; + int niov; + int nalloc; + size_t size; +}; + struct scsi_task { int status; @@ -217,7 +231,9 @@ struct scsi_task { uint32_t cmdsn; uint32_t lun; - struct scsi_data_buffer *in_buffers; + size_t buffers_offset; + int buffers_consumed; + struct scsi_iovector *buffers; }; /* This function will free a scsi task structure. @@ -707,6 +723,8 @@ EXTERN struct scsi_task *scsi_cdb_report_supported_opcodes(int report_timeouts, void *scsi_malloc(struct scsi_task *task, size_t size); +EXTERN void scsi_iovector_assign(struct scsi_task *task, struct scsi_iovector *iov); + #ifdef __cplusplus } #endif diff --git a/lib/init.c b/lib/init.c index 86ae172..521f4a2 100644 --- a/lib/init.c +++ b/lib/init.c @@ -32,6 +32,8 @@ #include "iscsi.h" #include "iscsi-private.h" #include "slist.h" +#include "scsi-lowlevel.h" + inline void* iscsi_malloc(struct iscsi_context *iscsi, size_t size) { void * ptr = malloc(size); diff --git a/lib/libiscsi.def b/lib/libiscsi.def index cbc374b..d24f487 100644 --- a/lib/libiscsi.def +++ b/lib/libiscsi.def @@ -176,4 +176,5 @@ scsi_sense_ascq_str scsi_sense_key_str scsi_set_task_private_ptr scsi_task_add_data_in_buffer +scsi_iovector_assign scsi_version_to_str diff --git a/lib/libiscsi.syms b/lib/libiscsi.syms index 19408a9..48b53d2 100644 --- a/lib/libiscsi.syms +++ b/lib/libiscsi.syms @@ -174,4 +174,5 @@ scsi_sense_ascq_str scsi_sense_key_str scsi_set_task_private_ptr scsi_task_add_data_in_buffer +scsi_iovector_assign scsi_version_to_str diff --git a/lib/scsi-lowlevel.c b/lib/scsi-lowlevel.c index 7a9c9b3..3cb2eb2 100644 --- a/lib/scsi-lowlevel.c +++ b/lib/scsi-lowlevel.c @@ -2533,58 +2533,101 @@ scsi_get_task_private_ptr(struct scsi_task *task) return task->ptr; } +void +scsi_iovector_assign(struct scsi_task *task, struct scsi_iovector *iov) +{ + task->buffers = iov; +} +#define IOVECTOR_INITAL_ALLOC (16) -struct scsi_data_buffer { - struct scsi_data_buffer *next; - uint32_t len; - unsigned char *data; -}; +int +scsi_iovector_add(struct scsi_task *task, int len, unsigned char *buf) +{ + if (len < 0) { + return -1; + } + + if (task->buffers == NULL) { + task->buffers = scsi_malloc(task, sizeof(struct scsi_iovector) + + IOVECTOR_INITAL_ALLOC*sizeof(struct iovec) + 15); + if (task->buffers == NULL) { + return -1; + } + + memset(task->buffers, 0, sizeof(struct scsi_iovector)); + /* align by 16 bytes */ + task->buffers->iov = (struct iovec *) (((uintptr_t)task->buffers + + sizeof(struct scsi_iovector) + 15)&~0xf); + task->buffers->nalloc = IOVECTOR_INITAL_ALLOC; + } + + /* iovec allocation is too small */ + if (task->buffers->nalloc < task->buffers->niov + 1) { + struct iovec *old_iov = task->buffers->iov; + task->buffers->iov = scsi_malloc(task, 2 * task->buffers->nalloc * sizeof(struct iovec)); + if (task->buffers->iov == NULL) { + return -1; + } + if (old_iov != NULL) { + memcpy(task->buffers->iov, old_iov, task->buffers->niov * sizeof(struct iovec)); + } + task->buffers->nalloc <<= 1; + } + + task->buffers->iov[task->buffers->niov].iov_len = len; + task->buffers->iov[task->buffers->niov].iov_base = buf; + task->buffers->niov++; + task->buffers->size += len; + + return 0; +} + +unsigned char * +scsi_iovector_get_buffer(struct scsi_task *task, uint32_t pos, ssize_t *count) +{ + if (task->buffers == NULL) { + return NULL; + } + + if (pos == 0 && count == NULL) return task->buffers->iov[0].iov_base; + + if (task->buffers->niov <= task->buffers_consumed) { + /* someone issued a read but did not provide enough user buffers for all the data. + * maybe someone tried to read just 512 bytes off a MMC device? + */ + return NULL; + } + + pos-= task->buffers_offset; + + struct iovec *iov = &task->buffers->iov[task->buffers_consumed]; + + while (pos >= iov->iov_len) { + task->buffers_offset += iov->iov_len; + task->buffers_consumed++; + pos -= iov->iov_len; + if (task->buffers->niov <= task->buffers_consumed) { + return NULL; + } + iov = &task->buffers->iov[task->buffers_consumed]; + } + + if (count && *count >= (ssize_t)(iov->iov_len - pos)) { + *count = iov->iov_len - pos; + } + + return (unsigned char *) iov->iov_base + pos; +} int scsi_task_add_data_in_buffer(struct scsi_task *task, int len, unsigned char *buf) { - struct scsi_data_buffer *data_buf; - - if (len < 0) { - return -1; - } - data_buf = scsi_malloc(task, sizeof(struct scsi_data_buffer)); - if (data_buf == NULL) { - return -1; - } - - data_buf->len = len; - data_buf->data = buf; - - SLIST_ADD_END(&task->in_buffers, data_buf); - return 0; + return scsi_iovector_add(task, len, buf); } unsigned char * scsi_task_get_data_in_buffer(struct scsi_task *task, uint32_t pos, ssize_t *count) { - struct scsi_data_buffer *sdb; - - sdb = task->in_buffers; - if (sdb == NULL) { - return NULL; - } - - while (pos >= sdb->len) { - pos -= sdb->len; - sdb = sdb->next; - if (sdb == NULL) { - /* someone issued a read but did not provide enough user buffers for all the data. - * maybe someone tried to read just 512 bytes off a MMC device? - */ - return NULL; - } - } - - if (count && *count > (ssize_t)(sdb->len - pos)) { - *count = sdb->len - pos; - } - - return &sdb->data[pos]; + return scsi_iovector_get_buffer(task, pos, count); }