From patchwork Wed Apr 5 18:33:14 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: 9665423 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 3352C60353 for ; Wed, 5 Apr 2017 18:33:19 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2AE2B28178 for ; Wed, 5 Apr 2017 18:33:19 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1FC63285A3; Wed, 5 Apr 2017 18:33:19 +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 4EE2128178 for ; Wed, 5 Apr 2017 18:33:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932443AbdDESdR (ORCPT ); Wed, 5 Apr 2017 14:33:17 -0400 Received: from esa5.hgst.iphmx.com ([216.71.153.144]:60712 "EHLO esa5.hgst.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755283AbdDESdR (ORCPT ); Wed, 5 Apr 2017 14:33:17 -0400 X-IronPort-AV: E=Sophos;i="5.37,279,1488816000"; d="scan'208";a="8451182" Received: from mail-dm3nam03lp0023.outbound.protection.outlook.com (HELO NAM03-DM3-obe.outbound.protection.outlook.com) ([207.46.163.23]) by ob1.hgst.iphmx.com with ESMTP; 06 Apr 2017 02:33:16 +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=hnLg/rsdOJWsoDX28Wvn9A1GT5nv6qzx9zfXHiV+EiE=; b=RQit18CHPjmLlLE4/Iz7PXfvvdaQX8MRfY8VaCC/7tISnOlaSzW+io6dFudS+CP/FOey3gIueKoxxDenMl7wb+yde+GoTteYy98I9qcXDpp1GFFvac6BE5dUx+ESWnNr6rxH0lYsOTS7TKeaONRwHhM/uwt0XdvriZf6Y7/wnyE= 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.1005.10; Wed, 5 Apr 2017 18:33:14 +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.1005.020; Wed, 5 Apr 2017 18:33:14 +0000 From: Bart Van Assche To: "osandov@osandov.com" , "linux-block@vger.kernel.org" , "axboe@fb.com" CC: "kernel-team@fb.com" Subject: Re: [PATCH v2 1/8] blk-mq: use the right hctx when getting a driver tag fails Thread-Topic: [PATCH v2 1/8] blk-mq: use the right hctx when getting a driver tag fails Thread-Index: AQHSrjqQ3viBPBMTOk2JOq4h7M8UlqG3GbQA Date: Wed, 5 Apr 2017 18:33:14 +0000 Message-ID: <1491417192.2787.11.camel@sandisk.com> References: <89da4a6561df3e24af3ba1c8625470d3088d2fa1.1491416593.git.osandov@fb.com> In-Reply-To: <89da4a6561df3e24af3ba1c8625470d3088d2fa1.1491416593.git.osandov@fb.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: osandov.com; dkim=none (message not signed) header.d=none; osandov.com; dmarc=none action=none header.from=sandisk.com; x-ms-exchange-messagesentrepresentingtype: 1 x-originating-ip: [63.163.107.100] x-microsoft-exchange-diagnostics: 1; CY1PR0401MB1534; 7:u34ptBPX2eNt9yc5Ea2npBbPGLgbiJ9BBCMwTR+SCTMjIqplG4SS8jFtbQMkYzFT6cOfv8w2wQH20ZL6gNGbkWNxQ5Kp0vw5CHWyvatZ05uYttakNo0iqwVQDOnRhAnQ4/yXRBc/3yDzNQbDe6jPDk5GRTUVShlLoZMDsq6soMAmIk+1MhonyABe11t/s3nuWgiNYzJHrICAXdDomNkm7I1mAMghRNohMc1oP5TugXdgVf0Ueq3Zd0iozfUJ5fNpvJGvsQgmoJh64UF/slZgYCJg/sHqu5zTN6gTsCy/gbQQxJqMOar/9oPgwz0I034qBDYS9usbjANu7t7/aXHicw==; 20:NcuNJzApbkvRnbpcULAINpEX+eMW4Jgcqx8sAj2maBVBFcgzI969LBFtC7Bc5bUGtftU4AeGrgR/R14tAA/z1eJGr++DTdLTP5jzf/jJ2sxGL5s++mfkKFEJhKwWAwNPNA/sYKSrgcjw5pNt0RafnPNLSVzsyseL9iYE6WeThBQ= x-ms-office365-filtering-correlation-id: 1aaaabeb-f6c8-4afc-f7f6-08d47c523862 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:(67672495146484)(42932892334569); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040450)(601004)(2401047)(5005006)(8121501046)(93006095)(93001095)(10201501046)(3002001)(6055026)(6041248)(20161123560025)(20161123564025)(20161123555025)(201703131423075)(201702281528075)(201703061421075)(20161123562025)(6072148); SRVR:CY1PR0401MB1534; BCL:0; PCL:0; RULEID:; SRVR:CY1PR0401MB1534; x-forefront-prvs: 0268246AE7 x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(6009001)(39860400002)(39840400002)(39450400003)(39850400002)(39400400002)(39410400002)(377424004)(24454002)(7736002)(2900100001)(25786009)(3280700002)(81166006)(3846002)(102836003)(6116002)(3660700001)(103116003)(122556002)(8676002)(99286003)(36756003)(77096006)(6436002)(6486002)(6512007)(305945005)(66066001)(6506006)(8936002)(76176999)(54356999)(50986999)(5660300001)(229853002)(53936002)(4326008)(189998001)(2201001)(575784001)(38730400002)(6246003)(2950100002)(2906002)(86362001)(33646002)(2501003); 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 Content-ID: MIME-Version: 1.0 X-OriginatorOrg: sandisk.com X-MS-Exchange-CrossTenant-originalarrivaltime: 05 Apr 2017 18:33:14.0424 (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-block-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, 2017-04-05 at 11:28 -0700, Omar Sandoval wrote: > From: Omar Sandoval > > While dispatching requests, if we fail to get a driver tag, we mark the > hardware queue as waiting for a tag and put the requests on a > hctx->dispatch list to be run later when a driver tag is freed. However, > blk_mq_dispatch_rq_list() may dispatch requests from multiple hardware > queues if using a single-queue scheduler with a multiqueue device. If > blk_mq_get_driver_tag() fails, it doesn't update the hardware queue we > are processing. This means we end up using the hardware queue of the > previous request, which may or may not be the same as that of the > current request. If it isn't, the wrong hardware queue will end up > waiting for a tag, and the requests will be on the wrong dispatch list, > leading to a hang. > > The fix is twofold: > > 1. Make sure we save which hardware queue we were trying to get a > request for in blk_mq_get_driver_tag() regardless of whether it > succeeds or not. > 2. Make blk_mq_dispatch_rq_list() take a request_queue instead of a > blk_mq_hw_queue to make it clear that it must handle multiple > hardware queues, since I've already messed this up on a couple of > occasions. > > This didn't appear in testing with nvme and mq-deadline because nvme has > more driver tags than the default number of scheduler tags. However, > with the blk_mq_update_nr_hw_queues() fix, it showed up with nbd. Would the patch below be a valid alternative? Thanks, Bart. [PATCH] blk-mq: Simplify blk_mq_get_driver_tag() The blk_mq_get_driver_tag() callers either assume that *hctx is not modified or that it points to a valid hctx pointer upon return if tag allocation succeeded. Avoid this confusion by returning the hctx pointer if and only if tag allocation succeeded and by only storing the return value into hctx in those blk_mq_get_driver_tag() callers for which the hctx pointer had not yet been computed before the blk_mq_get_driver_tag() call. Signed-off-by: Bart Van Assche ---  block/blk-mq-sched.c |  4 +++-  block/blk-mq.c       | 24 ++++++++++--------------  block/blk-mq.h       |  3 +--  3 files changed, 14 insertions(+), 17 deletions(-) --  2.12.0 diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c index a85d939ef450..bfb8bdb95a87 100644 --- a/block/blk-mq-sched.c +++ b/block/blk-mq-sched.c @@ -386,7 +386,9 @@ void blk_mq_sched_restart_a_queue(struct blk_mq_hw_ctx *hctx)  static void blk_mq_sched_insert_flush(struct blk_mq_hw_ctx *hctx,         struct request *rq, bool can_block)  { - if (blk_mq_get_driver_tag(rq, &hctx, can_block)) { + WARN_ON_ONCE(!hctx); + + if (blk_mq_get_driver_tag(rq, can_block)) {   blk_insert_flush(rq);   blk_mq_run_hw_queue(hctx, true);   } else diff --git a/block/blk-mq.c b/block/blk-mq.c index 45e7f597cea3..c8e0c02dc8ca 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -857,8 +857,7 @@ static inline unsigned int queued_to_index(unsigned int queued)   return min(BLK_MQ_MAX_DISPATCH_ORDER - 1, ilog2(queued) + 1);  }   -bool blk_mq_get_driver_tag(struct request *rq, struct blk_mq_hw_ctx **hctx, -    bool wait) +struct blk_mq_hw_ctx *blk_mq_get_driver_tag(struct request *rq, bool wait)  {   struct blk_mq_alloc_data data = {   .q = rq->q, @@ -866,12 +865,8 @@ bool blk_mq_get_driver_tag(struct request *rq, struct blk_mq_hw_ctx **hctx,   .flags = wait ? 0 : BLK_MQ_REQ_NOWAIT,   };   - if (rq->tag != -1) { -done: - if (hctx) - *hctx = data.hctx; - return true; - } + if (rq->tag != -1) + return data.hctx;     if (blk_mq_tag_is_reserved(data.hctx->sched_tags, rq->internal_tag))   data.flags |= BLK_MQ_REQ_RESERVED; @@ -883,10 +878,10 @@ bool blk_mq_get_driver_tag(struct request *rq, struct blk_mq_hw_ctx **hctx,   atomic_inc(&data.hctx->nr_active);   }   data.hctx->tags->rqs[rq->tag] = rq; - goto done; + return data.hctx;   }   - return false; + return NULL;  }    static void blk_mq_put_driver_tag_hctx(struct blk_mq_hw_ctx *hctx, @@ -985,7 +980,7 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list)   struct blk_mq_queue_data bd;     rq = list_first_entry(list, struct request, queuelist); - if (!blk_mq_get_driver_tag(rq, &hctx, false)) { + if (!blk_mq_get_driver_tag(rq, false)) {   if (!queued && reorder_tags_to_front(list))   continue;   @@ -999,7 +994,7 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list)    * window between the allocation failure and    * adding the hardware queue to the wait queue.    */ - if (!blk_mq_get_driver_tag(rq, &hctx, false)) + if (!blk_mq_get_driver_tag(rq, false))   break;   } else {   break; @@ -1020,7 +1015,7 @@ bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *hctx, struct list_head *list)   struct request *nxt;     nxt = list_first_entry(list, struct request, queuelist); - bd.last = !blk_mq_get_driver_tag(nxt, NULL, false); + bd.last = !blk_mq_get_driver_tag(nxt, false);   }     ret = q->mq_ops->queue_rq(hctx, &bd); @@ -1435,7 +1430,8 @@ static void __blk_mq_try_issue_directly(struct request *rq, blk_qc_t *cookie,   if (q->elevator)   goto insert;   - if (!blk_mq_get_driver_tag(rq, &hctx, false)) + hctx = blk_mq_get_driver_tag(rq, false); + if (!hctx)   goto insert;     new_cookie = request_to_qc_t(hctx, rq); diff --git a/block/blk-mq.h b/block/blk-mq.h index 8d49c06fc520..b1917fbe955c 100644 --- a/block/blk-mq.h +++ b/block/blk-mq.h @@ -33,8 +33,7 @@ void blk_mq_wake_waiters(struct request_queue *q);  bool blk_mq_dispatch_rq_list(struct blk_mq_hw_ctx *, struct list_head *);  void blk_mq_flush_busy_ctxs(struct blk_mq_hw_ctx *hctx, struct list_head *list);  bool blk_mq_hctx_has_pending(struct blk_mq_hw_ctx *hctx); -bool blk_mq_get_driver_tag(struct request *rq, struct blk_mq_hw_ctx **hctx, - bool wait); +struct blk_mq_hw_ctx *blk_mq_get_driver_tag(struct request *rq, bool wait);    /*   * Internal helpers for allocating/freeing the request map