lkml.org 
[lkml]   [2011]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH-v2 1/4] target: Fix multi task->task_sg[] chaining logic bug
Acked-by: Kiran Patil <kiran.patil@intel.com>

On 5/10/2011 9:35 PM, Nicholas A. Bellinger wrote:
> From: Nicholas Bellinger<nab@linux-iscsi.org>
>
> This patch fixes a bug in transport_do_task_sg_chain() used by HW target
> mode modules with sg_chain() to provide a single sg_next() walkable memory
> layout for use with pci_map_sg() and friends. This patch addresses an
> issue with mapping multiple small block max_sector tasks across multiple
> struct se_task->task_sg[] mappings for HW target mode operation.
>
> This was causing OOPs with (cmd->t_task->t_tasks_no> 1) I/O traffic for
> HW target drivers using transport_do_task_sg_chain(), and has been tested
> so far with tcm_fc(openfcoe), tcm_qla2xxx, and ib_srpt fabrics with
> t_tasks_no> 1 IBLOCK backends using a smaller max_sectors to trigger the
> original issue.
>
> Reported-by: Kiran Patil<kiran.patil@intel.com>
> Signed-off-by: Nicholas Bellinger<nab@linux-iscsi.org>
> ---
> drivers/target/target_core_transport.c | 26 +++++++++++++++-----------
> 1 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> index 9583b23..fefe10a 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
> @@ -4776,18 +4776,20 @@ void transport_do_task_sg_chain(struct se_cmd *cmd)
> sg_end_cur->page_link&= ~0x02;
>
> sg_chain(sg_head, task_sg_num, sg_head_cur);
> - sg_count += (task->task_sg_num + 1);
> - } else
> sg_count += task->task_sg_num;
> + task_sg_num = (task->task_sg_num + 1);
> + } else {
> + sg_chain(sg_head, task_sg_num, sg_head_cur);
> + sg_count += task->task_sg_num;
> + task_sg_num = task->task_sg_num;
> + }
>
> sg_head = sg_head_cur;
> sg_link = sg_link_cur;
> - task_sg_num = task->task_sg_num;
> continue;
> }
> sg_head = sg_first =&task->task_sg[0];
> sg_link =&task->task_sg[task->task_sg_num];
> - task_sg_num = task->task_sg_num;
> /*
> * Check for single task..
> */
> @@ -4798,9 +4800,12 @@ void transport_do_task_sg_chain(struct se_cmd *cmd)
> */
> sg_end =&task->task_sg[task->task_sg_num - 1];
> sg_end->page_link&= ~0x02;
> - sg_count += (task->task_sg_num + 1);
> - } else
> sg_count += task->task_sg_num;
> + task_sg_num = (task->task_sg_num + 1);
> + } else {
> + sg_count += task->task_sg_num;
> + task_sg_num = task->task_sg_num;
> + }
> }
> /*
> * Setup the starting pointer and total t_tasks_sg_linked_no including
> @@ -4809,21 +4814,20 @@ void transport_do_task_sg_chain(struct se_cmd *cmd)
> T_TASK(cmd)->t_tasks_sg_chained = sg_first;
> T_TASK(cmd)->t_tasks_sg_chained_no = sg_count;
>
> - DEBUG_CMD_M("Setup T_TASK(cmd)->t_tasks_sg_chained: %p and"
> - " t_tasks_sg_chained_no: %u\n", T_TASK(cmd)->t_tasks_sg_chained,
> + DEBUG_CMD_M("Setup cmd: %p T_TASK(cmd)->t_tasks_sg_chained: %p and"
> + " t_tasks_sg_chained_no: %u\n", cmd, T_TASK(cmd)->t_tasks_sg_chained,
> T_TASK(cmd)->t_tasks_sg_chained_no);
>
> for_each_sg(T_TASK(cmd)->t_tasks_sg_chained, sg,
> T_TASK(cmd)->t_tasks_sg_chained_no, i) {
>
> - DEBUG_CMD_M("SG: %p page: %p length: %d offset: %d\n",
> - sg, sg_page(sg), sg->length, sg->offset);
> + DEBUG_CMD_M("SG[%d]: %p page: %p length: %d offset: %d, magic: 0x%08x\n",
> + i, sg, sg_page(sg), sg->length, sg->offset, sg->sg_magic);
> if (sg_is_chain(sg))
> DEBUG_CMD_M("SG: %p sg_is_chain=1\n", sg);
> if (sg_is_last(sg))
> DEBUG_CMD_M("SG: %p sg_is_last=1\n", sg);
> }
> -
> }
> EXPORT_SYMBOL(transport_do_task_sg_chain);
>



\
 
 \ /
  Last update: 2011-05-17 23:29    [W:0.058 / U:0.628 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site