[ofa-general] [PATCH][REPOST] drivers/infiniband/ulp/srpt: Fix target data corruption
davem at systemfabricworks.com
davem at systemfabricworks.com
Fri Jan 11 14:44:18 PST 2008
This is an updated version of [PATCH] drivers/infiniband/ulp/srpt: Fix
target data corruption
It was pointed out to me that the code to round up to a power of 2 was
not as clean as it should be, plus I extracted two unrelated patches and
submitted them separately.
=====================================================================
Change the local buffer allocator to use a spin-lock protected linked
list instead of an array of atomic_t used/free variables. The atomic_t
code was open to a multi-thread race between test and set. This has
been observed with the result that the same data buffer was used for
more than one SCSI operation, either writing the wrong data to the disk
or sending the wrong data to the initiator.
Signed-off-by: Robert Pearson <rpearson at systemfabricworks.com>
Signed-off-by: David A. McMillen <davem at systemfabricworks.com>
---
ib_srpt.c | 95 +++++++++++++++++++++++++++++++++++++++---------------------
1 files changed, 62 insertions(+), 33 deletions(-)
diff --git a/ib_srpt.c b/ib_srpt.c
index 1fb9d56..97088ea 100644
--- a/ib_srpt.c
+++ b/ib_srpt.c
@@ -56,10 +56,14 @@ MODULE_LICENSE("Dual BSD/GPL");
struct mem_elem {
struct page *page;
+ struct mem_elem *next;
+ int ndx;
u32 len;
- atomic_t used;
};
+static spinlock_t mempool_lock;
+static struct mem_elem *mempool_freelist;
+
struct srpt_thread {
spinlock_t thread_lock;
struct list_head thread_ioctx_list;
@@ -70,7 +74,7 @@ static u64 mellanox_ioc_guid = 0;
static struct list_head srpt_devices;
static int mem_size = 32768;
static int mem_elements = 4096;
-static atomic_t mem_avail;
+static int mem_avail;
static int cur_pos = 1;
static struct mem_elem *srpt_mempool = NULL;
static int thread = 1;
@@ -98,28 +102,29 @@ static struct ib_client srpt_client = {
.remove = srpt_remove_one
};
+static int round_up_power_of_2(int val)
+{
+ while (val & (val - 1))
+ val += val & ~(val - 1);
+ return val;
+}
+
static struct mem_elem *srpt_get_mem_elem(int *index)
{
- int i, end_pos;
+ struct mem_elem *elem;
*index = 0;
- end_pos = mem_elements;
- for (i = cur_pos; i <= end_pos; ++i) {
- if (i == mem_elements) {
- end_pos = cur_pos;
- i = 1;
- }
- if (atomic_read(&srpt_mempool[i].used) == 0) {
- atomic_inc(&srpt_mempool[i].used);
- smp_mb__after_atomic_inc();
- *index = i;
- cur_pos = i + 1;
- atomic_dec(&mem_avail);
- return &srpt_mempool[i];
- }
+ spin_lock(&mempool_lock);
+ elem = mempool_freelist;
+ if (elem) {
+ mempool_freelist = elem->next;
+ mem_avail--;
+ *index = elem->ndx;
}
- return NULL;
+ spin_unlock(&mempool_lock);
+
+ return elem;
}
static int srpt_free_mem_elem(int index)
@@ -127,23 +132,45 @@ static int srpt_free_mem_elem(int index)
if (!index || index >= mem_elements)
return -EINVAL;
- atomic_dec(&srpt_mempool[index].used);
- smp_mb__after_atomic_dec();
- atomic_inc(&mem_avail);
+ spin_lock(&mempool_lock);
+ srpt_mempool[index].next = mempool_freelist;
+ mempool_freelist = &srpt_mempool[index];
+ mem_avail++;
+ spin_unlock(&mempool_lock);
return 0;
}
static int srpt_mempool_create(void)
{
- int i, order;
+ int i, order, array_size;
- order = get_order(mem_elements * sizeof(struct mem_elem));
+ if (mem_elements <= 0)
+ return -ENOMEM;
+ array_size = mem_elements * sizeof(struct mem_elem);
+ array_size = round_up_power_of_2(array_size);
+ /* array_size now first power of 2 >= actual array size */
+
+ if (mem_size < PAGE_SIZE) {
+ printk(KERN_ERR PFX "mem_size parameter changed from"
+ " %d to %lu (PAGE_SIZE)\n", mem_size, PAGE_SIZE);
+ mem_size = PAGE_SIZE;
+ }
+ i = round_up_power_of_2(mem_size);
+ if (i != mem_size) {
+ printk(KERN_ERR PFX "mem_size parameter rounded up from"
+ " %d to %d\n", mem_size, i);
+ mem_size = i;
+ }
+ /* mem_size now also a power of 2 >= PAGE_SIZE */
+
+ order = get_order(array_size);
+ spin_lock_init(&mempool_lock);
srpt_mempool = (struct mem_elem *) __get_free_pages(GFP_KERNEL, order);
if (!srpt_mempool)
return -ENOMEM;
+ order = get_order(mem_size);
for (i = 1; i < mem_elements; ++i) {
- order = get_order(mem_size);
srpt_mempool[i].page = alloc_pages(GFP_KERNEL, order);
if (!srpt_mempool[i].page) {
printk(KERN_ERR PFX
@@ -151,18 +178,19 @@ static int srpt_mempool_create(void)
mem_size, i);
goto free_mem;
}
- atomic_set(&srpt_mempool[i].used, 0);
+ srpt_mempool[i].ndx = i;
+ srpt_mempool[i].next = mempool_freelist;
+ mempool_freelist = &srpt_mempool[i];
}
- atomic_set(&mem_avail, mem_elements);
+ mem_avail = mem_elements;
return 0;
free_mem:
while (i > 1)
__free_pages(srpt_mempool[--i].page, order);
- free_pages((unsigned long) srpt_mempool,
- get_order(mem_elements * sizeof(struct mem_elem)));
+ free_pages((unsigned long) srpt_mempool, get_order(array_size));
return -ENOMEM;
}
@@ -176,7 +204,8 @@ static void srpt_mempool_destroy(void)
__free_pages(srpt_mempool[i].page, order);
free_pages((unsigned long) srpt_mempool,
- get_order(mem_elements * sizeof(struct mem_elem)));
+ get_order(round_up_power_of_2(
+ mem_elements * sizeof(struct mem_elem))));
}
static void srpt_event_handler(struct ib_event_handler *handler,
@@ -2116,8 +2145,8 @@ static int srpt_alloc_data_buf(struct scst_cmd *scmnd)
tsize = scst_cmd_get_bufflen(scmnd);
nrdma = tsize / mem_size + ioctx->n_rbuf;
- if (nrdma > atomic_read(&mem_avail)) {
- printk(KERN_ERR PFX "!!ALERT mem_avail= %d\n", atomic_read(&mem_avail));
+ if (nrdma > mem_avail) {
+ printk(KERN_ERR PFX "!!ALERT mem_avail= %d\n", mem_avail);
goto out;
}
@@ -2161,7 +2190,7 @@ static int srpt_alloc_data_buf(struct scst_cmd *scmnd)
release_mem:
printk(KERN_WARNING PFX "ALERT! alloc_mem mem_avail= %d n_mem_elem=%d\n",
- atomic_read(&mem_avail), i);
+ mem_avail, i);
riu = ioctx->rdma_ius;
while (i)
@@ -2295,7 +2324,7 @@ static CLASS_DEVICE_ATTR(login_info, S_IRUGO, show_login_info, NULL);
static ssize_t show_mem_info(struct class_device *class_dev, char *buf)
{
return sprintf(buf, "mem_avail= %d mem_elements= %d mem_size= %d\n",
- atomic_read(&mem_avail), mem_elements, mem_size);
+ mem_avail, mem_elements, mem_size);
}
static CLASS_DEVICE_ATTR(mem_info, S_IRUGO, show_mem_info, NULL);
More information about the general
mailing list