From patchwork Mon Apr 20 16:24:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 11499321 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A110013B2 for ; Mon, 20 Apr 2020 16:25:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8982D2082E for ; Mon, 20 Apr 2020 16:25:25 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="oLc3AJ2z" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726972AbgDTQZZ (ORCPT ); Mon, 20 Apr 2020 12:25:25 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39438 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1725958AbgDTQZY (ORCPT ); Mon, 20 Apr 2020 12:25:24 -0400 Received: from mail-pl1-x641.google.com (mail-pl1-x641.google.com [IPv6:2607:f8b0:4864:20::641]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 97D22C061A10 for ; Mon, 20 Apr 2020 09:25:24 -0700 (PDT) Received: by mail-pl1-x641.google.com with SMTP id n24so4115996plp.13 for ; Mon, 20 Apr 2020 09:25:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=5/HuxgmmHlqReW6h8RDiDPaUU+foeFL9gMHI9mR80zQ=; b=oLc3AJ2zDSkJUS41so2hjSzJWNKd+wQTQRY105W2+x1kDo67B9HBOmeWMj71MixgMx Or9Sk5sj3rNm2US+gKQTp3Pa4AgxhFV4SqxT7+A6HxzfaKFMFsWyQbXAeSfmPoABqpNl Co26ffksskFcKs3YvP8b3uw/2SKxJVLuMvcn8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=5/HuxgmmHlqReW6h8RDiDPaUU+foeFL9gMHI9mR80zQ=; b=b6Tmg2ww1j/6K+kGdIxZhFWWCmKd9Vph5vMlOlGPqo8cAlV73qrxnqDm8VnArqYXU2 FcnafoEFWbBzuV8HvwHNxDIo/KBCmIX8aaW1Go24p9TODFBEufwSTAJQSfPTK6iHl+nb 7l+9qFfEfzyd6U2k/31kZeSY/bL7cq1v73erCY3Lr1Dbe9HEbDRniGCWKfh0MHVL87cV i19n81If7TDxp+nJeSgp5dbzBjWYXVgTHqi2dxLXwhpIO9rbQT3Fv6cmApG+VEwCQFAc ckowRHLc4AX9Worx4qpirHvfg1lGpnWttDi4iXjZL+6f+JtLDJWhqN41JySD1kcx+RdE Q2+Q== X-Gm-Message-State: AGi0PuZNVnY53dx7t767sfpfoZUDO3y49Ug7+5Nk79V/Y2zCK3jGt3Bt pfyMS0kZhAXVL0k07NPHYMSNiw== X-Google-Smtp-Source: APiQypKpCrNnEeTYcINBy79o3eKcejCinyX6OW/b2TJcQxL+18Xe1Dv9nBf3MYUu730zR6e0r+X2hA== X-Received: by 2002:a17:90a:710a:: with SMTP id h10mr213647pjk.152.1587399924208; Mon, 20 Apr 2020 09:25:24 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:24fa:e766:52c9:e3b2]) by smtp.gmail.com with ESMTPSA id p64sm93150pjp.7.2020.04.20.09.25.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Apr 2020 09:25:23 -0700 (PDT) From: Douglas Anderson To: axboe@kernel.dk, jejb@linux.ibm.com, martin.petersen@oracle.com Cc: linux-scsi@vger.kernel.org, Gwendal Grignou , sqazi@google.com, groeck@chromium.org, Ming Lei , linux-block@vger.kernel.org, paolo.valente@linaro.org, Douglas Anderson , linux-kernel@vger.kernel.org Subject: [PATCH v5 1/4] blk-mq: In blk_mq_dispatch_rq_list() "no budget" is a reason to kick Date: Mon, 20 Apr 2020 09:24:51 -0700 Message-Id: <20200420092357.v5.1.I1f95c459e51962b8d2c83e869913b6befda2255c@changeid> X-Mailer: git-send-email 2.26.1.301.g55bc3eb7cb9-goog In-Reply-To: <20200420162454.48679-1-dianders@chromium.org> References: <20200420162454.48679-1-dianders@chromium.org> MIME-Version: 1.0 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org In blk_mq_dispatch_rq_list(), if blk_mq_sched_needs_restart() returns true and the driver returns BLK_STS_RESOURCE then we'll kick the queue. However, there's another case where we might need to kick it. If we were unable to get budget we can be in much the same state as when the driver returns BLK_STS_RESOURCE, so we should treat it the same. It should be noted that even if we add a whole bunch of extra kicking to the queue in other patches this patch is still important. Specifically any kicking that happened before we re-spliced leftover requests into 'hctx->dispatch' wouldn't have found any work, so we really need to make sure we kick ourselves after we've done the splicing. Signed-off-by: Douglas Anderson Reviewed-by: Ming Lei --- Changes in v5: - Rebase atop commit 5fe56de799ad ("...Put driver tag...when no budget") Changes in v4: None Changes in v3: - Note why blk_mq_dispatch_rq_list() change is needed. Changes in v2: None block/blk-mq.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/block/blk-mq.c b/block/blk-mq.c index a7785df2c944..1c4bedf500c5 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -1206,6 +1206,7 @@ bool blk_mq_dispatch_rq_list(struct request_queue *q, struct list_head *list, bool no_tag = false; int errors, queued; blk_status_t ret = BLK_STS_OK; + bool no_budget_avail = false; if (list_empty(list)) return false; @@ -1224,6 +1225,7 @@ bool blk_mq_dispatch_rq_list(struct request_queue *q, struct list_head *list, hctx = rq->mq_hctx; if (!got_budget && !blk_mq_get_dispatch_budget(hctx)) { blk_mq_put_driver_tag(rq); + no_budget_avail = true; break; } @@ -1320,13 +1322,15 @@ bool blk_mq_dispatch_rq_list(struct request_queue *q, struct list_head *list, * * If driver returns BLK_STS_RESOURCE and SCHED_RESTART * bit is set, run queue after a delay to avoid IO stalls - * that could otherwise occur if the queue is idle. + * that could otherwise occur if the queue is idle. We'll do + * similar if we couldn't get budget and SCHED_RESTART is set. */ needs_restart = blk_mq_sched_needs_restart(hctx); if (!needs_restart || (no_tag && list_empty_careful(&hctx->dispatch_wait.entry))) blk_mq_run_hw_queue(hctx, true); - else if (needs_restart && (ret == BLK_STS_RESOURCE)) + else if (needs_restart && (ret == BLK_STS_RESOURCE || + no_budget_avail)) blk_mq_delay_run_hw_queue(hctx, BLK_MQ_RESOURCE_DELAY); blk_mq_update_dispatch_busy(hctx, true); From patchwork Mon Apr 20 16:24:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 11499333 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 86E9F1575 for ; Mon, 20 Apr 2020 16:25:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6F6C020B1F for ; Mon, 20 Apr 2020 16:25:44 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="KUBJDxas" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728879AbgDTQZ2 (ORCPT ); Mon, 20 Apr 2020 12:25:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39438 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1728433AbgDTQZZ (ORCPT ); Mon, 20 Apr 2020 12:25:25 -0400 Received: from mail-pj1-x1042.google.com (mail-pj1-x1042.google.com [IPv6:2607:f8b0:4864:20::1042]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DBD9CC061A0C for ; Mon, 20 Apr 2020 09:25:25 -0700 (PDT) Received: by mail-pj1-x1042.google.com with SMTP id ng8so74377pjb.2 for ; Mon, 20 Apr 2020 09:25:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=sJb/FZkBYSseiSKNzzWagXNE/s2TMt80AisrI4SdHXg=; b=KUBJDxasRKeNRfT7LjBcYw8jIuMRS8knhr1jWgdF5aQN6fN8PPTIUhvHLDOfDDuwbn VFfvRCq+JaHti7X9smTvMSfDYe2LfpA+OE0FyosSdWFCSMXr0uox0foWB1qm0Gz69CmX 9X3o+uDxr0q3mMra9qG4Jm+7h5x3WC2SPvu5s= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=sJb/FZkBYSseiSKNzzWagXNE/s2TMt80AisrI4SdHXg=; b=H9xsj4UphMCF9EcUzLbtdKrF50m31nT4iVvhdIZLMPyBiMIz8ZI9l4eBcIHCvicL17 bvQug2uzKskw5nCXXs8Xc8FcpkR6asyCbPe/nGZbPc4TXKViEkcigpRRdGTTuIGIAfSZ D+ckBzaCOidMVsruQJptjb95fOSPkCrHo1E/Cfk+ZMd1IXsYbiEszXt+f9Wt55VYlDmY W+5HWF5sMY4U2G2dt3KoOMdzv24AA2qNwsjkT5eZNkRdm1pQsaGYXQZ4l9tjvyu2E5D8 opu4A5LIoEa5zelljyqfDSy80ZEtMMqf0fx1O0xDS2KZt7y5SZLkrbB75pUW5OovSlcM 61LQ== X-Gm-Message-State: AGi0PubiMNvOaohlqmcdW/fAXRQNyfFG2swkhH595J5KpMZB0fnEkQXz 1YllTi44pPbKvwF12L4JceuXXg== X-Google-Smtp-Source: APiQypJs4GaJ6rb7KxgKEfa+hDVAZ8kHOtrz2+nvnHERM7J1Y9Bab5AsRC8ri3uoYqw+0duAqDbWnQ== X-Received: by 2002:a17:90a:270d:: with SMTP id o13mr247209pje.34.1587399925403; Mon, 20 Apr 2020 09:25:25 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:24fa:e766:52c9:e3b2]) by smtp.gmail.com with ESMTPSA id p64sm93150pjp.7.2020.04.20.09.25.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Apr 2020 09:25:24 -0700 (PDT) From: Douglas Anderson To: axboe@kernel.dk, jejb@linux.ibm.com, martin.petersen@oracle.com Cc: linux-scsi@vger.kernel.org, Gwendal Grignou , sqazi@google.com, groeck@chromium.org, Ming Lei , linux-block@vger.kernel.org, paolo.valente@linaro.org, Douglas Anderson , =?utf-8?q?Andr=C3=A9_Almeida?= , Bart Van Assche , Damien Le Moal , John Garry , Pavel Begunkov , Sagi Grimberg , linux-kernel@vger.kernel.org Subject: [PATCH v5 2/4] blk-mq: Add blk_mq_delay_run_hw_queues() API call Date: Mon, 20 Apr 2020 09:24:52 -0700 Message-Id: <20200420092357.v5.2.I4c665d70212a5b33e103fec4d5019a59b4c05577@changeid> X-Mailer: git-send-email 2.26.1.301.g55bc3eb7cb9-goog In-Reply-To: <20200420162454.48679-1-dianders@chromium.org> References: <20200420162454.48679-1-dianders@chromium.org> MIME-Version: 1.0 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org We have: * blk_mq_run_hw_queue() * blk_mq_delay_run_hw_queue() * blk_mq_run_hw_queues() ...but not blk_mq_delay_run_hw_queues(), presumably because nobody needed it before now. Since we need it for a later patch in this series, add it. Signed-off-by: Douglas Anderson Reviewed-by: Ming Lei --- Changes in v5: None Changes in v4: None Changes in v3: - ("blk-mq: Add blk_mq_delay_run_hw_queues() API call") new for v3 Changes in v2: None block/blk-mq.c | 19 +++++++++++++++++++ include/linux/blk-mq.h | 1 + 2 files changed, 20 insertions(+) diff --git a/block/blk-mq.c b/block/blk-mq.c index 1c4bedf500c5..cf95e8e0881a 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -1545,6 +1545,25 @@ void blk_mq_run_hw_queues(struct request_queue *q, bool async) } EXPORT_SYMBOL(blk_mq_run_hw_queues); +/** + * blk_mq_delay_run_hw_queues - Run all hardware queues asynchronously. + * @q: Pointer to the request queue to run. + * @msecs: Microseconds of delay to wait before running the queues. + */ +void blk_mq_delay_run_hw_queues(struct request_queue *q, unsigned long msecs) +{ + struct blk_mq_hw_ctx *hctx; + int i; + + queue_for_each_hw_ctx(q, hctx, i) { + if (blk_mq_hctx_stopped(hctx)) + continue; + + blk_mq_delay_run_hw_queue(hctx, msecs); + } +} +EXPORT_SYMBOL(blk_mq_delay_run_hw_queues); + /** * blk_mq_queue_stopped() - check whether one or more hctxs have been stopped * @q: request queue. diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h index f389d7c724bd..3bbc730eca72 100644 --- a/include/linux/blk-mq.h +++ b/include/linux/blk-mq.h @@ -508,6 +508,7 @@ void blk_mq_unquiesce_queue(struct request_queue *q); void blk_mq_delay_run_hw_queue(struct blk_mq_hw_ctx *hctx, unsigned long msecs); void blk_mq_run_hw_queue(struct blk_mq_hw_ctx *hctx, bool async); void blk_mq_run_hw_queues(struct request_queue *q, bool async); +void blk_mq_delay_run_hw_queues(struct request_queue *q, unsigned long msecs); void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset, busy_tag_iter_fn *fn, void *priv); void blk_mq_tagset_wait_completed_request(struct blk_mq_tag_set *tagset); From patchwork Mon Apr 20 16:24:53 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 11499339 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 4504F1667 for ; Mon, 20 Apr 2020 16:25:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2C33B21927 for ; Mon, 20 Apr 2020 16:25:46 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="WQu1Py4p" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728998AbgDTQZp (ORCPT ); Mon, 20 Apr 2020 12:25:45 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39448 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1726277AbgDTQZ1 (ORCPT ); Mon, 20 Apr 2020 12:25:27 -0400 Received: from mail-pl1-x644.google.com (mail-pl1-x644.google.com [IPv6:2607:f8b0:4864:20::644]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 17713C061A0C for ; Mon, 20 Apr 2020 09:25:27 -0700 (PDT) Received: by mail-pl1-x644.google.com with SMTP id h11so4123158plr.11 for ; Mon, 20 Apr 2020 09:25:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=Ldoi92XzLDL5GmwlBs29GVsxLcqj9XgBbznjrhI/F9w=; b=WQu1Py4pjlKbln+ZwFecz4SGL9jeZQKRrSpk6BMlLDnRY4XKYNmNlwCFzF0FA+lEDc CnZSQ+BSb5GiDXEmKUqmZYNhL9b85SQTnZ0S2c4Z9oaTgVHrGoXeGqHH/qzCJOuZb192 MJR/RT9iydbu4PmoReBPdpB9ycm41+YWcltRo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=Ldoi92XzLDL5GmwlBs29GVsxLcqj9XgBbznjrhI/F9w=; b=cy61EUSWfznQaE0ghQy4j9oOwUz4rUqpT7dq64Hs1WCW6zDNSGfvWLFOLU1n4tLoW0 RjYCDRgw8yZrD6J+uZ75bwTqAasmd9M9Vnuj6Wedu9Q48rc/4ohMVZPbYRndlsmYHHpz 7QXZw52UnGQSnHfhB9hLr/DyYtRQgeC0NBoTlSgnAC8aRT0pAbY/fcN8N4PN+XJlvnVS +deiw2PuHrWamEH+JqM54OoNQmInNYysqifOR+0qsvt9vZFe/7xM8amBBvb4aSLQ2yYx e1X3jFjkUclB2wzFYr+MVxI5e4wwcGc9QECtPA1TrB1ou9884Vt4d/lVJiFmS6Y5V0t4 WUHA== X-Gm-Message-State: AGi0PuY3ab71rXMhAVN8oQdbHzJgXWNyf4XIvPDXzcSRAm5wRgRPvv3K 9WAPtWferca7Q1DiIUZ0PfFYaQ== X-Google-Smtp-Source: APiQypL69kHe0wYkqx9kXC15/zZwy25cVl4zb6IH+uDT4n2qB1qS3Ta/9Wd5uHhPqCX9zHrwvSTNEg== X-Received: by 2002:a17:90a:f985:: with SMTP id cq5mr164679pjb.193.1587399926573; Mon, 20 Apr 2020 09:25:26 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:24fa:e766:52c9:e3b2]) by smtp.gmail.com with ESMTPSA id p64sm93150pjp.7.2020.04.20.09.25.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Apr 2020 09:25:26 -0700 (PDT) From: Douglas Anderson To: axboe@kernel.dk, jejb@linux.ibm.com, martin.petersen@oracle.com Cc: linux-scsi@vger.kernel.org, Gwendal Grignou , sqazi@google.com, groeck@chromium.org, Ming Lei , linux-block@vger.kernel.org, paolo.valente@linaro.org, Douglas Anderson , linux-kernel@vger.kernel.org Subject: [PATCH v5 3/4] blk-mq: Rerun dispatching in the case of budget contention Date: Mon, 20 Apr 2020 09:24:53 -0700 Message-Id: <20200420092357.v5.3.I28278ef8ea27afc0ec7e597752a6d4e58c16176f@changeid> X-Mailer: git-send-email 2.26.1.301.g55bc3eb7cb9-goog In-Reply-To: <20200420162454.48679-1-dianders@chromium.org> References: <20200420162454.48679-1-dianders@chromium.org> MIME-Version: 1.0 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org If ever a thread running blk-mq code tries to get budget and fails it immediately stops doing work and assumes that whenever budget is freed up that queues will be kicked and whatever work the thread was trying to do will be tried again. One path where budget is freed and queues are kicked in the normal case can be seen in scsi_finish_command(). Specifically: - scsi_finish_command() - scsi_device_unbusy() - # Decrement "device_busy", AKA release budget - scsi_io_completion() - scsi_end_request() - blk_mq_run_hw_queues() The above is all well and good. The problem comes up when a thread claims the budget but then releases it without actually dispatching any work. Since we didn't schedule any work we'll never run the path of finishing work / kicking the queues. This isn't often actually a problem which is why this issue has existed for a while and nobody noticed. Specifically we only get into this situation when we unexpectedly found that we weren't going to do any work. Code that later receives new work kicks the queues. All good, right? The problem shows up, however, if timing is just wrong and we hit a race. To see this race let's think about the case where we only have a budget of 1 (only one thread can hold budget). Now imagine that a thread got budget and then decided not to dispatch work. It's about to call put_budget() but then the thread gets context switched out for a long, long time. While in this state, any and all kicks of the queue (like the when we received new work) will be no-ops because nobody can get budget. Finally the thread holding budget gets to run again and returns. All the normal kicks will have been no-ops and we have an I/O stall. As you can see from the above, you need just the right timing to see the race. To start with, the only case it happens if we thought we had work, actually managed to get the budget, but then actually didn't have work. That's pretty rare to start with. Even then, there's usually a very small amount of time between realizing that there's no work and putting the budget. During this small amount of time new work has to come in and the queue kick has to make it all the way to trying to get the budget and fail. It's pretty unlikely. One case where this could have failed is illustrated by an example of threads running blk_mq_do_dispatch_sched(): * Threads A and B both run has_work() at the same time with the same "hctx". Imagine has_work() is exact. There's no lock, so it's OK if Thread A and B both get back true. * Thread B gets interrupted for a long time right after it decides that there is work. Maybe its CPU gets an interrupt and the interrupt handler is slow. * Thread A runs, get budget, dispatches work. * Thread A's work finishes and budget is released. * Thread B finally runs again and gets budget. * Since Thread A already took care of the work and no new work has come in, Thread B will get NULL from dispatch_request(). I believe this is specifically why dispatch_request() is allowed to return NULL in the first place if has_work() must be exact. * Thread B will now be holding the budget and is about to call put_budget(), but hasn't called it yet. * Thread B gets interrupted for a long time (again). Dang interrupts. * Now Thread C (maybe with a different "hctx" but the same queue) comes along and runs blk_mq_do_dispatch_sched(). * Thread C won't do anything because it can't get budget. * Finally Thread B will run again and put the budget without kicking any queues. Even though the example above is with blk_mq_do_dispatch_sched() I believe the race is possible any time someone is holding budget but doesn't do work. Unfortunately, the unlikely has become more likely if you happen to be using the BFQ I/O scheduler. BFQ, by design, sometimes returns "true" for has_work() but then NULL for dispatch_request() and stays in this state for a while (currently up to 9 ms). Suddenly you only need one race to hit, not two races in a row. With my current setup this is easy to reproduce in reboot tests and traces have actually shown that we hit a race similar to the one described above. Note that we only need to fix blk_mq_do_dispatch_sched() and blk_mq_do_dispatch_ctx() and not the other places that put budget. In other cases we know that we have work to do on at least one "hctx" and code already exists to kick that "hctx"'s queue. When that work finally finishes all the queues will be kicked using the normal flow. One last note is that (at least in the SCSI case) budget is shared by all "hctx"s that have the same queue. Thus we need to make sure to kick the whole queue, not just re-run dispatching on a single "hctx". Signed-off-by: Douglas Anderson Reviewed-by: Ming Lei --- Changes in v5: None Changes in v4: - Only kick in blk_mq_do_dispatch_ctx() / blk_mq_do_dispatch_sched(). Changes in v3: - Always kick when putting the budget. - Delay blk_mq_do_dispatch_sched() kick by 3 ms for inexact has_work(). - Totally rewrote commit message. Changes in v2: - Replace ("scsi: core: Fix stall...") w/ ("blk-mq: Rerun dispatch...") block/blk-mq-sched.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/block/blk-mq-sched.c b/block/blk-mq-sched.c index 74cedea56034..eca81bd4010c 100644 --- a/block/blk-mq-sched.c +++ b/block/blk-mq-sched.c @@ -80,6 +80,8 @@ void blk_mq_sched_restart(struct blk_mq_hw_ctx *hctx) blk_mq_run_hw_queue(hctx, true); } +#define BLK_MQ_BUDGET_DELAY 3 /* ms units */ + /* * Only SCSI implements .get_budget and .put_budget, and SCSI restarts * its queue by itself in its completion handler, so we don't need to @@ -103,6 +105,14 @@ static void blk_mq_do_dispatch_sched(struct blk_mq_hw_ctx *hctx) rq = e->type->ops.dispatch_request(hctx); if (!rq) { blk_mq_put_dispatch_budget(hctx); + /* + * We're releasing without dispatching. Holding the + * budget could have blocked any "hctx"s with the + * same queue and if we didn't dispatch then there's + * no guarantee anyone will kick the queue. Kick it + * ourselves. + */ + blk_mq_delay_run_hw_queues(q, BLK_MQ_BUDGET_DELAY); break; } @@ -149,6 +159,14 @@ static void blk_mq_do_dispatch_ctx(struct blk_mq_hw_ctx *hctx) rq = blk_mq_dequeue_from_ctx(hctx, ctx); if (!rq) { blk_mq_put_dispatch_budget(hctx); + /* + * We're releasing without dispatching. Holding the + * budget could have blocked any "hctx"s with the + * same queue and if we didn't dispatch then there's + * no guarantee anyone will kick the queue. Kick it + * ourselves. + */ + blk_mq_delay_run_hw_queues(q, BLK_MQ_BUDGET_DELAY); break; } From patchwork Mon Apr 20 16:24:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 11499329 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id CDA8D1575 for ; Mon, 20 Apr 2020 16:25:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B627E20B1F for ; Mon, 20 Apr 2020 16:25:41 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="bf9D+rR4" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729431AbgDTQZj (ORCPT ); Mon, 20 Apr 2020 12:25:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39448 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1728932AbgDTQZ2 (ORCPT ); Mon, 20 Apr 2020 12:25:28 -0400 Received: from mail-pl1-x643.google.com (mail-pl1-x643.google.com [IPv6:2607:f8b0:4864:20::643]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5301EC061A41 for ; Mon, 20 Apr 2020 09:25:28 -0700 (PDT) Received: by mail-pl1-x643.google.com with SMTP id t4so4118622plq.12 for ; Mon, 20 Apr 2020 09:25:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=Eeish4b0dApiVROXwgSxWPD6R6u9/nVb7Rxz/mEcfI0=; b=bf9D+rR4RVTl/d8EDJ4rebre5pMf0BlvggkGCYG0ek5YLxATE4clEcjGna06GBTP4w 7KzCuHcuZ4k5557dVd5iFpPPe7uodJpjSNuvWbApzF0x+2rntuS/MOdDuyErz1u4A38e nV17RYe/ZjPw4XBZPZnhY2nMCMYSjEly/8cGI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=Eeish4b0dApiVROXwgSxWPD6R6u9/nVb7Rxz/mEcfI0=; b=hJVCoSK73BeWyJo7zY8cjnahmmpUw6QyYKpMM0M91KXtJ2iyLgUfW2vOb7z813vB3b 9jBYYEpkJxzWcjIye9V9LNnCTPIQqStuQs/sDT8jKoG4Pd1XP9eVU2MrGMMdKg6Cyxdj AEe+DZCPWs5ImNu0+iwm/uQuaPeLLvQ8snlAevf7eE8dWR3W9XFd6YzL3Y8tnyyJA5yw 73F+JtX+eQVh+9zpkygywl+MCMCi65RN8yeHoHuDjzmakMhTzUEZ2CfXeA4CdBu8An56 Q4k1PpIUeddqTgvkmWX8Y7mhRHpIKb3KZh0ok/14m2WKmnpZuHZw/WbNy0SLn92bxAmg jbVg== X-Gm-Message-State: AGi0PuZ30+lnivmaNssjVqZGsOGkxwGhS4vhL1NU2wQOpO4yA91uNSsJ VpSasSUvbPFT/uXrtO/XkcQr6g== X-Google-Smtp-Source: APiQypJ2um6WEi/dNweSMis0tXgK55PhuM1j2i0i3wYSHB8seu03qmbLpIwN83SaTaH9ExAeqMD74Q== X-Received: by 2002:a17:90a:1955:: with SMTP id 21mr193496pjh.25.1587399927822; Mon, 20 Apr 2020 09:25:27 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:24fa:e766:52c9:e3b2]) by smtp.gmail.com with ESMTPSA id p64sm93150pjp.7.2020.04.20.09.25.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Apr 2020 09:25:27 -0700 (PDT) From: Douglas Anderson To: axboe@kernel.dk, jejb@linux.ibm.com, martin.petersen@oracle.com Cc: linux-scsi@vger.kernel.org, Gwendal Grignou , sqazi@google.com, groeck@chromium.org, Ming Lei , linux-block@vger.kernel.org, paolo.valente@linaro.org, Douglas Anderson , linux-kernel@vger.kernel.org Subject: [PATCH v5 4/4] Revert "scsi: core: run queue if SCSI device queue isn't ready and queue is idle" Date: Mon, 20 Apr 2020 09:24:54 -0700 Message-Id: <20200420092357.v5.4.I630e6ca4cdcf9ab13ea899274745f9e3174eb12b@changeid> X-Mailer: git-send-email 2.26.1.301.g55bc3eb7cb9-goog In-Reply-To: <20200420162454.48679-1-dianders@chromium.org> References: <20200420162454.48679-1-dianders@chromium.org> MIME-Version: 1.0 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org This reverts commit 7e70aa789d4a0c89dbfbd2c8a974a4df717475ec. Now that we have the patches ("blk-mq: In blk_mq_dispatch_rq_list() "no budget" is a reason to kick") and ("blk-mq: Rerun dispatching in the case of budget contention") we should no longer need the fix in the SCSI code. Revert it, resolving conflicts with other patches that have touched this code. With this revert (and the two new patches) I can run the script that was in commit 7e70aa789d4a ("scsi: core: run queue if SCSI device queue isn't ready and queue is idle") in a loop with no failure. If I do this revert without the two new patches I can easily get a failure. Signed-off-by: Douglas Anderson Reviewed-by: Ming Lei Acked-by: Martin K. Petersen --- Changes in v5: None Changes in v4: None Changes in v3: - ("Revert "scsi: core: run queue...") new for v3. Changes in v2: None drivers/scsi/scsi_lib.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 47835c4b4ee0..ea18f618dc66 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -1610,12 +1610,7 @@ static bool scsi_mq_get_budget(struct blk_mq_hw_ctx *hctx) struct request_queue *q = hctx->queue; struct scsi_device *sdev = q->queuedata; - if (scsi_dev_queue_ready(q, sdev)) - return true; - - if (atomic_read(&sdev->device_busy) == 0 && !scsi_device_blocked(sdev)) - blk_mq_delay_run_hw_queue(hctx, SCSI_QUEUE_DELAY); - return false; + return scsi_dev_queue_ready(q, sdev); } static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,