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 <pl@kamp.de>

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)
 
 /*

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

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

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

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

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