From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.6 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,USER_AGENT_SANE_2 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BCA3DC433DF for ; Wed, 22 Jul 2020 10:07:47 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 99D8220729 for ; Wed, 22 Jul 2020 10:07:47 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=mediatek.com header.i=@mediatek.com header.b="mRHGFzuK" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730931AbgGVKHp (ORCPT ); Wed, 22 Jul 2020 06:07:45 -0400 Received: from mailgw01.mediatek.com ([210.61.82.183]:53379 "EHLO mailgw01.mediatek.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1726153AbgGVKHo (ORCPT ); Wed, 22 Jul 2020 06:07:44 -0400 X-UUID: 173e8a484fba49b2a7d3d912af0f09fb-20200722 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=mediatek.com; s=dk; h=Content-Transfer-Encoding:MIME-Version:Content-Type:References:In-Reply-To:Date:CC:To:From:Subject:Message-ID; bh=hpw2HAjj+VZcqiXE9JCKu3K69kU6aLIL+g4N7qmAH6w=; b=mRHGFzuKX9VZw96ff4c5f+9dfqcbnhKnypVjEdvx0T5KV1k4oP6RFw4kVncj2pVoQda4IKsJ4OtFALdk1eJSyo3oHZuseqxJn9ceDA0E8IyOvEUUdbneUyiDDTuvxZkOJm8pOuqHExboSGjjCc+KLfEUz+uKS4Or1vsw62KCgvU=; X-UUID: 173e8a484fba49b2a7d3d912af0f09fb-20200722 Received: from mtkexhb02.mediatek.inc [(172.21.101.103)] by mailgw01.mediatek.com (envelope-from ) (Cellopoint E-mail Firewall v4.1.10 Build 0809 with TLS) with ESMTP id 365314642; Wed, 22 Jul 2020 18:07:39 +0800 Received: from mtkcas08.mediatek.inc (172.21.101.126) by mtkmbs02n1.mediatek.inc (172.21.101.77) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Wed, 22 Jul 2020 18:07:36 +0800 Received: from [172.21.77.33] (172.21.77.33) by mtkcas08.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Wed, 22 Jul 2020 18:07:36 +0800 Message-ID: <1595412457.27178.36.camel@mtkswgap22> Subject: Re: [PATCH v3] scsi: ufs: Cleanup completed request without interrupt notification From: Stanley Chu To: Bart Van Assche CC: Avri Altman , "linux-scsi@vger.kernel.org" , "martin.petersen@oracle.com" , "alim.akhtar@samsung.com" , "jejb@linux.ibm.com" , "beanhuo@micron.com" , "asutoshd@codeaurora.org" , "cang@codeaurora.org" , "matthias.bgg@gmail.com" , "linux-mediatek@lists.infradead.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "kuohong.wang@mediatek.com" , "peter.wang@mediatek.com" , "chun-hung.wu@mediatek.com" , "andy.teng@mediatek.com" , "chaotian.jing@mediatek.com" , "cc.chou@mediatek.com" Date: Wed, 22 Jul 2020 18:07:37 +0800 In-Reply-To: <912623e8-5915-8380-f39a-fac7b5868a6d@acm.org> References: <20200706132113.21096-1-stanley.chu@mediatek.com> <3d509c4b-d66d-2a4a-5fbd-a50a0610ad31@acm.org> <1594607245.22878.8.camel@mtkswgap22> <912623e8-5915-8380-f39a-fac7b5868a6d@acm.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.2.3-0ubuntu6 MIME-Version: 1.0 X-MTK: N Content-Transfer-Encoding: base64 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org SGkgQmFydCwgQXZyaSwNCg0KT24gVHVlLCAyMDIwLTA3LTE0IGF0IDIxOjAwIC0wNzAwLCBCYXJ0 IFZhbiBBc3NjaGUgd3JvdGU6DQo+IE9uIDIwMjAtMDctMTMgMDE6MTAsIEF2cmkgQWx0bWFuIHdy b3RlOg0KPiA+IEFydGlmaWNpYWxseSBpbmplY3RpbmcgZXJyb3JzIGlzIGEgdmVyeSBjb21tb24g dmFsaWRhdGlvbiBtZWNoYW5pc20sDQo+ID4gUHJvdmlkZWQgdGhhdCB5b3UgYXJlIG5vdCBicmVh a2luZyBhbnl0aGluZyBvZiB0aGUgdXBwZXItbGF5ZXJzLA0KPiA+IFdoaWNoIEkgZG9uJ3QgdGhp bmsgeW91IGFyZSBkb2luZy4NCj4gDQoNCkFzIHRoZSBjb25jZXJucyBvZiBiZWxvdyBxdWVzdGlv bnMsDQoNCiJzY3NpIHRpbWVvdXQgaXMgMzBzZWMgLSBkbyB5b3UgZXhwZWN0IGFuIGludGVycnVw dCB0byBhcnJpdmUgYWZ0ZXINCnRoYXQ/Ig0KDQpBY3R1YWxseSBpbiBteSB0ZXN0IHNjZW5hcmlv LCB0aGUgZmxvdyB3b3JrcyB3ZWxsIHdpdGhvdXQgcmUtY2hlY2tpbmcNCiJvdXRzdGFuZGluZ19y ZXFzIiBpbiAiY2xlYW51cCIgc2VjdGlvbiBpbiB1ZnNoY2RfYWJvcnQoKSwgc28gSSB3b3VsZA0K cmVtb3ZlIHRoaXMgY2hlY2tpbmcgZmlyc3QgYW5kIHJlc2VuZCB0aGlzIGZpeCAod2l0aCByZWZp bmVkIGNvbW1pdA0KbWVzc2FnZSBhY2NvcmRpbmcgdG8gYmxrLW1xLCBub3QgbGVnYWN5IGJsayku IFBsZWFzZSBsZXQgbWUga25vdyBpZiB5b3UNCmhhdmUgYW55IHN1Z2dlc3Rpb25zLg0KDQo+IEhp IEF2cmksDQo+IA0KPiBNeSBjb25jZXJuIGlzIHRoYXQgdGhlIGNvZGUgdGhhdCBpcyBiZWluZyBh ZGRlZCBpbiB0aGUgYWJvcnQgaGFuZGxlcg0KPiBzb29uZXIgb3IgbGF0ZXIgd2lsbCBldm9sdmUg aW50byBhIGR1cGxpY2F0ZSBvZiB0aGUgcmVndWxhciBjb21wbGV0aW9uDQo+IHBhdGguIFdvdWxk bid0IGl0IGJlIGJldHRlciB0byBwb2xsIGZvciBjb21wbGV0aW9ucyBmcm9tIHRoZSB0aW1lb3V0 DQo+IGhhbmRsZXIgYnkgY2FsbGluZyB1ZnNoY2RfdHJhbnNmZXJfcmVxX2NvbXBsKCkgaW5zdGVh ZCBvZiBkdXBsaWNhdGluZw0KPiB0aGF0IGZ1bmN0aW9uPw0KPiANCg0KVGhlIGR1cGxpY2F0ZWQg Y2FsbHMgb2YgY2xlYW51cCBqb2Igd291bGQgYmUgYXMgYmVsb3csDQoNCnNjc2lfZG1hX3VubWFw KGNtZCk7DQpoYmEtPmxyYlt0YWddLmNtZCA9IE5VTEw7DQp1ZnNoY2Rfb3V0c3RhbmRpbmdfcmVx X2NsZWFyKGhiYSwgdGFnKTsNCg0KQXMgeW91ciBzdWdnZXN0aW9ucywgYWJvdmUgY2FsbHMgY291 bGQgYmUgcmUtZmFjdG9yZWQgYnV0IHRoZSB0aGlyZCBjYWxsDQppbiBfX3Vmc2hjZF90cmFuc2Zl cl9yZXFfY29tcGwoKSB3b3VsZCBiZSBtb3JlIGVmZmljaWVudCBieQ0KDQpoYmEtPm91dHN0YW5k aW5nX3JlcXMgXj0gY29tcGxldGVkX3JlcXM7DQoNCmZvciBhbGwgaGFuZGxlZCByZXF1ZXN0cyBp biBpbnRlcnJ1cHQgaGFuZGxlci4NCg0KDQpIZXJlIHdlIGNvdWxkIG5vdCBkaXJlY3RseSB1c2Ug InVmc2hjZF90cmFuc2Zlcl9yZXFfY29tcGwoKSIgb3IgaXRzDQppbm5lciBmdW5jdGlvbiAiX191 ZnNoY2RfdHJhbnNmZXJfcmVxX2NvbXBsKCkiIHNpbmNlIGF0IGxlYXN0DQpzY3NpX2RvbmUoKSBp cyBub3QgcmVxdWlyZWQgaW4gdWZzaGNkX2Fib3J0KCkgYmVjYXVzZSB0aGUgY29tcGxldGlvbg0K ZmxvdyB3aWxsIGJlIGhhbmRsZWQgYnkgU0NTSSBlcnJvciBoYW5kbGVyLCBub3QgdWZzaGNkX2Fi b3J0KCkgaXRzZWxmLg0KDQo+ID4+PiBJbiBzZWN0aW9uIDcuMi4zIG9mIHRoZSBVRlMgc3BlY2lm aWNhdGlvbiBJIGZvdW5kIHRoZSBmb2xsb3dpbmcgYWJvdXQgaG93DQo+ID4+PiB0byBwcm9jZXNz IHJlcXVlc3QgY29tcGxldGlvbnM6ICJTb2Z0d2FyZSBkZXRlcm1pbmVzIGlmIG5ldyBUUnMgaGF2 ZQ0KPiA+Pj4gY29tcGxldGVkIHNpbmNlIHN0ZXAgIzIsIGJ5IHJlcGVhdGluZyBvbmUgb2YgdGhl IHR3byBtZXRob2RzIGRlc2NyaWJlZCBpbg0KPiA+Pj4gc3RlcCAjMi4gSWYgbmV3IFRScyBoYXZl IGNvbXBsZXRlZCwgc29mdHdhcmUgcmVwZWF0cyB0aGUgc2VxdWVuY2UgZnJvbQ0KPiA+Pj4gc3Rl cCAjMy4iIElzIHN1Y2ggYSBsb29wIHBlcmhhcHMgbWlzc2luZyBmcm9tIHRoZSBMaW51eCBVRlMg ZHJpdmVyPw0KPiA+DQo+ID4gQ291bGQgbm90IGZpbmQgdGhhdCBjaXRhdGlvbi4NCj4gPiBXaGF0 IHZlcnNpb24gb2YgdGhlIHNwZWMgYXJlIHlvdSB1c2luZz8NCj4gDQo+IFRoYXQgcXVvdGUgY29t ZXMgZnJvbSB0aGUgZm9sbG93aW5nIGRvY3VtZW50OiAiVW5pdmVyc2FsIEZsYXNoIFN0b3JhZ2UN Cj4gSG9zdCBDb250cm9sbGVyIEludGVyZmFjZSAoVUZTSENJKTsgVmVyc2lvbiAyLjE7IEpFU0Qy MjNDOyAoUmV2aXNpb24gb2YNCj4gSkVTRDIyM0IsIFNlcHRlbWJlciAyMDEzKTsgTUFSQ0ggMjAx NiIuDQoNCkFib3ZlIGRlc2NyaXB0aW9uIGhhcyBhbHJlYWR5IGJlIGltcGxlbWVudGVkIGluIHVm c2hjZF9pbnRyKCkgYW5kDQp1ZnNoY2RfdHJhbnNmZXJfcmVxX2NvbXBsKCkuIEJ1dCB0aGlzIGxv b3AgY2Fubm90IHNhdmUgIm1pc3NpbmcNCmludGVycnVwdCIganVzdCBsaWtlIHRoaXMgaW5qZWN0 ZWQgZXJyb3IgY2FzZS4NCg0KVGhhbmtzLA0KU3RhbmxleSBDaHUNCg0K From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,USER_AGENT_SANE_2 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 13EC2C433E3 for ; Wed, 22 Jul 2020 10:07:59 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D29B0206D7 for ; Wed, 22 Jul 2020 10:07:58 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="KyrCKG4y"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=mediatek.com header.i=@mediatek.com header.b="mRHGFzuK" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D29B0206D7 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=mediatek.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To:Date:To:From: Subject:Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=r1mymk99rzYjz4PMVe47WrtZJe1Yh5Ca6T01+oO4KxM=; b=KyrCKG4ysamAMpBCl8vIvjJUT SRPKgjyIkJqIDWzSJNwwLq2JoZoTvaZt92hf1MzORrkb89cup5utrKKEq3hiBOGI4f8VKysxdAYFO /shUoyksJcL3uKpkfX1n6U843ieskx5WKqAHWI49Cq0PSulvhkff8Iq6+8ONNgaEWSuXvdsODsqyv VbJjHs1WQzlvFcRvsKNT+wOeFYC1ee6YqeAo0pFPC6SAAyv82sux6vf8R91HtpTA+1Ck8ZUTHSAJk vIGN1lkFKzOqpG6JQb6WE5B6CZT4V4v0UeF5AsgM5VCQAj8dAh7ChoyfzjMQWTXLze5k6zx06mVbf uKRO5xMPA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jyBfA-0003vE-Pu; Wed, 22 Jul 2020 10:07:48 +0000 Received: from mailgw02.mediatek.com ([216.200.240.185]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jyBf6-0003uT-9c; Wed, 22 Jul 2020 10:07:45 +0000 X-UUID: ff93e59bf6cc464fb30e288ad042ce6d-20200722 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=mediatek.com; s=dk; h=Content-Transfer-Encoding:MIME-Version:Content-Type:References:In-Reply-To:Date:CC:To:From:Subject:Message-ID; bh=hpw2HAjj+VZcqiXE9JCKu3K69kU6aLIL+g4N7qmAH6w=; b=mRHGFzuKX9VZw96ff4c5f+9dfqcbnhKnypVjEdvx0T5KV1k4oP6RFw4kVncj2pVoQda4IKsJ4OtFALdk1eJSyo3oHZuseqxJn9ceDA0E8IyOvEUUdbneUyiDDTuvxZkOJm8pOuqHExboSGjjCc+KLfEUz+uKS4Or1vsw62KCgvU=; X-UUID: ff93e59bf6cc464fb30e288ad042ce6d-20200722 Received: from mtkcas67.mediatek.inc [(172.29.193.45)] by mailgw02.mediatek.com (envelope-from ) (musrelay.mediatek.com ESMTP with TLS) with ESMTP id 1776867086; Wed, 22 Jul 2020 02:07:37 -0800 Received: from MTKMBS02N1.mediatek.inc (172.21.101.77) by MTKMBS62N2.mediatek.inc (172.29.193.42) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Wed, 22 Jul 2020 03:07:37 -0700 Received: from mtkcas08.mediatek.inc (172.21.101.126) by mtkmbs02n1.mediatek.inc (172.21.101.77) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Wed, 22 Jul 2020 18:07:36 +0800 Received: from [172.21.77.33] (172.21.77.33) by mtkcas08.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Wed, 22 Jul 2020 18:07:36 +0800 Message-ID: <1595412457.27178.36.camel@mtkswgap22> Subject: Re: [PATCH v3] scsi: ufs: Cleanup completed request without interrupt notification From: Stanley Chu To: Bart Van Assche Date: Wed, 22 Jul 2020 18:07:37 +0800 In-Reply-To: <912623e8-5915-8380-f39a-fac7b5868a6d@acm.org> References: <20200706132113.21096-1-stanley.chu@mediatek.com> <3d509c4b-d66d-2a4a-5fbd-a50a0610ad31@acm.org> <1594607245.22878.8.camel@mtkswgap22> <912623e8-5915-8380-f39a-fac7b5868a6d@acm.org> X-Mailer: Evolution 3.2.3-0ubuntu6 MIME-Version: 1.0 X-MTK: N X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200722_060744_627568_C91E8F87 X-CRM114-Status: GOOD ( 19.14 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "linux-scsi@vger.kernel.org" , "martin.petersen@oracle.com" , "andy.teng@mediatek.com" , "jejb@linux.ibm.com" , "chun-hung.wu@mediatek.com" , "kuohong.wang@mediatek.com" , "linux-kernel@vger.kernel.org" , Avri Altman , "cang@codeaurora.org" , "linux-mediatek@lists.infradead.org" , "peter.wang@mediatek.com" , "alim.akhtar@samsung.com" , "matthias.bgg@gmail.com" , "asutoshd@codeaurora.org" , "chaotian.jing@mediatek.com" , "cc.chou@mediatek.com" , "linux-arm-kernel@lists.infradead.org" , "beanhuo@micron.com" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org Hi Bart, Avri, On Tue, 2020-07-14 at 21:00 -0700, Bart Van Assche wrote: > On 2020-07-13 01:10, Avri Altman wrote: > > Artificially injecting errors is a very common validation mechanism, > > Provided that you are not breaking anything of the upper-layers, > > Which I don't think you are doing. > As the concerns of below questions, "scsi timeout is 30sec - do you expect an interrupt to arrive after that?" Actually in my test scenario, the flow works well without re-checking "outstanding_reqs" in "cleanup" section in ufshcd_abort(), so I would remove this checking first and resend this fix (with refined commit message according to blk-mq, not legacy blk). Please let me know if you have any suggestions. > Hi Avri, > > My concern is that the code that is being added in the abort handler > sooner or later will evolve into a duplicate of the regular completion > path. Wouldn't it be better to poll for completions from the timeout > handler by calling ufshcd_transfer_req_compl() instead of duplicating > that function? > The duplicated calls of cleanup job would be as below, scsi_dma_unmap(cmd); hba->lrb[tag].cmd = NULL; ufshcd_outstanding_req_clear(hba, tag); As your suggestions, above calls could be re-factored but the third call in __ufshcd_transfer_req_compl() would be more efficient by hba->outstanding_reqs ^= completed_reqs; for all handled requests in interrupt handler. Here we could not directly use "ufshcd_transfer_req_compl()" or its inner function "__ufshcd_transfer_req_compl()" since at least scsi_done() is not required in ufshcd_abort() because the completion flow will be handled by SCSI error handler, not ufshcd_abort() itself. > >>> In section 7.2.3 of the UFS specification I found the following about how > >>> to process request completions: "Software determines if new TRs have > >>> completed since step #2, by repeating one of the two methods described in > >>> step #2. If new TRs have completed, software repeats the sequence from > >>> step #3." Is such a loop perhaps missing from the Linux UFS driver? > > > > Could not find that citation. > > What version of the spec are you using? > > That quote comes from the following document: "Universal Flash Storage > Host Controller Interface (UFSHCI); Version 2.1; JESD223C; (Revision of > JESD223B, September 2013); MARCH 2016". Above description has already be implemented in ufshcd_intr() and ufshcd_transfer_req_compl(). But this loop cannot save "missing interrupt" just like this injected error case. Thanks, Stanley Chu _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,USER_AGENT_SANE_2 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 655C0C433E1 for ; Wed, 22 Jul 2020 10:09:20 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2F8AD20729 for ; Wed, 22 Jul 2020 10:09:20 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="x7uDbINw"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=mediatek.com header.i=@mediatek.com header.b="mRHGFzuK" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2F8AD20729 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=mediatek.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To:Date:To:From: Subject:Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=FEMxypCSLyXT7Yz+zEd9zYGNXSSSS9jYrMLR/QHqutU=; b=x7uDbINwCNG6+qZk0Qi1K/CpK q/bNYQLHbhc8t3jADrOPc/u1VvTfgKK42ZNZJaejWHte5OYk0IV5vZxlXj8ZQ3wZtkPGYzEVNhaNB a9a/03vBPWHNW1W1Wi7EnlW0r4gZQKa+QRt6Vr8QpdVtHDuxPKj73j0LvYj8Ts5auAMA213YCYoyD T6SEXxz8OGrDdcpYD9UG3nIjk2qFatGpY3vgS1YnCsZmk9UiglXhyUQIkvXToNc92UPRyK1L411BT NGGl6MHTxj2IoJm2WZ58FMlvL3Ig7FS3MJjR3Hq79U79vrdi5+f0tjPvoScFL7VijG2FVscmBnO4y afz8V5GEg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jyBf9-0003uz-Ci; Wed, 22 Jul 2020 10:07:47 +0000 Received: from mailgw02.mediatek.com ([216.200.240.185]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jyBf6-0003uT-9c; Wed, 22 Jul 2020 10:07:45 +0000 X-UUID: ff93e59bf6cc464fb30e288ad042ce6d-20200722 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=mediatek.com; s=dk; h=Content-Transfer-Encoding:MIME-Version:Content-Type:References:In-Reply-To:Date:CC:To:From:Subject:Message-ID; bh=hpw2HAjj+VZcqiXE9JCKu3K69kU6aLIL+g4N7qmAH6w=; b=mRHGFzuKX9VZw96ff4c5f+9dfqcbnhKnypVjEdvx0T5KV1k4oP6RFw4kVncj2pVoQda4IKsJ4OtFALdk1eJSyo3oHZuseqxJn9ceDA0E8IyOvEUUdbneUyiDDTuvxZkOJm8pOuqHExboSGjjCc+KLfEUz+uKS4Or1vsw62KCgvU=; X-UUID: ff93e59bf6cc464fb30e288ad042ce6d-20200722 Received: from mtkcas67.mediatek.inc [(172.29.193.45)] by mailgw02.mediatek.com (envelope-from ) (musrelay.mediatek.com ESMTP with TLS) with ESMTP id 1776867086; Wed, 22 Jul 2020 02:07:37 -0800 Received: from MTKMBS02N1.mediatek.inc (172.21.101.77) by MTKMBS62N2.mediatek.inc (172.29.193.42) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Wed, 22 Jul 2020 03:07:37 -0700 Received: from mtkcas08.mediatek.inc (172.21.101.126) by mtkmbs02n1.mediatek.inc (172.21.101.77) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Wed, 22 Jul 2020 18:07:36 +0800 Received: from [172.21.77.33] (172.21.77.33) by mtkcas08.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Wed, 22 Jul 2020 18:07:36 +0800 Message-ID: <1595412457.27178.36.camel@mtkswgap22> Subject: Re: [PATCH v3] scsi: ufs: Cleanup completed request without interrupt notification From: Stanley Chu To: Bart Van Assche Date: Wed, 22 Jul 2020 18:07:37 +0800 In-Reply-To: <912623e8-5915-8380-f39a-fac7b5868a6d@acm.org> References: <20200706132113.21096-1-stanley.chu@mediatek.com> <3d509c4b-d66d-2a4a-5fbd-a50a0610ad31@acm.org> <1594607245.22878.8.camel@mtkswgap22> <912623e8-5915-8380-f39a-fac7b5868a6d@acm.org> X-Mailer: Evolution 3.2.3-0ubuntu6 MIME-Version: 1.0 X-MTK: N X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200722_060744_627568_C91E8F87 X-CRM114-Status: GOOD ( 19.14 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "linux-scsi@vger.kernel.org" , "martin.petersen@oracle.com" , "andy.teng@mediatek.com" , "jejb@linux.ibm.com" , "chun-hung.wu@mediatek.com" , "kuohong.wang@mediatek.com" , "linux-kernel@vger.kernel.org" , Avri Altman , "cang@codeaurora.org" , "linux-mediatek@lists.infradead.org" , "peter.wang@mediatek.com" , "alim.akhtar@samsung.com" , "matthias.bgg@gmail.com" , "asutoshd@codeaurora.org" , "chaotian.jing@mediatek.com" , "cc.chou@mediatek.com" , "linux-arm-kernel@lists.infradead.org" , "beanhuo@micron.com" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Bart, Avri, On Tue, 2020-07-14 at 21:00 -0700, Bart Van Assche wrote: > On 2020-07-13 01:10, Avri Altman wrote: > > Artificially injecting errors is a very common validation mechanism, > > Provided that you are not breaking anything of the upper-layers, > > Which I don't think you are doing. > As the concerns of below questions, "scsi timeout is 30sec - do you expect an interrupt to arrive after that?" Actually in my test scenario, the flow works well without re-checking "outstanding_reqs" in "cleanup" section in ufshcd_abort(), so I would remove this checking first and resend this fix (with refined commit message according to blk-mq, not legacy blk). Please let me know if you have any suggestions. > Hi Avri, > > My concern is that the code that is being added in the abort handler > sooner or later will evolve into a duplicate of the regular completion > path. Wouldn't it be better to poll for completions from the timeout > handler by calling ufshcd_transfer_req_compl() instead of duplicating > that function? > The duplicated calls of cleanup job would be as below, scsi_dma_unmap(cmd); hba->lrb[tag].cmd = NULL; ufshcd_outstanding_req_clear(hba, tag); As your suggestions, above calls could be re-factored but the third call in __ufshcd_transfer_req_compl() would be more efficient by hba->outstanding_reqs ^= completed_reqs; for all handled requests in interrupt handler. Here we could not directly use "ufshcd_transfer_req_compl()" or its inner function "__ufshcd_transfer_req_compl()" since at least scsi_done() is not required in ufshcd_abort() because the completion flow will be handled by SCSI error handler, not ufshcd_abort() itself. > >>> In section 7.2.3 of the UFS specification I found the following about how > >>> to process request completions: "Software determines if new TRs have > >>> completed since step #2, by repeating one of the two methods described in > >>> step #2. If new TRs have completed, software repeats the sequence from > >>> step #3." Is such a loop perhaps missing from the Linux UFS driver? > > > > Could not find that citation. > > What version of the spec are you using? > > That quote comes from the following document: "Universal Flash Storage > Host Controller Interface (UFSHCI); Version 2.1; JESD223C; (Revision of > JESD223B, September 2013); MARCH 2016". Above description has already be implemented in ufshcd_intr() and ufshcd_transfer_req_compl(). But this loop cannot save "missing interrupt" just like this injected error case. Thanks, Stanley Chu _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel