From patchwork Wed Apr 26 00:41:48 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Song Liu X-Patchwork-Id: 9700097 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 D14F8603F6 for ; Wed, 26 Apr 2017 00:45:10 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B162E284D5 for ; Wed, 26 Apr 2017 00:45:10 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A566D284EE; Wed, 26 Apr 2017 00:45:10 +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=-7.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, 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 88349284D5 for ; Wed, 26 Apr 2017 00:45:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1433748AbdDZAoi (ORCPT ); Tue, 25 Apr 2017 20:44:38 -0400 Received: from [67.231.145.42] ([67.231.145.42]:48643 "EHLO mx0a-00082601.pphosted.com" rhost-flags-FAIL-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1433746AbdDZAof (ORCPT ); Tue, 25 Apr 2017 20:44:35 -0400 Received: from pps.filterd (m0044010.ppops.net [127.0.0.1]) by mx0a-00082601.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v3Q0dxUk009190; Tue, 25 Apr 2017 17:41:50 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fb.com; h=from : to : cc : subject : date : message-id : references : in-reply-to : content-type : content-id : content-transfer-encoding : mime-version; s=facebook; bh=WphJ9SVWmLwMcQnQo3US0aMDcAl/gBnccIAJqs7vIEQ=; b=Jel6b5RdCYiK0W+bLpIK8isELJfqG5qOFiBa+gcXe8DxIYfCxezLK9XcadSHP37W4uJ9 PRV5esKM3bA5qchEKGnzIUlKeWHTiCDy5Cvck5XTTKahMVEwqoe2OeeQdLtvNCDbbhbD 705n0TFSf4c2Wn18st10O6TtbXjHPST20Xc= Received: from mail.thefacebook.com ([199.201.64.23]) by mx0a-00082601.pphosted.com with ESMTP id 2a2ebg8ktc-1 (version=TLSv1 cipher=ECDHE-RSA-AES256-SHA bits=256 verify=NOT); Tue, 25 Apr 2017 17:41:50 -0700 Received: from NAM03-BY2-obe.outbound.protection.outlook.com (192.168.54.28) by o365-in.thefacebook.com (192.168.16.11) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 25 Apr 2017 17:41:49 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fb.onmicrosoft.com; s=selector1-fb-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=WphJ9SVWmLwMcQnQo3US0aMDcAl/gBnccIAJqs7vIEQ=; b=bbb795QJWjDASkVIa5uWb52HifRomZrddZBtIm7mvvH/t1e27hXTWlapsUWBtw2kXRBwk4L8OVfMK8TH88pISrPOZWxVojN2yZgN+KAZP4eQ6tHe/Hq9XomYtFJN6aBh2NdV0ZHp1uA/VslWthaW/ONabSGcCkh6XXKXq/2mkpM= Received: from CY4PR15MB1512.namprd15.prod.outlook.com (10.172.161.146) by CY4PR15MB1509.namprd15.prod.outlook.com (10.172.161.143) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1047.13; Wed, 26 Apr 2017 00:41:49 +0000 Received: from CY4PR15MB1512.namprd15.prod.outlook.com ([10.172.161.146]) by CY4PR15MB1512.namprd15.prod.outlook.com ([10.172.161.146]) with mapi id 15.01.1047.019; Wed, 26 Apr 2017 00:41:48 +0000 From: Song Liu To: Bart Van Assche CC: "linux-scsi@vger.kernel.org" , "hch@infradead.org" Subject: Re: [RFC] scsi: reduce protection of scan_mutex in scsi_remove_device Thread-Topic: [RFC] scsi: reduce protection of scan_mutex in scsi_remove_device Thread-Index: AQHSuuQjdPrhhbWq8kCEWhsZp507kKHWW56AgAAFLQCAAAK+gIAAOUoAgAAQ4wCAAChHAA== Date: Wed, 26 Apr 2017 00:41:48 +0000 Message-ID: <7EED4CA5-AD83-4C49-810C-D796CEC2358A@fb.com> References: <20170421211302.2667649-1-songliubraving@fb.com> <1493141028.2628.4.camel@sandisk.com> <11AE7D30-E24A-4DF3-8237-AF97A342D239@fb.com> <1493142729.2628.6.camel@sandisk.com> <3B06D0B6-825E-4788-9B87-BE3F35055DB2@fb.com> <1493158657.2628.26.camel@sandisk.com> In-Reply-To: <1493158657.2628.26.camel@sandisk.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-mailer: Apple Mail (2.3273) authentication-results: sandisk.com; dkim=none (message not signed) header.d=none; sandisk.com; dmarc=none action=none header.from=fb.com; x-originating-ip: [199.201.64.136] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; CY4PR15MB1509; 7:KqZerQgpsIJCRSLCv72jsnw3k3COT2Q5j9aRalFKanmeI9TuKxF8/pZ/Hf1RYNBwyUSaZ18sW6jQPrwRsH0h+MxTh5n7Fu/yFhG9McrWZzfXIOy3+uDyKfGedJXxzffojnGtfVz9TakbzlPU0KNO6z39exMA7mxdrd7E8OpXZ69/s1HKvmwQHfiPUOY3euR5ONeR+SELH0XkW/p4LX4M8nd+jFF+f+Zb5k4rVgWXbOz5mQpIAWnsDc6UpG6X7M4moJdA6BSeR6bP8rCUapnox8XvhGZqYU84iKIE9IbrSuv0SS0be942LHJ0+Yf8SJeYzzFUy2+72o0MH9oS0j9wMw==; 20:QNmvDCPtPQUYCZEmQrckqL24tVyfvcCO5yn1XqBjKTn9ZmNFpfBB2O2I4er3Z4aJnhhbSSjPaloynnibpP3r+Uwjim2q/hEsQE44RebQPEKhcDDK5jbPUOX7iZoes7KgzY+20Y3UXf/NmYNV1Cz7BbI1Zk22w6hv/F6ZFmkZY5c= x-ms-office365-filtering-correlation-id: fcbcf345-1afc-42d7-789f-08d48c3d0609 x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001)(2017030254075)(201703131423075)(201703031133081)(201702281549075); SRVR:CY4PR15MB1509; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(42932892334569); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040450)(601004)(2401047)(8121501046)(5005006)(93006095)(93001095)(3002001)(10201501046)(6041248)(201703131423075)(201702281528075)(201703061421075)(20161123560025)(20161123562025)(20161123555025)(20161123564025)(6072148); SRVR:CY4PR15MB1509; BCL:0; PCL:0; RULEID:; SRVR:CY4PR15MB1509; x-forefront-prvs: 0289B6431E x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(6009001)(39400400002)(39840400002)(39450400003)(39850400002)(39410400002)(377424004)(377454003)(24454002)(38730400002)(76176999)(8656002)(5660300001)(189998001)(77096006)(229853002)(2906002)(50986999)(6506006)(99286003)(6486002)(25786009)(6512007)(7736002)(54906002)(110136004)(122556002)(4326008)(93886004)(305945005)(33656002)(36756003)(53936002)(2900100001)(6116002)(82746002)(3846002)(102836003)(8936002)(50226002)(81166006)(8676002)(6436002)(2950100002)(83716003)(3280700002)(66066001)(3660700001)(6916009)(6246003)(86362001)(57306001); DIR:OUT; SFP:1102; SCL:1; SRVR:CY4PR15MB1509; H:CY4PR15MB1512.namprd15.prod.outlook.com; FPR:; SPF:None; MLV:sfv; LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-ID: <35453D334EBB414EB994D45D149D6F4D@namprd15.prod.outlook.com> MIME-Version: 1.0 X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Apr 2017 00:41:48.5878 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2 X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR15MB1509 X-OriginatorOrg: fb.com X-Proofpoint-Spam-Reason: safe X-FB-Internal: Safe X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-04-25_18:, , signatures=0 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 Apr 25, 2017, at 3:17 PM, Bart Van Assche wrote: > > On Tue, 2017-04-25 at 21:17 +0000, Song Liu wrote: >> I am not sure I fully understand the problem here. If I understand the logic >> correctly, when a device is being removed, it will stay in scsi_host->__devices >> until fully the remove routine is finished. And LUN scanning in parallel will >> find the device with scsi_device_lookup_by_target(), and thus it would not >> rescan the device until the device is fully removed? Did I miss anything here? > > Hello Song, > > The SCSI core is already complicated enough. Please don't complicate it further > by making subtle changes to the semantics of scan_mutex. Please also note that > I have proposed an alternative, namely to make the START STOP UNIT command > asynchronous. > Hello Bart, I total agree with your concern in making SCSI core more complicated. However, I am not sure whether how asynchronous START STOP UNIT command makes multiple deletes run in parallel. If I understand correctly, each device delete has the following steps: 1. lock scan_mutex 2. issue async START STOP UNIT 3. wait START STOP UNIT to complete 4. unlock scan_mutex scan_mutex will prevent multiple device deletes to run in parallel. On the other hand, maybe the problem is simpler than I thought? Once we ensure all slave_destroy() holds proper lock to access host level data, something as follows might just work. In this way, echoing into delete handle is not protected by scan_mutex. However, when the sysfs entry exists, the device should be fully added to the system. And before delete operation finishes, future scan should not re-add the device (as the device can be found by scsi_device_lookup_by_target). Am I too optimistic here? Thanks, Song diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 82dfe07..d1c3338 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -710,7 +710,7 @@ sdev_store_delete(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { if (device_remove_file_self(dev, attr)) - scsi_remove_device(to_scsi_device(dev)); + __scsi_remove_device(to_scsi_device(dev)); return count; };