From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2BF3AC4332F for ; Sun, 18 Dec 2022 16:24:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231970AbiLRQYN (ORCPT ); Sun, 18 Dec 2022 11:24:13 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40218 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231610AbiLRQXC (ORCPT ); Sun, 18 Dec 2022 11:23:02 -0500 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 83B4A13E0C; Sun, 18 Dec 2022 08:09:27 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id A4872B80BA8; Sun, 18 Dec 2022 16:08:46 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 41DB6C433EF; Sun, 18 Dec 2022 16:08:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1671379725; bh=kP1ODcle7cl+j0fYMC+sUTP2LRY6lVqeTfHQaN893tc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=REum8SpxKxO3WPlJ5pRoVj3s7GOo1ApT44CVVVWr4xJ/ictc5IWA7G2oClBnHpzpA Y5qh6FNgk/mUDTwLtir9DyeB3oX8X65jTlO2arM8p3D/XxQrR/3tMJsDoZc6Y27jUL vk9IhvzSGQLqCzwcs4TDQOCurb3OkM1vnVOJhDgbw9rhagPMsJyIaPjRW3YjzGXc5Q AWWKm2K36GWyXjwSensOBmerJVq9DVbtxTBDqfJVlQslzlD9zjoVBgm9Xs15M8yNyE 3NaZa3id+UlK8Ztn/xyMJV6aS2MaRoL2/9BWvb4LVFsU1MhwgRvFfRRBf3iXgwAuOV uznxZMyaBbA7A== From: Sasha Levin To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: David Jeffery , Stefan Hajnoczi , Keith Busch , virtualization@lists.linux-foundation.org, Bart Van Assche , Ming Lei , Jens Axboe , Sasha Levin , linux-block@vger.kernel.org Subject: [PATCH AUTOSEL 6.0 14/73] blk-mq: avoid double ->queue_rq() because of early timeout Date: Sun, 18 Dec 2022 11:06:42 -0500 Message-Id: <20221218160741.927862-14-sashal@kernel.org> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20221218160741.927862-1-sashal@kernel.org> References: <20221218160741.927862-1-sashal@kernel.org> MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: David Jeffery [ Upstream commit 82c229476b8f6afd7e09bc4dc77d89dc19ff7688 ] David Jeffery found one double ->queue_rq() issue, so far it can be triggered in VM use case because of long vmexit latency or preempt latency of vCPU pthread or long page fault in vCPU pthread, then block IO req could be timed out before queuing the request to hardware but after calling blk_mq_start_request() during ->queue_rq(), then timeout handler may handle it by requeue, then double ->queue_rq() is caused, and kernel panic. So far, it is driver's responsibility to cover the race between timeout and completion, so it seems supposed to be solved in driver in theory, given driver has enough knowledge. But it is really one common problem, lots of driver could have similar issue, and could be hard to fix all affected drivers, even it isn't easy for driver to handle the race. So David suggests this patch by draining in-progress ->queue_rq() for solving this issue. Cc: Stefan Hajnoczi Cc: Keith Busch Cc: virtualization@lists.linux-foundation.org Cc: Bart Van Assche Signed-off-by: David Jeffery Signed-off-by: Ming Lei Reviewed-by: Bart Van Assche Link: https://lore.kernel.org/r/20221026051957.358818-1-ming.lei@redhat.com Signed-off-by: Jens Axboe Signed-off-by: Sasha Levin --- block/blk-mq.c | 56 +++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 12 deletions(-) diff --git a/block/blk-mq.c b/block/blk-mq.c index 3f1f5e3e0951..1a30f6580274 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -1442,7 +1442,13 @@ static void blk_mq_rq_timed_out(struct request *req) blk_add_timer(req); } -static bool blk_mq_req_expired(struct request *rq, unsigned long *next) +struct blk_expired_data { + bool has_timedout_rq; + unsigned long next; + unsigned long timeout_start; +}; + +static bool blk_mq_req_expired(struct request *rq, struct blk_expired_data *expired) { unsigned long deadline; @@ -1452,13 +1458,13 @@ static bool blk_mq_req_expired(struct request *rq, unsigned long *next) return false; deadline = READ_ONCE(rq->deadline); - if (time_after_eq(jiffies, deadline)) + if (time_after_eq(expired->timeout_start, deadline)) return true; - if (*next == 0) - *next = deadline; - else if (time_after(*next, deadline)) - *next = deadline; + if (expired->next == 0) + expired->next = deadline; + else if (time_after(expired->next, deadline)) + expired->next = deadline; return false; } @@ -1472,7 +1478,7 @@ void blk_mq_put_rq_ref(struct request *rq) static bool blk_mq_check_expired(struct request *rq, void *priv) { - unsigned long *next = priv; + struct blk_expired_data *expired = priv; /* * blk_mq_queue_tag_busy_iter() has locked the request, so it cannot @@ -1481,7 +1487,18 @@ static bool blk_mq_check_expired(struct request *rq, void *priv) * it was completed and reallocated as a new request after returning * from blk_mq_check_expired(). */ - if (blk_mq_req_expired(rq, next)) + if (blk_mq_req_expired(rq, expired)) { + expired->has_timedout_rq = true; + return false; + } + return true; +} + +static bool blk_mq_handle_expired(struct request *rq, void *priv) +{ + struct blk_expired_data *expired = priv; + + if (blk_mq_req_expired(rq, expired)) blk_mq_rq_timed_out(rq); return true; } @@ -1490,7 +1507,9 @@ static void blk_mq_timeout_work(struct work_struct *work) { struct request_queue *q = container_of(work, struct request_queue, timeout_work); - unsigned long next = 0; + struct blk_expired_data expired = { + .timeout_start = jiffies, + }; struct blk_mq_hw_ctx *hctx; unsigned long i; @@ -1510,10 +1529,23 @@ static void blk_mq_timeout_work(struct work_struct *work) if (!percpu_ref_tryget(&q->q_usage_counter)) return; - blk_mq_queue_tag_busy_iter(q, blk_mq_check_expired, &next); + /* check if there is any timed-out request */ + blk_mq_queue_tag_busy_iter(q, blk_mq_check_expired, &expired); + if (expired.has_timedout_rq) { + /* + * Before walking tags, we must ensure any submit started + * before the current time has finished. Since the submit + * uses srcu or rcu, wait for a synchronization point to + * ensure all running submits have finished + */ + blk_mq_wait_quiesce_done(q); + + expired.next = 0; + blk_mq_queue_tag_busy_iter(q, blk_mq_handle_expired, &expired); + } - if (next != 0) { - mod_timer(&q->timeout, next); + if (expired.next != 0) { + mod_timer(&q->timeout, expired.next); } else { /* * Request timeouts are handled as a forward rolling timer. If -- 2.35.1