Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932370Ab1EQV1l (ORCPT ); Tue, 17 May 2011 17:27:41 -0400 Received: from mga01.intel.com ([192.55.52.88]:22999 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932330Ab1EQV1k (ORCPT ); Tue, 17 May 2011 17:27:40 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.65,227,1304319600"; d="scan'208";a="3022185" Message-ID: <4DD2E846.6090009@intel.com> Date: Tue, 17 May 2011 14:27:34 -0700 From: Kiran Patil User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.17) Gecko/20110414 Thunderbird/3.1.10 MIME-Version: 1.0 To: "Nicholas A. Bellinger" CC: linux-kernel , linux-scsi , James Bottomley , Linus Torvalds , Christoph Hellwig Subject: Re: [PATCH-v2 1/4] target: Fix multi task->task_sg[] chaining logic bug References: <1305088535-27486-1-git-send-email-nab@linux-iscsi.org> <1305088535-27486-2-git-send-email-nab@linux-iscsi.org> In-Reply-To: <1305088535-27486-2-git-send-email-nab@linux-iscsi.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3909 Lines: 99 Acked-by: Kiran Patil On 5/10/2011 9:35 PM, Nicholas A. Bellinger wrote: > From: Nicholas Bellinger > > 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 > Signed-off-by: Nicholas Bellinger > --- > 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); > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/