From patchwork Thu Apr 20 22:27:28 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Bart Van Assche X-Patchwork-Id: 9691453 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 0B761601D4 for ; Thu, 20 Apr 2017 22:27:36 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E64E21FE6A for ; Thu, 20 Apr 2017 22:27:35 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D9C8627D0E; Thu, 20 Apr 2017 22:27:35 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E98091FE6A for ; Thu, 20 Apr 2017 22:27:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S947980AbdDTW1e (ORCPT ); Thu, 20 Apr 2017 18:27:34 -0400 Received: from esa5.hgst.iphmx.com ([216.71.153.144]:27068 "EHLO esa5.hgst.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S947977AbdDTW1d (ORCPT ); Thu, 20 Apr 2017 18:27:33 -0400 X-IronPort-AV: E=Sophos;i="5.37,227,1488816000"; d="scan'208,223";a="11976024" Received: from mail-sn1nam02lp0020.outbound.protection.outlook.com (HELO NAM02-SN1-obe.outbound.protection.outlook.com) ([216.32.180.20]) by ob1.hgst.iphmx.com with ESMTP; 21 Apr 2017 06:27:30 +0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sharedspace.onmicrosoft.com; s=selector1-sharedspace-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=TeNrYwWR5OypsdurHS/geIndweFDjqa8t3NbL4aGc2s=; b=ecpkxjOnUPBmYCIGWn+Q9LpcFOXUYXE0k5eRr3A1aGRekJds/Z2vLcJfF2gs7eW2XVeign/WN7IzceJC94wlwfvYgWr9CTeloKPhjP7BVZ52Q20xIha89jDzA4UvBF+hP2KFiph5JH+Hw1BPfwcKDhxccnSLvzfOW7jm7RSDiA4= Received: from CY1PR0401MB1536.namprd04.prod.outlook.com (10.163.19.154) by CY1PR0401MB1534.namprd04.prod.outlook.com (10.163.19.152) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1034.10; Thu, 20 Apr 2017 22:27:29 +0000 Received: from CY1PR0401MB1536.namprd04.prod.outlook.com ([10.163.19.154]) by CY1PR0401MB1536.namprd04.prod.outlook.com ([10.163.19.154]) with mapi id 15.01.1034.018; Thu, 20 Apr 2017 22:27:28 +0000 From: Bart Van Assche To: "James.Bottomley@HansenPartnership.com" , "bblock@linux.vnet.ibm.com" CC: "linux-scsi@vger.kernel.org" , "maxg@mellanox.com" , "israelr@mellanox.com" , "hare@suse.de" , "martin.petersen@oracle.com" Subject: Re: [PATCH v3 3/4] sd: Make synchronize cache upon shutdown asynchronous Thread-Topic: [PATCH v3 3/4] sd: Make synchronize cache upon shutdown asynchronous Thread-Index: AQHSt6Dnu2fwIPQi8EGD3QG5BkWsKaHLNU+AgAAUGACAAIONgIAAAoEAgAMD9gCAAAPygIAAA/WA Date: Thu, 20 Apr 2017 22:27:28 +0000 Message-ID: <1492727247.2642.12.camel@sandisk.com> References: <20170417173436.15555-1-bart.vanassche@sandisk.com> <20170417173436.15555-4-bart.vanassche@sandisk.com> <20170418144429.GA28949@bblock-ThinkPad-W530> <1492530984.3306.25.camel@HansenPartnership.com> <1492559235.2689.27.camel@sandisk.com> <1492559772.3306.58.camel@HansenPartnership.com> <1492725550.2642.9.camel@sandisk.com> <1492726397.21601.16.camel@HansenPartnership.com> In-Reply-To: <1492726397.21601.16.camel@HansenPartnership.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: yes X-MS-TNEF-Correlator: authentication-results: HansenPartnership.com; dkim=none (message not signed) header.d=none; HansenPartnership.com; dmarc=none action=none header.from=sandisk.com; x-originating-ip: [63.163.107.100] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; CY1PR0401MB1534; 7:+0QSIEVRbrGIb9JsK9RH+qQJfbJc2P0l7/5X81hHVt67rbl6r4F9iYldsKqHfcxxywOZLW1Cppg/mlsPu2WdDWU0ddl3jJz07INAzgj9HimbzlcwD9sfIR82O53xRQ6aRXFHw+FE7RR0aPfQPc8f+NqmOvLlGe0zJTvUy100V/nqAWtgxVMWCWzTzNvZruKTefdSwbJIvyJ3EPjC20ur3kavjs+28O9vCijcf7wzOi139gJL5WM356XD8t/qNiLWREivc8RiBgQj1XrJ9ewQlrO967pv6XVdauBiqDIBO9QihcNdvIIwNFn0K2HAZzaVHKsZdb3j4VPmJKRzwZFB2A==; 20:Nr3iVfl8vBxsTEtmQOjgZgJNIPSJi8z6jPaQgdBkVvUdPG0AvpmrIf2BuYFR6H3HWoGcBT49jFIZW1mCdIlOZo3319myB/8cnLLnu4+b2TxuO2Od5jBhuukVpUzH2U5vm0gwW9ouk8Z9WSpKlZto02TWaZ96Riq/NoPkL354B6s= x-ms-office365-filtering-correlation-id: a8fe79e7-628c-4f56-581f-08d4883c6dd3 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001)(2017030254075)(48565401081)(201703131423075)(201703031133081); SRVR:CY1PR0401MB1534; wdcipoutbound: EOP-TRUE x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(102415395)(6040450)(601004)(2401047)(8121501046)(5005006)(10201501046)(93006095)(93001095)(3002001)(6055026)(6041248)(20161123562025)(201703131423075)(201702281528075)(201703061421075)(20161123560025)(20161123555025)(20161123564025)(6072148); SRVR:CY1PR0401MB1534; BCL:0; PCL:0; RULEID:; SRVR:CY1PR0401MB1534; x-forefront-prvs: 02830F0362 x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(6009001)(39860400002)(39410400002)(39400400002)(39850400002)(39450400003)(39840400002)(377424004)(51234002)(24454002)(53936002)(81166006)(54906002)(7736002)(2906002)(86362001)(305945005)(8676002)(6512007)(102836003)(77096006)(3846002)(25786009)(6116002)(122556002)(6486002)(6246003)(36756003)(99286003)(66066001)(33646002)(6506006)(189998001)(6436002)(5890100001)(3660700001)(3280700002)(99936001)(76176999)(2501003)(5660300001)(8936002)(2950100002)(38730400002)(50986999)(93886004)(54356999)(103116003)(2900100001)(4326008); DIR:OUT; SFP:1102; SCL:1; SRVR:CY1PR0401MB1534; H:CY1PR0401MB1536.namprd04.prod.outlook.com; FPR:; SPF:None; MLV:sfv; LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: sandisk.com X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Apr 2017 22:27:28.5783 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86 X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR0401MB1534 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, 2017-04-20 at 15:13 -0700, James Bottomley wrote: > On Thu, 2017-04-20 at 21:59 +0000, Bart Van Assche wrote: > > This approach cannot work. A scsi_target_block() call by the > > transport layer can happen concurrently with the > > __scsi_remove_device() call and hence can occur at any time between > > the scsi_start_queue() call by __scsi_remove_device() and the > > sd_shutdown() call, resulting in a deadlock. > > How is that possible? Once the device goes into the CANCEL state, it > no longer can be found by starget_for_each_device() because > scsi_device_get() returns NULL ... unless you also have a patch > altering that? No changes were made in the SCSI core other than the attached two patches. I'm not sure about the root cause but every time I simulated a transport layer failure before I tried to remove the ib_srp kernel module I ran into a deadlock (see also the call traces below). Inspection of the lsscsi output and /sys/kernel/debug/block learned me that both queues involved in the deadlock were stopped. sysrq: SysRq : Show Blocked State   task                        PC stack   pid father kworker/11:3    D    0  2910      2 0x00000000 Workqueue: srp_remove srp_remove_work [ib_srp] Call Trace:  __schedule+0x3df/0xc10  schedule+0x3d/0x90  schedule_timeout+0x234/0x4b0  io_schedule_timeout+0x1e/0x50  wait_for_completion_io_timeout+0x118/0x180  blk_execute_rq+0x8e/0xc0  scsi_execute+0xe7/0x200  sd_sync_cache+0x8a/0x170  sd_shutdown+0x5f/0xe0  sd_remove+0x63/0xc0  device_release_driver_internal+0x13f/0x1e0  device_release_driver+0x12/0x20  bus_remove_device+0x114/0x190  device_del+0x205/0x320  __scsi_remove_device+0x132/0x140  scsi_forget_host+0x60/0x70  scsi_remove_host+0x71/0x110  srp_remove_work+0x90/0x220 [ib_srp]  process_one_work+0x20b/0x6a0  worker_thread+0x4e/0x4a0  kthread+0x113/0x150  ret_from_fork+0x2e/0x40 kworker/4:3     D    0  2913      2 0x00000000 Workqueue: srp_remove srp_remove_work [ib_srp] Call Trace:  __schedule+0x3df/0xc10  schedule+0x3d/0x90  schedule_timeout+0x234/0x4b0  io_schedule_timeout+0x1e/0x50  wait_for_completion_io_timeout+0x118/0x180  blk_execute_rq+0x8e/0xc0  scsi_execute+0xe7/0x200  sd_sync_cache+0x8a/0x170  sd_shutdown+0x5f/0xe0  sd_remove+0x63/0xc0  device_release_driver_internal+0x13f/0x1e0  device_release_driver+0x12/0x20  bus_remove_device+0x114/0x190  device_del+0x205/0x320  __scsi_remove_device+0x132/0x140  scsi_forget_host+0x60/0x70  scsi_remove_host+0x71/0x110  srp_remove_work+0x90/0x220 [ib_srp]  process_one_work+0x20b/0x6a0  worker_thread+0x4e/0x4a0  kthread+0x113/0x150  ret_from_fork+0x2e/0x40 modprobe        D    0  2916   2218 0x00000000 Call Trace:  __schedule+0x3df/0xc10  schedule+0x3d/0x90  schedule_timeout+0x273/0x4b0  wait_for_completion+0x108/0x170  flush_workqueue+0x207/0x720  srp_remove_one+0xbe/0x110 [ib_srp]  ib_unregister_client+0x18f/0x200 [ib_core]  srp_cleanup_module+0x10/0x618 [ib_srp]  SyS_delete_module+0x198/0x1f0  entry_SYSCALL_64_fastpath+0x18/0xad Bart. From fa69092d22f4f58ede7d37e68148a4aa4615d2ab Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 18 Apr 2017 10:11:02 -0700 Subject: [PATCH 2/2] Make __scsi_remove_device go straight from BLOCKED to DEL If a device is blocked, make __scsi_remove_device() cause it to transition to the DEL state. This means that all the commands issued in .shutdown() will error in the mid-layer, thus making the removal proceed without being stopped. This patch is a slightly modified version of a patch from James Bottomley. Signed-off-by: Bart Van Assche Cc: James Bottomley Cc: Israel Rukshin Cc: Max Gurtovoy Cc: Hannes Reinecke Cc: Benjamin Block --- drivers/scsi/scsi_lib.c | 2 +- drivers/scsi/scsi_sysfs.c | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index ffa6e61299a9..376cd1da102c 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -2611,7 +2611,6 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state) case SDEV_QUIESCE: case SDEV_OFFLINE: case SDEV_TRANSPORT_OFFLINE: - case SDEV_BLOCK: break; default: goto illegal; @@ -2625,6 +2624,7 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state) case SDEV_OFFLINE: case SDEV_TRANSPORT_OFFLINE: case SDEV_CANCEL: + case SDEV_BLOCK: case SDEV_CREATED_BLOCK: break; default: diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 82dfe07b1d47..5b03e58bdd67 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -1282,16 +1282,29 @@ void __scsi_remove_device(struct scsi_device *sdev) return; if (sdev->is_visible) { - if (scsi_device_set_state(sdev, SDEV_CANCEL) != 0) - return; + /* + * If blocked, we go straight to DEL and restart the queue so + * any commands issued during driver shutdown (like sync + * cache) are errored immediately. + */ + if (scsi_device_set_state(sdev, SDEV_CANCEL) != 0) { + if (scsi_device_set_state(sdev, SDEV_DEL) != 0) + return; + + scsi_start_queue(sdev); + sdev_printk(KERN_DEBUG, sdev, + "Changed state from BLOCKED to DEL\n"); + } bsg_unregister_queue(sdev->request_queue); device_unregister(&sdev->sdev_dev); transport_remove_device(dev); scsi_dh_remove_device(sdev); device_del(dev); - } else + } else { put_device(&sdev->sdev_dev); + scsi_start_queue(sdev); + } /* * Stop accepting new requests and wait until all queuecommand() and -- 2.12.2