Skip to content

Commit

Permalink
target/rd: fix or rewrite the copy routine
Browse files Browse the repository at this point in the history 
So the code assumes that the sg list is only a array while in reality
loopback SGL memory via scsi_cmnd into target-core may be already
chained.  This patch converts ramdisk code to use sg_miter logic from
scatterlist.h in order to properly support passthrough SGL usage with
transport_generic_map_mem_to_cmd() via loopback.

With this patch the bug goes away. However after umount/mount of the
device my files are gone. So something is still not right. After looking
at it for a while I decided to rewrite the that part of the code and now
things do work for me.

For reference:
- http://article.gmane.org/gmane.linux.scsi.target.devel/595
  the sg_next() conversion
- http://article.gmane.org/gmane.linux.scsi.target.devel/602
  the rewrite of the copy code

(nab: Fix compile warning in rd_MEMCPY)

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: stable@kernel.org
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
  • Loading branch information
Sebastian Andrzej Siewior authored and Nicholas Bellinger committed Dec 6, 2011
1 parent 5bff9e8 commit 65b0c78
Showing 1 changed file with 40 additions and 205 deletions.
245 changes: 40 additions & 205 deletions drivers/target/target_core_rd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -343,235 +343,74 @@ static struct rd_dev_sg_table *rd_get_sg_table(struct rd_dev *rd_dev, u32 page)
return NULL;
}

/* rd_MEMCPY_read():
*
*
*/
static int rd_MEMCPY_read(struct rd_request *req)
static int rd_MEMCPY(struct rd_request *req, u32 read_rd)
{
struct se_task *task = &req->rd_task;
struct rd_dev *dev = req->rd_task.task_se_cmd->se_dev->dev_ptr;
struct rd_dev_sg_table *table;
struct scatterlist *sg_d, *sg_s;
void *dst, *src;
u32 i = 0, j = 0, dst_offset = 0, src_offset = 0;
u32 length, page_end = 0, table_sg_end;
struct scatterlist *rd_sg;
struct sg_mapping_iter m;
u32 rd_offset = req->rd_offset;
u32 src_len;

table = rd_get_sg_table(dev, req->rd_page);
if (!table)
return -EINVAL;

table_sg_end = (table->page_end_offset - req->rd_page);
sg_d = task->task_sg;
sg_s = &table->sg_table[req->rd_page - table->page_start_offset];
rd_sg = &table->sg_table[req->rd_page - table->page_start_offset];

pr_debug("RD[%u]: Read LBA: %llu, Size: %u Page: %u, Offset:"
" %u\n", dev->rd_dev_id, task->task_lba, req->rd_size,
req->rd_page, req->rd_offset);

src_offset = rd_offset;
pr_debug("RD[%u]: %s LBA: %llu, Size: %u Page: %u, Offset: %u\n",
dev->rd_dev_id, read_rd ? "Read" : "Write",
task->task_lba, req->rd_size, req->rd_page,
rd_offset);

src_len = PAGE_SIZE - rd_offset;
sg_miter_start(&m, task->task_sg, task->task_sg_nents,
read_rd ? SG_MITER_TO_SG : SG_MITER_FROM_SG);
while (req->rd_size) {
if ((sg_d[i].length - dst_offset) <
(sg_s[j].length - src_offset)) {
length = (sg_d[i].length - dst_offset);

pr_debug("Step 1 - sg_d[%d]: %p length: %d"
" offset: %u sg_s[%d].length: %u\n", i,
&sg_d[i], sg_d[i].length, sg_d[i].offset, j,
sg_s[j].length);
pr_debug("Step 1 - length: %u dst_offset: %u"
" src_offset: %u\n", length, dst_offset,
src_offset);

if (length > req->rd_size)
length = req->rd_size;

dst = sg_virt(&sg_d[i++]) + dst_offset;
BUG_ON(!dst);

src = sg_virt(&sg_s[j]) + src_offset;
BUG_ON(!src);

dst_offset = 0;
src_offset = length;
page_end = 0;
} else {
length = (sg_s[j].length - src_offset);

pr_debug("Step 2 - sg_d[%d]: %p length: %d"
" offset: %u sg_s[%d].length: %u\n", i,
&sg_d[i], sg_d[i].length, sg_d[i].offset,
j, sg_s[j].length);
pr_debug("Step 2 - length: %u dst_offset: %u"
" src_offset: %u\n", length, dst_offset,
src_offset);

if (length > req->rd_size)
length = req->rd_size;

dst = sg_virt(&sg_d[i]) + dst_offset;
BUG_ON(!dst);

if (sg_d[i].length == length) {
i++;
dst_offset = 0;
} else
dst_offset = length;

src = sg_virt(&sg_s[j++]) + src_offset;
BUG_ON(!src);

src_offset = 0;
page_end = 1;
}
u32 len;
void *rd_addr;

memcpy(dst, src, length);
sg_miter_next(&m);
len = min((u32)m.length, src_len);
m.consumed = len;

pr_debug("page: %u, remaining size: %u, length: %u,"
" i: %u, j: %u\n", req->rd_page,
(req->rd_size - length), length, i, j);
rd_addr = sg_virt(rd_sg) + rd_offset;

req->rd_size -= length;
if (!req->rd_size)
return 0;
if (read_rd)
memcpy(m.addr, rd_addr, len);
else
memcpy(rd_addr, m.addr, len);

if (!page_end)
req->rd_size -= len;
if (!req->rd_size)
continue;

if (++req->rd_page <= table->page_end_offset) {
pr_debug("page: %u in same page table\n",
req->rd_page);
src_len -= len;
if (src_len) {
rd_offset += len;
continue;
}

pr_debug("getting new page table for page: %u\n",
req->rd_page);

table = rd_get_sg_table(dev, req->rd_page);
if (!table)
return -EINVAL;

sg_s = &table->sg_table[j = 0];
}

return 0;
}

/* rd_MEMCPY_write():
*
*
*/
static int rd_MEMCPY_write(struct rd_request *req)
{
struct se_task *task = &req->rd_task;
struct rd_dev *dev = req->rd_task.task_se_cmd->se_dev->dev_ptr;
struct rd_dev_sg_table *table;
struct scatterlist *sg_d, *sg_s;
void *dst, *src;
u32 i = 0, j = 0, dst_offset = 0, src_offset = 0;
u32 length, page_end = 0, table_sg_end;
u32 rd_offset = req->rd_offset;

table = rd_get_sg_table(dev, req->rd_page);
if (!table)
return -EINVAL;

table_sg_end = (table->page_end_offset - req->rd_page);
sg_d = &table->sg_table[req->rd_page - table->page_start_offset];
sg_s = task->task_sg;

pr_debug("RD[%d] Write LBA: %llu, Size: %u, Page: %u,"
" Offset: %u\n", dev->rd_dev_id, task->task_lba, req->rd_size,
req->rd_page, req->rd_offset);

dst_offset = rd_offset;

while (req->rd_size) {
if ((sg_s[i].length - src_offset) <
(sg_d[j].length - dst_offset)) {
length = (sg_s[i].length - src_offset);

pr_debug("Step 1 - sg_s[%d]: %p length: %d"
" offset: %d sg_d[%d].length: %u\n", i,
&sg_s[i], sg_s[i].length, sg_s[i].offset,
j, sg_d[j].length);
pr_debug("Step 1 - length: %u src_offset: %u"
" dst_offset: %u\n", length, src_offset,
dst_offset);

if (length > req->rd_size)
length = req->rd_size;

src = sg_virt(&sg_s[i++]) + src_offset;
BUG_ON(!src);

dst = sg_virt(&sg_d[j]) + dst_offset;
BUG_ON(!dst);

src_offset = 0;
dst_offset = length;
page_end = 0;
} else {
length = (sg_d[j].length - dst_offset);

pr_debug("Step 2 - sg_s[%d]: %p length: %d"
" offset: %d sg_d[%d].length: %u\n", i,
&sg_s[i], sg_s[i].length, sg_s[i].offset,
j, sg_d[j].length);
pr_debug("Step 2 - length: %u src_offset: %u"
" dst_offset: %u\n", length, src_offset,
dst_offset);

if (length > req->rd_size)
length = req->rd_size;

src = sg_virt(&sg_s[i]) + src_offset;
BUG_ON(!src);

if (sg_s[i].length == length) {
i++;
src_offset = 0;
} else
src_offset = length;

dst = sg_virt(&sg_d[j++]) + dst_offset;
BUG_ON(!dst);

dst_offset = 0;
page_end = 1;
}

memcpy(dst, src, length);

pr_debug("page: %u, remaining size: %u, length: %u,"
" i: %u, j: %u\n", req->rd_page,
(req->rd_size - length), length, i, j);

req->rd_size -= length;
if (!req->rd_size)
return 0;

if (!page_end)
continue;

if (++req->rd_page <= table->page_end_offset) {
pr_debug("page: %u in same page table\n",
req->rd_page);
/* rd page completed, next one please */
req->rd_page++;
rd_offset = 0;
src_len = PAGE_SIZE;
if (req->rd_page <= table->page_end_offset) {
rd_sg++;
continue;
}

pr_debug("getting new page table for page: %u\n",
req->rd_page);

table = rd_get_sg_table(dev, req->rd_page);
if (!table)
if (!table) {
sg_miter_stop(&m);
return -EINVAL;
}

sg_d = &table->sg_table[j = 0];
/* since we increment, the first sg entry is correct */
rd_sg = table->sg_table;
}

sg_miter_stop(&m);
return 0;
}

Expand All @@ -591,11 +430,7 @@ static int rd_MEMCPY_do_task(struct se_task *task)
req->rd_page = tmp;
req->rd_size = task->task_size;

if (task->task_data_direction == DMA_FROM_DEVICE)
ret = rd_MEMCPY_read(req);
else
ret = rd_MEMCPY_write(req);

ret = rd_MEMCPY(req, task->task_data_direction == DMA_FROM_DEVICE);
if (ret != 0)
return ret;

Expand Down

0 comments on commit 65b0c78

Please sign in to comment.