From patchwork Tue Apr 10 01:34:55 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bart Van Assche X-Patchwork-Id: 10332195 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 B5A186028A for ; Tue, 10 Apr 2018 01:37:14 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A563828C7F for ; Tue, 10 Apr 2018 01:37:14 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 99C4728C85; Tue, 10 Apr 2018 01:37:14 +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.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, MAILING_LIST_MULTI, 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 EF13E28C84 for ; Tue, 10 Apr 2018 01:37:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751635AbeDJBhL (ORCPT ); Mon, 9 Apr 2018 21:37:11 -0400 Received: from esa3.hgst.iphmx.com ([216.71.153.141]:59277 "EHLO esa3.hgst.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751543AbeDJBhK (ORCPT ); Mon, 9 Apr 2018 21:37:10 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=wdc.com; i=@wdc.com; q=dns/txt; s=dkim.wdc.com; t=1523324231; x=1554860231; h=from:to:cc:subject:date:message-id:mime-version; bh=MHbGmLhie/USdnQX75/uwGtdMLSGNIDt8vRMdNrVAdk=; b=I9nfSkiwXR4mjgMWFXvxa7IjD9MJpoDNhapWF7/KQvvQq982c4bUF1Ey mfDqF7vsRBhORkCzm1yyqSHKxfEQMjNWpZCccT0Pg/zwzVvWQt/CUS5eh qNGmnUh+EsAkxJbdLBF6FehqxYLGSauRHFutGFuiahZcFqr70XGAgaVNj G90/3KOvT3gAb0p97+CSwo2Ofuv9aqchSAGsjvaQA88JHwYiKy+EPQPdE diEQZ3kOpCKzv23I+mnNN+jxJP1MpCIWrfD0hX7rPMZcWrz+tDL/wy0je 7H1Ug1Esab7Zqu+PVzQTnj6Q0d78QzUy67aKZuQONTbC6S6ZDH9n9qfwc A==; X-IronPort-AV: E=Sophos;i="5.48,429,1517846400"; d="scan'208";a="76308338" Received: from mail-sn1nam02lp0019.outbound.protection.outlook.com (HELO NAM02-SN1-obe.outbound.protection.outlook.com) ([216.32.180.19]) by ob1.hgst.iphmx.com with ESMTP; 10 Apr 2018 09:37:09 +0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sharedspace.onmicrosoft.com; s=selector1-wdc-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=f0X9zoF4gvqgpEIkFHxbe8Z6xPfxRJeB2W3nXawKOY4=; b=LG82tiRmZtp6gJNEagheExWfuWnWEysg9jyBquUWND+DtuvSJduB8Bx6LcjNQnu2fjD7w9YqZpXg3+4p+tSy3jmeGXv8SIIMamf4/3WI+eyUzdREezJW2egRvOQFoyoVYQaMRkkEA6vE/t5lIAh8BKQswlFhC2UsKsY3VUpLQR0= Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Bart.VanAssche@wdc.com; Received: from localhost.localdomain (50.225.201.71) by BN6PR04MB1188.namprd04.prod.outlook.com (2603:10b6:404:92::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.653.12; Tue, 10 Apr 2018 01:37:04 +0000 From: Bart Van Assche To: Jens Axboe Cc: linux-block@vger.kernel.org, Christoph Hellwig , Bart Van Assche , Tejun Heo , Sagi Grimberg , Israel Rukshin , Max Gurtovoy , stable@vger.kernel.org Subject: [PATCH v4] blk-mq: Fix race conditions in request timeout handling Date: Mon, 9 Apr 2018 18:34:55 -0700 Message-Id: <20180410013455.7448-1-bart.vanassche@wdc.com> X-Mailer: git-send-email 2.16.2 MIME-Version: 1.0 X-Originating-IP: [50.225.201.71] X-ClientProxiedBy: SN4PR0501CA0025.namprd05.prod.outlook.com (2603:10b6:803:40::38) To BN6PR04MB1188.namprd04.prod.outlook.com (2603:10b6:404:92::13) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: deae23d7-d4ff-4bdd-983a-08d59e839102 X-MS-Office365-Filtering-HT: Tenant X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652020)(5600026)(4604075)(48565401081)(4534165)(4627221)(201703031133081)(201702281549075)(2017052603328)(7153060)(7193020); SRVR:BN6PR04MB1188; X-Microsoft-Exchange-Diagnostics: 1; BN6PR04MB1188; 3:aEEAfyxLhXSiOm1oOTZiX6ABDqg6FFCHfFe9p6mehU8aRhZM1Ubt1DLJSbDocUU4n7GLh2Oe9O2UC4mYzq1ptOmx4tYajPgsCYLSxspN+xEdqteavV4MF5GVKQOxEk+N42izD+i0sqRpNqB7iPKWLOUrdPQjP3wn/Y+3drGGgLWY+B3/Q52Yn2bxCWTCw/XNOJqbPg7kuBA7XgJBup/BpS+pcoSY1M7LiwrMb/hKA6Ad4+npZtS8f5V4G3kpKe5I; 25:V76zj3evZsSIiVgFRY/XhZ9hS2PIZv05qL3qpI4j+VJDmqr3fo6n+KWWnLS9nQIbaJHOKyre33p6IARR0aI6MT9z5x+iXAU3tbRc4pMwvTKMJssGbG4R8YnpebLte6k7CKzjtx8dYs4IyZo5H7sJerxLFIBR9ID0iipKUex0RjYXiZ98ofKys+gMaz0tWrF2cSuCmi2vxfx6iqK4Q59aBLg/Eqleutk3b1tSFB+dEn51QLRpiuFbOpuCtCvaAnhjIqjK1RguOqC3wfbOyG4Md/Nzk1R8+y8sgInlmp+eDjz1m1BNJZ1jVDLpf8vB/u6+RLmlHwv8h5nMvOf8pm4XUw==; 31:Oc+a5scQ4Zu9lDm08veb/gjejhOE3RDfZBECXgjMk9xJTDQQ7HyikmRelSx8ztRRmTniudtDzcPXHi6gviaK3lfyr1DhiCbba+ZQ7UOOHxyAMrfg0StAjl0mpU589rvEngdyL9r+DofirG8LlI/kcc7Y9LshYarkLqqJ33/bHhgKTPFobFuTYCDhvIH+5dl2x0jo4i7vOH1drsiP2yJ3omNxgWdsGxWC9jHg1dYqXNw= X-MS-TrafficTypeDiagnostic: BN6PR04MB1188: WDCIPOUTBOUND: EOP-TRUE X-Microsoft-Exchange-Diagnostics: 1; BN6PR04MB1188; 20:FYvyApBDzEbBT+UJiiI6fhON+f2uWfGDhi4TOkF8TBb49tOuNo//KHDstCQSOCjjixMRbVvK5RErjBv9uwDfnAaIXJypS6YFjE/P43zhv7THqKQD06JpqSx8cTr2/XbQfB4+qP0YKIZOqS77lfIR2m9wWxYOKwNAr1ObSGWs6QF+oKPvUd4bGK5rDaRbOffrLg0S9u5u0XayOUzF2UFqH707JSfklGXcFNOnnRnp6AatwF6VH4d66Pszpizvuj2PF/GXxNSqVsklR0Jz41R+wISYEaZHMeN2PVC9m1sJv8cny5aqCKjiUvzdfh0U1EZDxhtV1lq749ZTKiRYyDYHO9TQmPWfFscTVX+4mYRNvWVIBXFeCqDqKx9zpuwNP7ifxv6P3ZfSyaBOt77CMdJ+TMac9JxIsTecwwoIOKLDVcpB0ykwH3+FdRG7uznnF7Wz7EBKmY4TBnToKhYGghE/RgFbwbTQ6al1ycnnzUMAjdOTeXrY/t8lAgPVle3zI9V5 X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(9452136761055)(788757137089)(258649278758335)(278021516957215)(15185016700835); X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(6040522)(2401047)(5005006)(8121501046)(3002001)(10201501046)(93006095)(93001095)(3231221)(944501327)(52105095)(6055026)(6041310)(20161123564045)(20161123558120)(20161123560045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123562045)(6072148)(201708071742011); SRVR:BN6PR04MB1188; BCL:0; PCL:0; RULEID:; SRVR:BN6PR04MB1188; X-Microsoft-Exchange-Diagnostics: 1; BN6PR04MB1188; 4:155f8b7EqnlWGXj5RgZDV/vbDuNAQgNlNoDEEHoZEfd23Z8K3FnwS/nSqmqWDQgQZWTZecwB3lKBdi9ttwt4OYvrhS6FkFz0AJXuGnQK0oNBX9m55LD+q1FmQE3t1XwRsJ4zMgJARX672dpX1LRNeP/wejH4tzOvpY+lu5wiePea7kENRaMebrRREJpfq4FB5kPo0sLejVWp9KaZUeaNNAvdfAjrHDF8f1EJpcAaatO0mNitwsy0P16EDCtti9tDjUyTpOvVh4fN6fnTkq04Lu0VcBR+jzTuhamOoa2874vpapGt/O6tx/vUb/Yx7GLkjAniSBuY1gSEGm5TKI34ikCDHKsENwRm2mxDc9pXtwJublOoWewBz0lrYIgGgqf9rb9ZqpWbiH+mSJCbChDZ/YM1CTUNGmmM54zwho2OhiVq8siIh8EpCzFoUdrf+UBM X-Forefront-PRVS: 0638FD5066 X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10019020)(6069001)(346002)(376002)(396003)(39380400002)(366004)(39860400002)(51234002)(189003)(199004)(6506007)(26005)(68736007)(4326008)(106356001)(59450400001)(16526019)(956004)(8936002)(2906002)(50226002)(105586002)(47776003)(54906003)(81166006)(476003)(6916009)(51416003)(25786009)(8676002)(7736002)(2616005)(186003)(81156014)(72206003)(486006)(305945005)(16586007)(97736004)(48376002)(5660300001)(6666003)(66066001)(53936002)(478600001)(52116002)(86362001)(966005)(36756003)(6486002)(6116002)(3846002)(6512007)(316002)(1076002)(6306002)(50466002)(386003); DIR:OUT; SFP:1102; SCL:1; SRVR:BN6PR04MB1188; H:localhost.localdomain; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1; BN6PR04MB1188; 23:WAnDD0faVe3cqX5cKhY0KPgw4M7DiLq4Vg9IWfsLp?= =?us-ascii?Q?CfT5UBAxNOTE1TBJO56qzQxP7mkiS5R+b36vE8F3iCPf/LFm5ymF08VjkliD?= =?us-ascii?Q?2k/BGQJzb8qbl2uCLST0TCL7DUFHKm7ehyw2ovKOIIKX5y3vZkxlMt7JBNB/?= =?us-ascii?Q?hCPCPQ+Yk59tn3Y1ltoprKRhGWZse2zj6ZrBAJDvbO5eBX3PTx+hmpw5wbfn?= =?us-ascii?Q?1oG8jvqb72QJeZ6D8Kx5ksevsHDxnPnieBLJPFWYDOIUXdOlrW8AjyN7YEC+?= =?us-ascii?Q?EQKfCYCswcHQLrECaPQu8yZ8e9bUGbHh8tPT/24VnmNSRg5eivwk4P/XivOm?= =?us-ascii?Q?yzruynAlTV2SRLR1XyXg2Y1oqAOr8/9eWf9lDXkhnIVumpm4zes3ypcUiOFY?= =?us-ascii?Q?3a78ChRGQIObO5g1RXi7oOiwOqivyXBszQamDknM8k70SU+TaXGmEk4xSmMu?= =?us-ascii?Q?gl+dLpn9t8BMRasWEDpfxpnoMaP/i66PKQ8DCWs7JdvC8UA6oYo3LI+Dh1kY?= =?us-ascii?Q?CfpVDbmrDqyuCefQmB2ohz1LTZKPCNia2EBs1r6f1i4hQw0hJi20ga2fopRx?= =?us-ascii?Q?aAhLdl5nSVibN8pIDkmEebtgVdaUILwXrh2NmkpHGc8w1apPcSoMGUUY8XSU?= =?us-ascii?Q?PBq1Ukip3gbmGD/TtA3uzkOyL9AAJYTGwajfKuxXSzzFFqlfcqj8c0627Htu?= =?us-ascii?Q?mX877CelL3RSVMyyuZ9ijNv0/YxS6z77ZxZHP03Nk77sublIqPXubdDz9cre?= =?us-ascii?Q?zwn0EDcNo3vwwMl5r1DXVNHKWrMDPr6le9sy/O1YkuAfCOkUGwJMt7BgT7lT?= =?us-ascii?Q?JMZXh+7HLc1Uu4kze0sneSudsyCUHgceSQ6uEmQBrU6LttQyzAomLoD1OrIq?= =?us-ascii?Q?Iu+dJgaLiU3a/VDwu7hFbJMFRgDkRFr4Wx2Lt66IrdiOfZS77+i3u5XCos/O?= =?us-ascii?Q?ZP765vFSW36cZtpS6zikxsgCpPIbHurYs4cWIIc1x55wIMFOueChYw6VBxLl?= =?us-ascii?Q?yqGPJbdqAjE780GdHG+uIFP9cxYU0cfepqO+B2lSkNLHCNSv2EM3RuzVLYVl?= =?us-ascii?Q?XKSnv/eR2iF6hzZwG5jVhnuo+l5+wgqm5F5gkptxt1PGTFNEG52JIz0sPUYC?= =?us-ascii?Q?dT1Eaow7g3L6maYs0F788rYj5h0mm0HafFQYhI0uoAUcW0xmJwT9fGWRRL/+?= =?us-ascii?Q?kyoYYj8/F0OxrgX5W/91iG+ByI5mcvRnk95C7VX40pjSYob/faK0GZ+yoxak?= =?us-ascii?Q?dzZfGCNoA6fozy6Rgw+A5VkK1GM/d2fE6WgxXU7aG6u07Vhaq1dpSN4Y70LS?= =?us-ascii?B?dz09?= X-Microsoft-Antispam-Message-Info: hHnv9htKRacNum5GLYeYEvsOTpei8oxuLL5i6+LJkbbSAylJQFrZz3z45SH93U1v/rcvzkYg3rcKk8glOkNnb0d+ztLA3yFpn3/ET73mBKy7sfHJy1ewXkpnaygzCUvuceOrIVn1yipX9MN2VPC+eG5tcR3JH2h/maw4LwaxGEXD2mikW6gG3XrJm+FK1PsmOHCzAKLqr8dr338ZydI1BA== X-Microsoft-Exchange-Diagnostics: 1; BN6PR04MB1188; 6:t2JoSYWVX2fo3RI/c8yCoBOhorH6tcKBhhKxFBk++SURFNUJsHuf/ToNHqv4QWx+u8FtQDjpT8pmYRWMLmwW8POL2iafzfIGZT0gvi7uhhJh27ter6bQuGrxTBBqK038rZvGgMyo+Cai/68UnzMCy9nX9wDF+Pg2hmic6+CqGdDRYX9yXq32Z9HsSwjQoWRhMqKAhxzbkHcqs9FigbxhdR8wv25ibacrH/PqVejQdv/pSihYL8nrNLCeWXnd7yX1g3oB91eVFGzTAf6/IHq06ozl+AqA7bNcWzEYfRag17BQhlSIOQD28bEPMI5Z79ti4NkHPyvjwGPhAx+AwLKWr/ZwYmpKsJHObtZtZS6k+QbJysj0mH0R9/3ABrl96NP5Xf50mqIFDm4O5xIlgIXxaSsoQ7fetr+tTeVQNN83OKQchNEJbRhpZExUfum0NEV4zSkk4hEk2LUFWv6M5cVeog==; 5:nt85pJ4R+fZnWzQ7MAoH3OJ8CYfRdChq96B4dFpPxjAq5/YSqh4a/WwvohW5preTIect4YY00bu0xePwvRbFR3mSu6GugdveG6lqu4rdI8XaoZvOhm5Dt6T+0zcmGi5sroE/s6xjndg1/Sp2kKQbAGZ+1L7y2o9E5bTEZzOfRmg=; 24:xvZIViqAOmmDLS9wjpkbCCrm6JDrP+yYtsx60PNDk5qjGGk58C7+kbZSdBW68akpC4sshpk9/whbfPIV3IYT5TtzkprIMiHu4kjmw8bfOSE= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1; BN6PR04MB1188; 7:X2LwWegUZpfyKe3O+eQPPs0SvGf+2xaZcZop3ng2CG6+AweaBXhDXH2dpHAf9Ty0oW9mbrm5os11tHwOt84n0iODrxr3YGy4chp27nTNTKP0W7iUUxio8vNgZO2hpCJPnaZUh8At/3XhN1/n8+T9y8csM7HefxPJU4n6H2II66GgidBHQBs8vEUzkLFBXSVEuJwSo+oUVJu6QOT1CEJjUs7BoUz9rcO6nrTkQCkKdBUvQylsusFfcxCb1aHHEEfh; 20:U1N0ejtK6pTtnALFBojqf0H77tVVmLohDa+OzksMzR4vKH6uBDF4Jg3OIj4QN140dhJx091GLRdPH04SFNbu5/vmYx9gBVHpHLbcukyjYJKFTgulOZRmW7CbFCBZ9aUO2bsoHDcx5lRr385NqM5L2Zn0edjwLXIDHHJEcRCyuKk= X-OriginatorOrg: wdc.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Apr 2018 01:37:04.6978 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: deae23d7-d4ff-4bdd-983a-08d59e839102 X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: b61c8803-16f3-4c35-9b17-6f65f441df86 X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR04MB1188 Sender: linux-block-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP If a completion occurs after blk_mq_rq_timed_out() has reset rq->aborted_gstate and the request is again in flight when the timeout expires then a request will be completed twice: a first time by the timeout handler and a second time when the regular completion occurs. Additionally, the blk-mq timeout handling code ignores completions that occur after blk_mq_check_expired() has been called and before blk_mq_rq_timed_out() has reset rq->aborted_gstate. If a block driver timeout handler always returns BLK_EH_RESET_TIMER then the result will be that the request never terminates. Since the request state can be updated from two different contexts, namely regular completion and request timeout, this race cannot be fixed with RCU synchronization only. Fix this race as follows: - Split __deadline in two variables, namely lq_deadline for legacy request queues and mq_deadline for blk-mq request queues. Use atomic operations to update mq_deadline. - Use the deadline instead of the request generation to detect whether or not a request timer fired after reinitialization of a request. - Store the request state in the lowest two bits of the deadline instead of the lowest two bits of 'gstate'. - Remove all request member variables that became superfluous due to this change: gstate, aborted_gstate, gstate_seq and aborted_gstate_sync. - Remove the request state information that became superfluous due to this patch, namely RQF_MQ_TIMEOUT_EXPIRED. - Remove the hctx member that became superfluous due to these changes, namely nr_expired. - Remove the code that became superfluous due to this change, namely the RCU lock and unlock statements in blk_mq_complete_request() and also the synchronize_rcu() call in the timeout handler. This patch fixes the following kernel crashes: BUG: unable to handle kernel NULL pointer dereference at (null) Oops: 0000 [#1] PREEMPT SMP CPU: 2 PID: 151 Comm: kworker/2:1H Tainted: G W 4.15.0-dbg+ #3 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.0.0-prebuilt.qemu-project.org 04/01/2014 Workqueue: kblockd blk_mq_timeout_work RIP: 0010:scsi_times_out+0x17/0x2c0 [scsi_mod] Call Trace: blk_mq_terminate_expired+0x42/0x80 bt_iter+0x3d/0x50 blk_mq_queue_tag_busy_iter+0xe9/0x200 blk_mq_timeout_work+0x181/0x2e0 process_one_work+0x21c/0x6d0 worker_thread+0x35/0x380 kthread+0x117/0x130 ret_from_fork+0x24/0x30 This patch also fixes a double completion problem in the NVMeOF initiator driver. See also http://lists.infradead.org/pipermail/linux-nvme/2018-February/015848.html. Fixes: 1d9bd5161ba3 ("blk-mq: replace timeout synchronization with a RCU and generation based scheme") Signed-off-by: Bart Van Assche Cc: Tejun Heo Cc: Christoph Hellwig Cc: Sagi Grimberg Cc: Israel Rukshin , Cc: Max Gurtovoy Cc: # v4.16 --- Changes compared to v3 (see also https://www.mail-archive.com/linux-block@vger.kernel.org/msg20073.html): - Removed the spinlock again that was introduced to protect the request state. v4 uses atomic_long_cmpxchg() instead. - Split __deadline into two variables - one for the legacy block layer and one for blk-mq. Changes compared to v2 (https://www.mail-archive.com/linux-block@vger.kernel.org/msg18338.html): - Rebased and retested on top of kernel v4.16. Changes compared to v1 (https://www.mail-archive.com/linux-block@vger.kernel.org/msg18089.html): - Removed the gstate and aborted_gstate members of struct request and used the __deadline member to encode both the generation and state information. block/blk-core.c | 2 - block/blk-mq-debugfs.c | 1 - block/blk-mq.c | 166 +++---------------------------------------------- block/blk-mq.h | 47 ++++++++------ block/blk-timeout.c | 57 ++++++++++++----- block/blk.h | 41 ++++++++++-- include/linux/blk-mq.h | 1 - include/linux/blkdev.h | 32 +++------- 8 files changed, 122 insertions(+), 225 deletions(-) diff --git a/block/blk-core.c b/block/blk-core.c index 0c48bef8490f..422b79b61bb9 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -200,8 +200,6 @@ void blk_rq_init(struct request_queue *q, struct request *rq) rq->start_time = jiffies; set_start_time_ns(rq); rq->part = NULL; - seqcount_init(&rq->gstate_seq); - u64_stats_init(&rq->aborted_gstate_sync); } EXPORT_SYMBOL(blk_rq_init); diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c index 6f72413b6cab..80c7c585769f 100644 --- a/block/blk-mq-debugfs.c +++ b/block/blk-mq-debugfs.c @@ -345,7 +345,6 @@ static const char *const rqf_name[] = { RQF_NAME(STATS), RQF_NAME(SPECIAL_PAYLOAD), RQF_NAME(ZONE_WRITE_LOCKED), - RQF_NAME(MQ_TIMEOUT_EXPIRED), RQF_NAME(MQ_POLL_SLEPT), }; #undef RQF_NAME diff --git a/block/blk-mq.c b/block/blk-mq.c index 7816d28b7219..337e10a5a30c 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -305,7 +305,6 @@ static struct request *blk_mq_rq_ctx_init(struct blk_mq_alloc_data *data, rq->special = NULL; /* tag was already set */ rq->extra_len = 0; - rq->__deadline = 0; INIT_LIST_HEAD(&rq->timeout_list); rq->timeout = 0; @@ -527,8 +526,7 @@ static void __blk_mq_complete_request(struct request *rq) bool shared = false; int cpu; - WARN_ON_ONCE(blk_mq_rq_state(rq) != MQ_RQ_IN_FLIGHT); - blk_mq_rq_update_state(rq, MQ_RQ_COMPLETE); + WARN_ON_ONCE(blk_mq_rq_state(rq) != MQ_RQ_COMPLETE); if (rq->internal_tag != -1) blk_mq_sched_completed_request(rq); @@ -577,36 +575,6 @@ static void hctx_lock(struct blk_mq_hw_ctx *hctx, int *srcu_idx) *srcu_idx = srcu_read_lock(hctx->srcu); } -static void blk_mq_rq_update_aborted_gstate(struct request *rq, u64 gstate) -{ - unsigned long flags; - - /* - * blk_mq_rq_aborted_gstate() is used from the completion path and - * can thus be called from irq context. u64_stats_fetch in the - * middle of update on the same CPU leads to lockup. Disable irq - * while updating. - */ - local_irq_save(flags); - u64_stats_update_begin(&rq->aborted_gstate_sync); - rq->aborted_gstate = gstate; - u64_stats_update_end(&rq->aborted_gstate_sync); - local_irq_restore(flags); -} - -static u64 blk_mq_rq_aborted_gstate(struct request *rq) -{ - unsigned int start; - u64 aborted_gstate; - - do { - start = u64_stats_fetch_begin(&rq->aborted_gstate_sync); - aborted_gstate = rq->aborted_gstate; - } while (u64_stats_fetch_retry(&rq->aborted_gstate_sync, start)); - - return aborted_gstate; -} - /** * blk_mq_complete_request - end I/O on a request * @rq: the request being processed @@ -618,27 +586,12 @@ static u64 blk_mq_rq_aborted_gstate(struct request *rq) void blk_mq_complete_request(struct request *rq) { struct request_queue *q = rq->q; - struct blk_mq_hw_ctx *hctx = blk_mq_map_queue(q, rq->mq_ctx->cpu); - int srcu_idx; if (unlikely(blk_should_fake_timeout(q))) return; - /* - * If @rq->aborted_gstate equals the current instance, timeout is - * claiming @rq and we lost. This is synchronized through - * hctx_lock(). See blk_mq_timeout_work() for details. - * - * Completion path never blocks and we can directly use RCU here - * instead of hctx_lock() which can be either RCU or SRCU. - * However, that would complicate paths which want to synchronize - * against us. Let stay in sync with the issue path so that - * hctx_lock() covers both issue and completion paths. - */ - hctx_lock(hctx, &srcu_idx); - if (blk_mq_rq_aborted_gstate(rq) != rq->gstate) + if (blk_mq_change_rq_state(rq, MQ_RQ_IN_FLIGHT, MQ_RQ_COMPLETE)) __blk_mq_complete_request(rq); - hctx_unlock(hctx, srcu_idx); } EXPORT_SYMBOL(blk_mq_complete_request); @@ -662,27 +615,8 @@ void blk_mq_start_request(struct request *rq) wbt_issue(q->rq_wb, &rq->issue_stat); } - WARN_ON_ONCE(blk_mq_rq_state(rq) != MQ_RQ_IDLE); - - /* - * Mark @rq in-flight which also advances the generation number, - * and register for timeout. Protect with a seqcount to allow the - * timeout path to read both @rq->gstate and @rq->deadline - * coherently. - * - * This is the only place where a request is marked in-flight. If - * the timeout path reads an in-flight @rq->gstate, the - * @rq->deadline it reads together under @rq->gstate_seq is - * guaranteed to be the matching one. - */ - preempt_disable(); - write_seqcount_begin(&rq->gstate_seq); - - blk_mq_rq_update_state(rq, MQ_RQ_IN_FLIGHT); - blk_add_timer(rq); - - write_seqcount_end(&rq->gstate_seq); - preempt_enable(); + /* Mark @rq in-flight and set its deadline. */ + blk_mq_add_timer(rq, MQ_RQ_IDLE, MQ_RQ_IN_FLIGHT); if (q->dma_drain_size && blk_rq_bytes(rq)) { /* @@ -695,11 +629,6 @@ void blk_mq_start_request(struct request *rq) } EXPORT_SYMBOL(blk_mq_start_request); -/* - * When we reach here because queue is busy, it's safe to change the state - * to IDLE without checking @rq->aborted_gstate because we should still be - * holding the RCU read lock and thus protected against timeout. - */ static void __blk_mq_requeue_request(struct request *rq) { struct request_queue *q = rq->q; @@ -811,7 +740,6 @@ EXPORT_SYMBOL(blk_mq_tag_to_rq); struct blk_mq_timeout_data { unsigned long next; unsigned int next_set; - unsigned int nr_expired; }; static void blk_mq_rq_timed_out(struct request *req, bool reserved) @@ -819,8 +747,6 @@ static void blk_mq_rq_timed_out(struct request *req, bool reserved) const struct blk_mq_ops *ops = req->q->mq_ops; enum blk_eh_timer_return ret = BLK_EH_RESET_TIMER; - req->rq_flags |= RQF_MQ_TIMEOUT_EXPIRED; - if (ops->timeout) ret = ops->timeout(req, reserved); @@ -829,13 +755,7 @@ static void blk_mq_rq_timed_out(struct request *req, bool reserved) __blk_mq_complete_request(req); break; case BLK_EH_RESET_TIMER: - /* - * As nothing prevents from completion happening while - * ->aborted_gstate is set, this may lead to ignored - * completions and further spurious timeouts. - */ - blk_mq_rq_update_aborted_gstate(req, 0); - blk_add_timer(req); + blk_mq_add_timer(req, MQ_RQ_COMPLETE, MQ_RQ_IN_FLIGHT); break; case BLK_EH_NOT_HANDLED: break; @@ -849,60 +769,23 @@ static void blk_mq_check_expired(struct blk_mq_hw_ctx *hctx, struct request *rq, void *priv, bool reserved) { struct blk_mq_timeout_data *data = priv; - unsigned long gstate, deadline; - int start; - - might_sleep(); - - if (rq->rq_flags & RQF_MQ_TIMEOUT_EXPIRED) - return; - - /* read coherent snapshots of @rq->state_gen and @rq->deadline */ - while (true) { - start = read_seqcount_begin(&rq->gstate_seq); - gstate = READ_ONCE(rq->gstate); - deadline = blk_rq_deadline(rq); - if (!read_seqcount_retry(&rq->gstate_seq, start)) - break; - cond_resched(); - } + unsigned long deadline = blk_mq_rq_deadline(rq); - /* if in-flight && overdue, mark for abortion */ - if ((gstate & MQ_RQ_STATE_MASK) == MQ_RQ_IN_FLIGHT && - time_after_eq(jiffies, deadline)) { - blk_mq_rq_update_aborted_gstate(rq, gstate); - data->nr_expired++; - hctx->nr_expired++; + if (time_after_eq(jiffies, deadline) && + blk_mq_change_rq_state(rq, MQ_RQ_IN_FLIGHT, MQ_RQ_COMPLETE)) { + blk_mq_rq_timed_out(rq, reserved); } else if (!data->next_set || time_after(data->next, deadline)) { data->next = deadline; data->next_set = 1; } -} -static void blk_mq_terminate_expired(struct blk_mq_hw_ctx *hctx, - struct request *rq, void *priv, bool reserved) -{ - /* - * We marked @rq->aborted_gstate and waited for RCU. If there were - * completions that we lost to, they would have finished and - * updated @rq->gstate by now; otherwise, the completion path is - * now guaranteed to see @rq->aborted_gstate and yield. If - * @rq->aborted_gstate still matches @rq->gstate, @rq is ours. - */ - if (!(rq->rq_flags & RQF_MQ_TIMEOUT_EXPIRED) && - READ_ONCE(rq->gstate) == rq->aborted_gstate) - blk_mq_rq_timed_out(rq, reserved); } static void blk_mq_timeout_work(struct work_struct *work) { struct request_queue *q = container_of(work, struct request_queue, timeout_work); - struct blk_mq_timeout_data data = { - .next = 0, - .next_set = 0, - .nr_expired = 0, - }; + struct blk_mq_timeout_data data = { }; struct blk_mq_hw_ctx *hctx; int i; @@ -925,33 +808,6 @@ static void blk_mq_timeout_work(struct work_struct *work) /* scan for the expired ones and set their ->aborted_gstate */ blk_mq_queue_tag_busy_iter(q, blk_mq_check_expired, &data); - if (data.nr_expired) { - bool has_rcu = false; - - /* - * Wait till everyone sees ->aborted_gstate. The - * sequential waits for SRCUs aren't ideal. If this ever - * becomes a problem, we can add per-hw_ctx rcu_head and - * wait in parallel. - */ - queue_for_each_hw_ctx(q, hctx, i) { - if (!hctx->nr_expired) - continue; - - if (!(hctx->flags & BLK_MQ_F_BLOCKING)) - has_rcu = true; - else - synchronize_srcu(hctx->srcu); - - hctx->nr_expired = 0; - } - if (has_rcu) - synchronize_rcu(); - - /* terminate the ones we won */ - blk_mq_queue_tag_busy_iter(q, blk_mq_terminate_expired, NULL); - } - if (data.next_set) { data.next = blk_rq_timeout(round_jiffies_up(data.next)); mod_timer(&q->timeout, data.next); @@ -2087,8 +1943,6 @@ static int blk_mq_init_request(struct blk_mq_tag_set *set, struct request *rq, return ret; } - seqcount_init(&rq->gstate_seq); - u64_stats_init(&rq->aborted_gstate_sync); return 0; } diff --git a/block/blk-mq.h b/block/blk-mq.h index 88c558f71819..4f96fd66eb8a 100644 --- a/block/blk-mq.h +++ b/block/blk-mq.h @@ -27,10 +27,7 @@ struct blk_mq_ctx { struct kobject kobj; } ____cacheline_aligned_in_smp; -/* - * Bits for request->gstate. The lower two bits carry MQ_RQ_* state value - * and the upper bits the generation number. - */ +/* Lowest two bits of request->mq_deadline. */ enum mq_rq_state { MQ_RQ_IDLE = 0, MQ_RQ_IN_FLIGHT = 1, @@ -38,7 +35,6 @@ enum mq_rq_state { MQ_RQ_STATE_BITS = 2, MQ_RQ_STATE_MASK = (1 << MQ_RQ_STATE_BITS) - 1, - MQ_RQ_GEN_INC = 1 << MQ_RQ_STATE_BITS, }; void blk_mq_freeze_queue(struct request_queue *q); @@ -104,9 +100,30 @@ void blk_mq_release(struct request_queue *q); * blk_mq_rq_state() - read the current MQ_RQ_* state of a request * @rq: target request. */ -static inline int blk_mq_rq_state(struct request *rq) +static inline enum mq_rq_state blk_mq_rq_state(struct request *rq) { - return READ_ONCE(rq->gstate) & MQ_RQ_STATE_MASK; + return atomic_long_read(&rq->mq_deadline) & MQ_RQ_STATE_MASK; +} + +/** + * blk_mq_change_rq_state - atomically test and set request state + * @rq: Request pointer. + * @old: Old request state. + * @new: New request state. + * + * Returns %true if and only if the old state was @old and if the state has + * been changed into @new. + */ +static inline bool blk_mq_change_rq_state(struct request *rq, + enum mq_rq_state old_s, + enum mq_rq_state new_s) +{ + unsigned long old_d = (atomic_long_read(&rq->mq_deadline) & + ~(unsigned long)MQ_RQ_STATE_MASK) | old_s; + unsigned long new_d = (old_d & ~(unsigned long)MQ_RQ_STATE_MASK) | + new_s; + + return atomic_long_cmpxchg(&rq->mq_deadline, old_d, new_d) == old_d; } /** @@ -114,23 +131,13 @@ static inline int blk_mq_rq_state(struct request *rq) * @rq: target request. * @state: new state to set. * - * Set @rq's state to @state. The caller is responsible for ensuring that - * there are no other updaters. A request can transition into IN_FLIGHT - * only from IDLE and doing so increments the generation number. + * Set @rq's state to @state. */ static inline void blk_mq_rq_update_state(struct request *rq, - enum mq_rq_state state) + enum mq_rq_state new_s) { - u64 old_val = READ_ONCE(rq->gstate); - u64 new_val = (old_val & ~MQ_RQ_STATE_MASK) | state; - - if (state == MQ_RQ_IN_FLIGHT) { - WARN_ON_ONCE((old_val & MQ_RQ_STATE_MASK) != MQ_RQ_IDLE); - new_val += MQ_RQ_GEN_INC; + while (!blk_mq_change_rq_state(rq, blk_mq_rq_state(rq), new_s)) { } - - /* avoid exposing interim values */ - WRITE_ONCE(rq->gstate, new_val); } static inline struct blk_mq_ctx *__blk_mq_get_ctx(struct request_queue *q, diff --git a/block/blk-timeout.c b/block/blk-timeout.c index 50a191720055..3ca829dce2d6 100644 --- a/block/blk-timeout.c +++ b/block/blk-timeout.c @@ -165,8 +165,9 @@ void blk_abort_request(struct request *req) * immediately and that scan sees the new timeout value. * No need for fancy synchronizations. */ - blk_rq_set_deadline(req, jiffies); - kblockd_schedule_work(&req->q->timeout_work); + if (blk_mq_rq_set_deadline(req, jiffies, MQ_RQ_IN_FLIGHT, + MQ_RQ_IN_FLIGHT)) + kblockd_schedule_work(&req->q->timeout_work); } else { if (blk_mark_rq_complete(req)) return; @@ -187,15 +188,8 @@ unsigned long blk_rq_timeout(unsigned long timeout) return timeout; } -/** - * blk_add_timer - Start timeout timer for a single request - * @req: request that is about to start running. - * - * Notes: - * Each request has its own timer, and as it is added to the queue, we - * set up the timer. When the request completes, we cancel the timer. - */ -void blk_add_timer(struct request *req) +static void __blk_add_timer(struct request *req, enum mq_rq_state old, + enum mq_rq_state new) { struct request_queue *q = req->q; unsigned long expiry; @@ -216,15 +210,17 @@ void blk_add_timer(struct request *req) if (!req->timeout) req->timeout = q->rq_timeout; - blk_rq_set_deadline(req, jiffies + req->timeout); - req->rq_flags &= ~RQF_MQ_TIMEOUT_EXPIRED; - /* * Only the non-mq case needs to add the request to a protected list. * For the mq case we simply scan the tag map. */ - if (!q->mq_ops) + if (!q->mq_ops) { + blk_rq_set_deadline(req, jiffies + req->timeout); list_add_tail(&req->timeout_list, &req->q->timeout_list); + } else { + WARN_ON_ONCE(!blk_mq_rq_set_deadline(req, jiffies + + req->timeout, old, new)); + } /* * If the timer isn't already pending or this timeout is earlier @@ -249,3 +245,34 @@ void blk_add_timer(struct request *req) } } + +/** + * blk_add_timer - Start timeout timer for a single request + * @req: request that is about to start running. + * + * Notes: + * Each request has its own timer, and as it is added to the queue, we + * set up the timer. When the request completes, we cancel the timer. + */ +void blk_add_timer(struct request *req) +{ + return __blk_add_timer(req, MQ_RQ_IDLE/*ignored*/, + MQ_RQ_IDLE/*ignored*/); +} + +/** + * blk_mq_add_timer - set the deadline for a single request + * @req: request for which to set the deadline. + * @old: current request state. + * @new: new request state. + * + * Sets the deadline of a request if and only if it has state @old and + * at the same time changes the request state from @old into @new. The caller + * must guarantee that the request state won't be modified while this function + * is in progress. + */ +void blk_mq_add_timer(struct request *req, enum mq_rq_state old, + enum mq_rq_state new) +{ + return __blk_add_timer(req, old, new); +} diff --git a/block/blk.h b/block/blk.h index b034fd2460c4..7665d4af777e 100644 --- a/block/blk.h +++ b/block/blk.h @@ -170,6 +170,8 @@ static inline bool bio_integrity_endio(struct bio *bio) void blk_timeout_work(struct work_struct *work); unsigned long blk_rq_timeout(unsigned long timeout); void blk_add_timer(struct request *req); +void blk_mq_add_timer(struct request *req, enum mq_rq_state old, + enum mq_rq_state new); void blk_delete_timer(struct request *); @@ -191,21 +193,21 @@ void blk_account_io_done(struct request *req); /* * EH timer and IO completion will both attempt to 'grab' the request, make * sure that only one of them succeeds. Steal the bottom bit of the - * __deadline field for this. + * lq_deadline field for this. */ static inline int blk_mark_rq_complete(struct request *rq) { - return test_and_set_bit(0, &rq->__deadline); + return test_and_set_bit(0, &rq->lq_deadline); } static inline void blk_clear_rq_complete(struct request *rq) { - clear_bit(0, &rq->__deadline); + clear_bit(0, &rq->lq_deadline); } static inline bool blk_rq_is_complete(struct request *rq) { - return test_bit(0, &rq->__deadline); + return test_bit(0, &rq->lq_deadline); } /* @@ -311,15 +313,42 @@ static inline void req_set_nomerge(struct request_queue *q, struct request *req) * Steal a bit from this field for legacy IO path atomic IO marking. Note that * setting the deadline clears the bottom bit, potentially clearing the * completed bit. The user has to be OK with this (current ones are fine). + * Must be called with the request queue lock held. */ static inline void blk_rq_set_deadline(struct request *rq, unsigned long time) { - rq->__deadline = time & ~0x1UL; + rq->lq_deadline = time & ~0x1UL; } static inline unsigned long blk_rq_deadline(struct request *rq) { - return rq->__deadline & ~0x1UL; + return rq->lq_deadline & ~0x1UL; +} + +/* + * If the state of request @rq equals @old_s, update deadline and request state + * atomically to @time and @new_s. blk-mq only. + */ +static inline bool blk_mq_rq_set_deadline(struct request *rq, + unsigned long time, + enum mq_rq_state old_s, + enum mq_rq_state new_s) +{ + unsigned long old_d, new_d; + + do { + old_d = atomic_long_read(&rq->mq_deadline); + if ((old_d & MQ_RQ_STATE_MASK) != old_s) + return false; + new_d = (time & ~0x3UL) | (new_s & 3UL); + } while (atomic_long_cmpxchg(&rq->mq_deadline, old_d, new_d) != old_d); + + return true; +} + +static inline unsigned long blk_mq_rq_deadline(struct request *rq) +{ + return atomic_long_read(&rq->mq_deadline) & ~0x3UL; } /* diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h index 8efcf49796a3..13ccbb418e89 100644 --- a/include/linux/blk-mq.h +++ b/include/linux/blk-mq.h @@ -51,7 +51,6 @@ struct blk_mq_hw_ctx { unsigned int queue_num; atomic_t nr_active; - unsigned int nr_expired; struct hlist_node cpuhp_dead; struct kobject kobj; diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 6075d1a6760c..abf78819014b 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -27,7 +27,6 @@ #include #include #include -#include #include struct module; @@ -125,8 +124,6 @@ typedef __u32 __bitwise req_flags_t; #define RQF_SPECIAL_PAYLOAD ((__force req_flags_t)(1 << 18)) /* The per-zone write lock is held for this request */ #define RQF_ZONE_WRITE_LOCKED ((__force req_flags_t)(1 << 19)) -/* timeout is expired */ -#define RQF_MQ_TIMEOUT_EXPIRED ((__force req_flags_t)(1 << 20)) /* already slept for hybrid poll */ #define RQF_MQ_POLL_SLEPT ((__force req_flags_t)(1 << 21)) @@ -226,28 +223,15 @@ struct request { unsigned int extra_len; /* length of alignment and padding */ /* - * On blk-mq, the lower bits of ->gstate (generation number and - * state) carry the MQ_RQ_* state value and the upper bits the - * generation number which is monotonically incremented and used to - * distinguish the reuse instances. - * - * ->gstate_seq allows updates to ->gstate and other fields - * (currently ->deadline) during request start to be read - * atomically from the timeout path, so that it can operate on a - * coherent set of information. + * Access through blk_rq_set_deadline(), blk_rq_deadline() and + * blk_mark_rq_complete(), blk_clear_rq_complete() and + * blk_rq_is_complete() for legacy queues or blk_mq_rq_set_deadline(), + * blk_mq_rq_deadline() and blk_mq_rq_state() for blk-mq queues. */ - seqcount_t gstate_seq; - u64 gstate; - - /* - * ->aborted_gstate is used by the timeout to claim a specific - * recycle instance of this request. See blk_mq_timeout_work(). - */ - struct u64_stats_sync aborted_gstate_sync; - u64 aborted_gstate; - - /* access through blk_rq_set_deadline, blk_rq_deadline */ - unsigned long __deadline; + union { + unsigned long lq_deadline; + atomic_long_t mq_deadline; + }; struct list_head timeout_list;