From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:38724 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752378AbdECS4B (ORCPT ); Wed, 3 May 2017 14:56:01 -0400 Received: from pps.filterd (m0098413.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v43Irc3a069567 for ; Wed, 3 May 2017 14:56:00 -0400 Received: from e33.co.us.ibm.com (e33.co.us.ibm.com [32.97.110.151]) by mx0b-001b2d01.pphosted.com with ESMTP id 2a7jp9p22f-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Wed, 03 May 2017 14:56:00 -0400 Received: from localhost by e33.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 3 May 2017 12:55:59 -0600 Subject: Re: [PATCH v2] ibmvscsis: Do not send aborted task response To: "Nicholas A. Bellinger" References: <1493751261-81647-1-git-send-email-bryantly@linux.vnet.ibm.com> <1493783006.23202.87.camel@haakon3.risingtidesystems.com> <5c2536d3-7ff1-1e17-91c0-0be6a8befbfa@linux.vnet.ibm.com> Cc: Bart.VanAssche@sandisk.com, martin.petersen@oracle.com, turtle.in.the.kernel@gmail.com, seroyer@linux.vnet.ibm.com, linux-scsi@vger.kernel.org, target-devel@vger.kernel.org, stable@vger.kernel.org From: "Bryant G. Ly" Date: Wed, 3 May 2017 13:55:54 -0500 MIME-Version: 1.0 In-Reply-To: <5c2536d3-7ff1-1e17-91c0-0be6a8befbfa@linux.vnet.ibm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Message-Id: <7d900f15-4590-5b09-0f48-8e8be862665e@linux.vnet.ibm.com> Sender: stable-owner@vger.kernel.org List-ID: On 5/3/17 11:38 AM, Bryant G. Ly wrote: > Hi Nick, > > On 5/2/17 10:43 PM, Nicholas A. Bellinger wrote: > >> On Tue, 2017-05-02 at 13:54 -0500, Bryant G. Ly wrote: >>> The driver is sending a response to the actual scsi op that was >>> aborted by an abort task TM, while LIO is sending a response to >>> the abort task TM. >>> >>> ibmvscsis_tgt does not send the response to the client until >>> release_cmd time. The reason for this was because if we did it >>> at queue_status time, then the client would be free to reuse the >>> tag for that command, but we're still using the tag until the >>> command is released at release_cmd time, so we chose to delay >>> sending the response until then. That then caused this issue, because >>> release_cmd is always called, even if queue_status is not. >>> >>> SCSI spec says that the initiator that sends the abort task >>> TM NEVER gets a response to the aborted op and with the current >>> code it will send a response. Thus this fix will remove that response >>> if the TAS bit is set. >>> >>> Cc: # v4.8+ >>> Signed-off-by: Bryant G. Ly >>> Reviewed-by: Tyrel Datwyler >>> --- >>> drivers/scsi/ibmvscsi_tgt/ibmvscsi_tgt.c | 66 >>> ++++++++++++++++++++++---------- >>> 1 file changed, 45 insertions(+), 21 deletions(-) >> Applied, with a small update to the last sentence of the commit log wrt >> to 'if ABORTED && !TAS bit is set'. >> >> Thanks Bryant + Tyrel. >> > So I have been running tests on this patch extensively and it seems > like after running 2 hrs of consecutive aborts it fails again with the > same problem of driver sending a response to the actual scsi op. > > It looks like when LIO processes the abort and if > (!__target_check_io_state(se_cmd, se_sess, 0)) then rsp gets set as > TMR_TASK_DOES_NOT_EXIST. > > With that response the CMD_T_ABORTED state is not set, so then the > ibmvscsis driver still sends that duplicate response. > > I think the best solution would be in driver when queue_tm_rsp gets > called and when the driver builds the response to check for the > TMR_TASK_DOES_NOT_EXIST and set our own flag. > Then we would also check for that flag, so it would be: > > if ((cmd->se_cmd.transport_state & CMD_T_ABORTED && > !(cmd->se_cmd.transport_state & CMD_T_TAS)) || cmd->flags & > CMD_ABORTED) { > > This will ensure in the CMD_T_ABORTED w/o CMD_T_TAS scenarios are > handled and the ABORT_TASK: Sending TMR_TASK_DOES_NOT_EXIST case. > > -Bryant > > Hi Nick, You can ignore the email about the patch being broken. The script that was used for testing isn't right. So the patch is working as intended now, where on an abort the client does not receive an extra response to the actual scsi op. Thanks, Bryant Ly