From patchwork Thu Sep 10 22:16:08 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Himanshu Madhani X-Patchwork-Id: 7156091 Return-Path: X-Original-To: patchwork-linux-scsi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 41FDABEEC1 for ; Thu, 10 Sep 2015 22:16:50 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 17C082086D for ; Thu, 10 Sep 2015 22:16:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 9F84920868 for ; Thu, 10 Sep 2015 22:16:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751282AbbIJWQd (ORCPT ); Thu, 10 Sep 2015 18:16:33 -0400 Received: from mx0b-0016ce01.pphosted.com ([67.231.156.153]:23322 "EHLO mx0b-0016ce01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751157AbbIJWQN (ORCPT ); Thu, 10 Sep 2015 18:16:13 -0400 Received: from pps.filterd (m0085408.ppops.net [127.0.0.1]) by mx0b-0016ce01.pphosted.com (8.15.0.59/8.15.0.59) with SMTP id t8AMGCts005943 for ; Thu, 10 Sep 2015 15:16:12 -0700 Received: from avcashub1.qlogic.com (avcashub2.qlogic.com [198.70.193.116]) by mx0b-0016ce01.pphosted.com with ESMTP id 1wqycnfx70-1 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NOT) for ; Thu, 10 Sep 2015 15:16:12 -0700 Received: from AVMB3.qlogic.org ([fe80::4cd9:35c8:c701:e42d]) by avcashub2.qlogic.org ([::1]) with mapi id 14.03.0235.001; Thu, 10 Sep 2015 15:16:09 -0700 From: Himanshu Madhani To: "Nicholas A. Bellinger" , Arun Easi CC: "Nicholas A. Bellinger" , target-devel , linux-scsi , Craig Watson , Roland Dreier , Giridhar Malavali , Andrew Vasquez , Christoph Hellwig , Hannes Reinecke , "Martin K. Petersen" Subject: Re: [PATCH] target/qla2xxx: Honor max_data_sg_nents I/O transfer limit Thread-Topic: [PATCH] target/qla2xxx: Honor max_data_sg_nents I/O transfer limit Thread-Index: AQHQ1aVNB1MHDA92l0aRnTUAsppqvJ4UjfQAgAF4y4CAH3YugIABBFeA Date: Thu, 10 Sep 2015 22:16:08 +0000 Message-ID: References: <1439455550-2626-1-git-send-email-nab@daterainc.com> <1440112633.14304.23.camel@haakon3.risingtidesystems.com> <1441842255.6836.13.camel@haakon3.risingtidesystems.com> In-Reply-To: <1441842255.6836.13.camel@haakon3.risingtidesystems.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: user-agent: Microsoft-MacOutlook/14.3.9.131030 x-originating-ip: [10.1.4.10] disclaimer: bypass MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=nai engine=5700 definitions=7920 signatures=670636 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1507310000 definitions=main-1509100299 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Nic, Sorry about the delay in response. I have tested with RHEL 6.5 and noticed that IO were not able to complete with 16M and 32M read/write. IO¹s were getting error due to Mid-layer underflow With initiator running upstream kernel version 4.2 as well I was seeing error with Mid-layer reporting under-run. I made change in the driver to report DID_OK to Mid-layer with residual count and I was able to see IO completed without any error. Basically, overriding driver¹s logic of failing an I/O, when residual makes it an error with scsi_cmnd->underflow set. In reality, though, with scsi_cmnd->underflow being set, Linux Qlogic initiator will always see this as an error. Maybe this is useful in other OSes. We have verified both read and write data on FC trace and it looks good. Here¹s diff that was done in qla2xxx driver to report response to mid-layer as DID_OK. # git diff } else if (lscsi_status != SAM_STAT_TASK_SET_FULL && (END) Please go ahead and add patch to mainline kernel. Thanks, -Himanshu On 9/9/15, 4:44 PM, "Nicholas A. Bellinger" wrote: >Hi Arun & Co, > >On Thu, 2015-08-20 at 16:17 -0700, Nicholas A. Bellinger wrote: >> On Wed, 2015-08-19 at 17:48 -0700, Arun Easi wrote: >> > Hi nab, >> > >> > On Thu, 13 Aug 2015, 1:45am -0700, Nicholas A. Bellinger wrote: >> > >> > > From: Nicholas Bellinger >> > > >> > > Hi Arun, Roland & Co, >> > > >> > > Based upon the feedback from last week, here is a proper >> > > patch for target-core to honor a fabric provided SGL limit >> > > using residual count plus underflow response bit. >> > > >> > > Everything appears to be working as expected with tcm-loop >> > > LUNs with basic I/O and sg_raw to test CDB underflow, but >> > > still needs to be verified on real qla2xxx hardware over >> > > the next days for v4.2.0 code. >> > > >> > > Please review + test. >> > >> > Changes look good. I could not test the changes, though. I have >>requested >> > internally to test this patch. Himanshu (copied) will get back with >>the >> > test results (Thanks Himanshu). >> > >> >> Thanks for the update. Btw, this patch has been pushed to >> refs/heads/queue, atop the other recent v4.2-rc fixes here: >> >> >>https://git.kernel.org/cgit/linux/kernel/git/nab/target-pending.git/commi >>t/?h=queue >> >> Just an FYI for Himanshu, for testing against Linux hosts that do honor >> EVPD block-limits, you'll want to include the following small hack to >> report a larger block-limits value so the new residual handling can >> actually get invoked. >> >> diff --git a/drivers/target/target_core_spc.c >>b/drivers/target/target_core_spc.c >> index 01421e9..f02767b 100644 >> --- a/drivers/target/target_core_spc.c >> +++ b/drivers/target/target_core_spc.c >> @@ -523,6 +523,7 @@ spc_emulate_evpd_b0(struct se_cmd *cmd, unsigned >>char *buf) >> if (cmd->se_tfo->max_data_sg_nents) { >> mtl = (cmd->se_tfo->max_data_sg_nents * PAGE_SIZE) / >> dev->dev_attrib.block_size; >> + mtl *= 2; >> } >> put_unaligned_be32(min_not_zero(mtl, >>dev->dev_attrib.hw_max_sectors), &buf[8]); >> >> Also for testing, I'd recommend lowering tcm_qla2xxx's max_data_sg_nents >> to something arbitrarily small to exercise the new code. >> >> Craig, have you had a chance to test the new logic on your setup with >> hosts that don't honor EVPD block-limits..? >> > >I've not seen any testing feedback on this patch over the last weeks >from Himanshu or Craig. > >So barring any specific objections, I think it's safe enough to include >in the upcoming v4.3-rc1 PULL request, minus the stable CC'. > >Himanshu + Craig, please let me know how your testing is going. > >Based upon your feedback we can address any regressions post merge, and >subsequently get the patch (plus any other bugfixes) picked up for >stable code. > >Thank you, > >--nab diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c index ccf6a7f..fc7b6a2 100644 --- a/drivers/scsi/qla2xxx/qla_isr.c +++ b/drivers/scsi/qla2xxx/qla_isr.c @@ -2257,7 +2257,7 @@ qla2x00_status_entry(scsi_qla_host_t *vha, struct rsp_que *rsp, void *pkt) "detected (0x%x of 0x%x bytes).\n", resid, scsi_bufflen(cp)); - res = DID_ERROR << 16; + res = DID_OK << 16; break; }