From patchwork Mon Feb 27 16:15:27 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jens Axboe X-Patchwork-Id: 9593551 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 5551760578 for ; Mon, 27 Feb 2017 16:15:48 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4738B2838E for ; Mon, 27 Feb 2017 16:15:48 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3BF2E28487; Mon, 27 Feb 2017 16:15:48 +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.4 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM 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 9F91A2848D for ; Mon, 27 Feb 2017 16:15:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751338AbdB0QPo (ORCPT ); Mon, 27 Feb 2017 11:15:44 -0500 Received: from mail-io0-f175.google.com ([209.85.223.175]:35247 "EHLO mail-io0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751387AbdB0QPg (ORCPT ); Mon, 27 Feb 2017 11:15:36 -0500 Received: by mail-io0-f175.google.com with SMTP id j18so34552190ioe.2 for ; Mon, 27 Feb 2017 08:15:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel-dk.20150623.gappssmtp.com; s=20150623; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=YNieWQO5K8j7sUY99DDf0XuOWrmkyJU3GmKSs8Vwc1Y=; b=X05TPg/ns/Cg8rLjXyKx4UIwDOVLd5v+/4T2QjAHuUHf0BGLClFfJiGKwSsXaDMmFy xQGG53IZoF1GfMec/gnQUDHHUi7d1hHGX+LYOtTTf9X6/QfPuZsFLT1+apHeeE1p90yP AaetJOL8a/xAXDdUdD3lXQ3mAnaSTxOFFROYNA/UgCRjLg8tLHEuHdvrLnz/v2c2nSnm MG+I4THiHNMbW4Ol1+KNsialCdxOh66o7V1M4nNVQCDhAgQgKtxX/2t9ZTBcxZuskdrY ig5QAkYXta1KDwqF9qTDkk+0FJTMGG55XoEnWz7ZeWePUN5AyOtGTqG/fl7K47jpUTQu S0qQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=YNieWQO5K8j7sUY99DDf0XuOWrmkyJU3GmKSs8Vwc1Y=; b=jrZADMAboZNDFfGAemDse7HrHGwZObWXRlq/3FVrt7bShVQG6dZQRBu6OSg2SbXTFG u0b9XLjIozn9YkaN41A2WkD6F/ZwABZrf7tm/ekjEdskPByhPbiu+XUWFQrwrwIB9Mn/ JPCHMQXme1Akw/qg1bEGqs2O0nd9YKX/crHCFPJAgNF93OYKtJ3baEEkMAkGafRpEUck A8Cr123/ONEzYUHwiE8X1co6vyI5sofVjIvcXCN15JDYdsrv7p+h7Yf9FNgJjMnZvHgq yapDrAWw9OhhvLiHLDCT3Bnz9NJYFbUxpCYB4soji+3zw6ArIWR/Te33qq8nlyjpGZZN od5Q== X-Gm-Message-State: AMke39nj5wzOrr71HTJllqM/HdwUY1LLkFaGt0XZ0ytaBIYZChIGU27CjTmenfnYtOpLGA== X-Received: by 10.107.9.28 with SMTP id j28mr14471342ioi.132.1488212128614; Mon, 27 Feb 2017 08:15:28 -0800 (PST) Received: from [192.168.1.154] ([216.160.245.98]) by smtp.gmail.com with ESMTPSA id n6sm4462077itb.25.2017.02.27.08.15.27 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 27 Feb 2017 08:15:27 -0800 (PST) Subject: Re: [PATCH 1/2] blk-mq-sched: Allocate sched reserved tags as specified in the original queue tagset To: Sagi Grimberg , Omar Sandoval References: <1488209781-1084-1-git-send-email-sagi@grimberg.me> <20170227154939.GA10715@vader> <4829b70c-53fd-f622-310e-0a30e4ede6fa@kernel.dk> Cc: linux-block@vger.kernel.org, linux-nvme@lists.infradead.org From: Jens Axboe Message-ID: <353bb44b-7180-7a78-47f5-2ebf03b714bc@kernel.dk> Date: Mon, 27 Feb 2017 09:15:27 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: 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 02/27/2017 09:10 AM, Sagi Grimberg wrote: > >>> Hm, this may fix the crash, but I'm not sure it'll work as intended. >>> When we allocate the request, we'll get a reserved scheduler tag, but >>> then when we go to dispatch the request and call >>> blk_mq_get_driver_tag(), we'll be competing with all of the normal >>> requests for a regular driver tag. So maybe on top of this we should add >>> the BLK_MQ_REQ_RESERVED flag to the allocation attempt in >>> blk_mq_get_driver_tag() if the scheduler tag is reserved? I'm hazy on >>> what we expect from reserved tags, so feel free to call me crazy. >> >> Yeah good point, we need to carry it through. Reserved tags exist >> because drivers often need a request/tag for error handling. If all >> tags currently are used up for regular IO that is stuck, you need >> a reserved tag for error handling to guarantee progress. >> >> So Sagi's patch does take it half the way there, but get_driver_tag >> really needs to know about this as well, or we will just get stuck >> there as well. Two solutions, I can think of: >> >> 1) Check the tag value in get_driver_tag, add BLK_MQ_REQ_RESERVED >> when allocating a driver tag if above X. >> 2) Add an RQF_SOMETHING_RESERVED. Add BLK_MQ_REQ_RESERVED in >> get_driver_tag if that is set. >> >> Comments? > > Can't we just not go through the scheduler for reserved tags? Obviously > there is no point in scheduling them... Right, that would be possible. But I'd rather not treat any requests differently, it's a huge pain in the ass that flush request currently insert with a driver tag already allocated. So it's not because scheduling will add anything at all, it's more that I'd like to move flush requests to use regular inserts as well and not deal with some request being "special" in any way. The below should hopefully work. Totally untested... diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c index 54c84363c1b2..e48bc2c72615 100644 --- a/block/blk-mq-tag.c +++ b/block/blk-mq-tag.c @@ -181,7 +181,7 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data) void blk_mq_put_tag(struct blk_mq_hw_ctx *hctx, struct blk_mq_tags *tags, struct blk_mq_ctx *ctx, unsigned int tag) { - if (tag >= tags->nr_reserved_tags) { + if (!blk_mq_tag_is_reserved(tags, tag)) { const int real_tag = tag - tags->nr_reserved_tags; BUG_ON(real_tag >= tags->nr_tags); diff --git a/block/blk-mq-tag.h b/block/blk-mq-tag.h index 63497423c5cd..5cb51e53cc03 100644 --- a/block/blk-mq-tag.h +++ b/block/blk-mq-tag.h @@ -85,4 +85,10 @@ static inline void blk_mq_tag_set_rq(struct blk_mq_hw_ctx *hctx, hctx->tags->rqs[tag] = rq; } +static inline bool blk_mq_tag_is_reserved(struct blk_mq_tags *tags, + unsigned int tag) +{ + return tag < tags->nr_reserved_tags; +} + #endif diff --git a/block/blk-mq.c b/block/blk-mq.c index 9611cd9920e9..293e79c1ee95 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -853,6 +853,9 @@ bool blk_mq_get_driver_tag(struct request *rq, struct blk_mq_hw_ctx **hctx, return true; } + if (blk_mq_tag_is_reserved(data.hctx->sched_tags, rq->internal_tag)) + data.flags |= BLK_MQ_REQ_RESERVED; + rq->tag = blk_mq_get_tag(&data); if (rq->tag >= 0) { if (blk_mq_tag_busy(data.hctx)) {