From: Greg Kroah-Hartman Date: Mon, 18 May 2020 07:27:01 +0000 (+0200) Subject: 4.4-stable patches X-Git-Tag: v4.4.224~38 X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=49ed160e11bb818886814d8aca1cf7ab5702515b;p=thirdparty%2Fkernel%2Fstable-queue.git 4.4-stable patches added patches: blk-mq-allow-blocking-queue-tag-iter-callbacks.patch blk-mq-allow-timeouts-to-run-while-queue-is-freezing.patch blk-mq-sync-the-update-nr_hw_queues-with-blk_mq_queue_tag_busy_iter.patch block-defer-timeouts-to-a-workqueue.patch --- diff --git a/queue-4.4/blk-mq-allow-blocking-queue-tag-iter-callbacks.patch b/queue-4.4/blk-mq-allow-blocking-queue-tag-iter-callbacks.patch new file mode 100644 index 00000000000..9a0abf777d7 --- /dev/null +++ b/queue-4.4/blk-mq-allow-blocking-queue-tag-iter-callbacks.patch @@ -0,0 +1,51 @@ +From 530ca2c9bd6949c72c9b5cfc330cb3dbccaa3f5b Mon Sep 17 00:00:00 2001 +From: Keith Busch +Date: Tue, 25 Sep 2018 10:36:20 -0600 +Subject: blk-mq: Allow blocking queue tag iter callbacks + +From: Keith Busch + +commit 530ca2c9bd6949c72c9b5cfc330cb3dbccaa3f5b upstream. + +A recent commit runs tag iterator callbacks under the rcu read lock, +but existing callbacks do not satisfy the non-blocking requirement. +The commit intended to prevent an iterator from accessing a queue that's +being modified. This patch fixes the original issue by taking a queue +reference instead of reading it, which allows callbacks to make blocking +calls. + +Fixes: f5bbbbe4d6357 ("blk-mq: sync the update nr_hw_queues with blk_mq_queue_tag_busy_iter") +Acked-by: Jianchao Wang +Signed-off-by: Keith Busch +Signed-off-by: Jens Axboe +Signed-off-by: Giuliano Procida +Signed-off-by: Greg Kroah-Hartman + +--- + block/blk-mq-tag.c | 7 ++----- + 1 file changed, 2 insertions(+), 5 deletions(-) + +--- a/block/blk-mq-tag.c ++++ b/block/blk-mq-tag.c +@@ -484,11 +484,8 @@ void blk_mq_queue_tag_busy_iter(struct r + /* + * Avoid potential races with things like queue removal. + */ +- rcu_read_lock(); +- if (percpu_ref_is_zero(&q->q_usage_counter)) { +- rcu_read_unlock(); ++ if (!percpu_ref_tryget(&q->q_usage_counter)) + return; +- } + + queue_for_each_hw_ctx(q, hctx, i) { + struct blk_mq_tags *tags = hctx->tags; +@@ -505,7 +502,7 @@ void blk_mq_queue_tag_busy_iter(struct r + bt_for_each(hctx, &tags->bitmap_tags, tags->nr_reserved_tags, fn, priv, + false); + } +- rcu_read_unlock(); ++ blk_queue_exit(q); + } + + static unsigned int bt_unused_tags(struct blk_mq_bitmap_tags *bt) diff --git a/queue-4.4/blk-mq-allow-timeouts-to-run-while-queue-is-freezing.patch b/queue-4.4/blk-mq-allow-timeouts-to-run-while-queue-is-freezing.patch new file mode 100644 index 00000000000..134cc177b4e --- /dev/null +++ b/queue-4.4/blk-mq-allow-timeouts-to-run-while-queue-is-freezing.patch @@ -0,0 +1,110 @@ +From 71f79fb3179e69b0c1448a2101a866d871c66e7f Mon Sep 17 00:00:00 2001 +From: Gabriel Krisman Bertazi +Date: Mon, 1 Aug 2016 08:23:39 -0600 +Subject: blk-mq: Allow timeouts to run while queue is freezing + +From: Gabriel Krisman Bertazi + +commit 71f79fb3179e69b0c1448a2101a866d871c66e7f upstream. + +In case a submitted request gets stuck for some reason, the block layer +can prevent the request starvation by starting the scheduled timeout work. +If this stuck request occurs at the same time another thread has started +a queue freeze, the blk_mq_timeout_work will not be able to acquire the +queue reference and will return silently, thus not issuing the timeout. +But since the request is already holding a q_usage_counter reference and +is unable to complete, it will never release its reference, preventing +the queue from completing the freeze started by first thread. This puts +the request_queue in a hung state, forever waiting for the freeze +completion. + +This was observed while running IO to a NVMe device at the same time we +toggled the CPU hotplug code. Eventually, once a request got stuck +requiring a timeout during a queue freeze, we saw the CPU Hotplug +notification code get stuck inside blk_mq_freeze_queue_wait, as shown in +the trace below. + +[c000000deaf13690] [c000000deaf13738] 0xc000000deaf13738 (unreliable) +[c000000deaf13860] [c000000000015ce8] __switch_to+0x1f8/0x350 +[c000000deaf138b0] [c000000000ade0e4] __schedule+0x314/0x990 +[c000000deaf13940] [c000000000ade7a8] schedule+0x48/0xc0 +[c000000deaf13970] [c0000000005492a4] blk_mq_freeze_queue_wait+0x74/0x110 +[c000000deaf139e0] [c00000000054b6a8] blk_mq_queue_reinit_notify+0x1a8/0x2e0 +[c000000deaf13a40] [c0000000000e7878] notifier_call_chain+0x98/0x100 +[c000000deaf13a90] [c0000000000b8e08] cpu_notify_nofail+0x48/0xa0 +[c000000deaf13ac0] [c0000000000b92f0] _cpu_down+0x2a0/0x400 +[c000000deaf13b90] [c0000000000b94a8] cpu_down+0x58/0xa0 +[c000000deaf13bc0] [c0000000006d5dcc] cpu_subsys_offline+0x2c/0x50 +[c000000deaf13bf0] [c0000000006cd244] device_offline+0x104/0x140 +[c000000deaf13c30] [c0000000006cd40c] online_store+0x6c/0xc0 +[c000000deaf13c80] [c0000000006c8c78] dev_attr_store+0x68/0xa0 +[c000000deaf13cc0] [c0000000003974d0] sysfs_kf_write+0x80/0xb0 +[c000000deaf13d00] [c0000000003963e8] kernfs_fop_write+0x188/0x200 +[c000000deaf13d50] [c0000000002e0f6c] __vfs_write+0x6c/0xe0 +[c000000deaf13d90] [c0000000002e1ca0] vfs_write+0xc0/0x230 +[c000000deaf13de0] [c0000000002e2cdc] SyS_write+0x6c/0x110 +[c000000deaf13e30] [c000000000009204] system_call+0x38/0xb4 + +The fix is to allow the timeout work to execute in the window between +dropping the initial refcount reference and the release of the last +reference, which actually marks the freeze completion. This can be +achieved with percpu_refcount_tryget, which does not require the counter +to be alive. This way the timeout work can do it's job and terminate a +stuck request even during a freeze, returning its reference and avoiding +the deadlock. + +Allowing the timeout to run is just a part of the fix, since for some +devices, we might get stuck again inside the device driver's timeout +handler, should it attempt to allocate a new request in that path - +which is a quite common action for Abort commands, which need to be sent +after a timeout. In NVMe, for instance, we call blk_mq_alloc_request +from inside the timeout handler, which will fail during a freeze, since +it also tries to acquire a queue reference. + +I considered a similar change to blk_mq_alloc_request as a generic +solution for further device driver hangs, but we can't do that, since it +would allow new requests to disturb the freeze process. I thought about +creating a new function in the block layer to support unfreezable +requests for these occasions, but after working on it for a while, I +feel like this should be handled in a per-driver basis. I'm now +experimenting with changes to the NVMe timeout path, but I'm open to +suggestions of ways to make this generic. + +Signed-off-by: Gabriel Krisman Bertazi +Cc: Brian King +Cc: Keith Busch +Cc: linux-nvme@lists.infradead.org +Cc: linux-block@vger.kernel.org +Reviewed-by: Christoph Hellwig +Signed-off-by: Jens Axboe +Signed-off-by: Giuliano Procida +Signed-off-by: Greg Kroah-Hartman + +--- + block/blk-mq.c | 15 ++++++++++++++- + 1 file changed, 14 insertions(+), 1 deletion(-) + +--- a/block/blk-mq.c ++++ b/block/blk-mq.c +@@ -628,7 +628,20 @@ static void blk_mq_rq_timer(unsigned lon + }; + int i; + +- if (blk_queue_enter(q, GFP_NOWAIT)) ++ /* A deadlock might occur if a request is stuck requiring a ++ * timeout at the same time a queue freeze is waiting ++ * completion, since the timeout code would not be able to ++ * acquire the queue reference here. ++ * ++ * That's why we don't use blk_queue_enter here; instead, we use ++ * percpu_ref_tryget directly, because we need to be able to ++ * obtain a reference even in the short window between the queue ++ * starting to freeze, by dropping the first reference in ++ * blk_mq_freeze_queue_start, and the moment the last request is ++ * consumed, marked by the instant q_usage_counter reaches ++ * zero. ++ */ ++ if (!percpu_ref_tryget(&q->q_usage_counter)) + return; + + blk_mq_queue_tag_busy_iter(q, blk_mq_check_expired, &data); diff --git a/queue-4.4/blk-mq-sync-the-update-nr_hw_queues-with-blk_mq_queue_tag_busy_iter.patch b/queue-4.4/blk-mq-sync-the-update-nr_hw_queues-with-blk_mq_queue_tag_busy_iter.patch new file mode 100644 index 00000000000..394ac5e88fd --- /dev/null +++ b/queue-4.4/blk-mq-sync-the-update-nr_hw_queues-with-blk_mq_queue_tag_busy_iter.patch @@ -0,0 +1,69 @@ +From f5bbbbe4d63577026f908a809f22f5fd5a90ea1f Mon Sep 17 00:00:00 2001 +From: Jianchao Wang +Date: Tue, 21 Aug 2018 15:15:04 +0800 +Subject: blk-mq: sync the update nr_hw_queues with blk_mq_queue_tag_busy_iter + +From: Jianchao Wang + +commit f5bbbbe4d63577026f908a809f22f5fd5a90ea1f upstream. + +For blk-mq, part_in_flight/rw will invoke blk_mq_in_flight/rw to +account the inflight requests. It will access the queue_hw_ctx and +nr_hw_queues w/o any protection. When updating nr_hw_queues and +blk_mq_in_flight/rw occur concurrently, panic comes up. + +Before update nr_hw_queues, the q will be frozen. So we could use +q_usage_counter to avoid the race. percpu_ref_is_zero is used here +so that we will not miss any in-flight request. The access to +nr_hw_queues and queue_hw_ctx in blk_mq_queue_tag_busy_iter are +under rcu critical section, __blk_mq_update_nr_hw_queues could use +synchronize_rcu to ensure the zeroed q_usage_counter to be globally +visible. + +-------------- +NOTE: Back-ported to 4.4.y. + +The upstream commit was intended to prevent concurrent manipulation of +nr_hw_queues and iteration over queues. The former doesn't happen in +this in 4.4.7 (as __blk_mq_update_nr_hw_queues doesn't exist). The +extra locking is also buggy in this commit but fixed in a follow-up. + +It may protect against other concurrent accesses such as queue removal +by synchronising RCU locking around q_usage_counter. +-------------- + +Signed-off-by: Jianchao Wang +Reviewed-by: Ming Lei +Signed-off-by: Jens Axboe +Signed-off-by: Giuliano Procida +Signed-off-by: Greg Kroah-Hartman +--- + block/blk-mq-tag.c | 10 +++++++++- + 1 file changed, 9 insertions(+), 1 deletion(-) + +--- a/block/blk-mq-tag.c ++++ b/block/blk-mq-tag.c +@@ -481,6 +481,14 @@ void blk_mq_queue_tag_busy_iter(struct r + struct blk_mq_hw_ctx *hctx; + int i; + ++ /* ++ * Avoid potential races with things like queue removal. ++ */ ++ rcu_read_lock(); ++ if (percpu_ref_is_zero(&q->q_usage_counter)) { ++ rcu_read_unlock(); ++ return; ++ } + + queue_for_each_hw_ctx(q, hctx, i) { + struct blk_mq_tags *tags = hctx->tags; +@@ -497,7 +505,7 @@ void blk_mq_queue_tag_busy_iter(struct r + bt_for_each(hctx, &tags->bitmap_tags, tags->nr_reserved_tags, fn, priv, + false); + } +- ++ rcu_read_unlock(); + } + + static unsigned int bt_unused_tags(struct blk_mq_bitmap_tags *bt) diff --git a/queue-4.4/block-defer-timeouts-to-a-workqueue.patch b/queue-4.4/block-defer-timeouts-to-a-workqueue.patch new file mode 100644 index 00000000000..80f6e7e6785 --- /dev/null +++ b/queue-4.4/block-defer-timeouts-to-a-workqueue.patch @@ -0,0 +1,85 @@ +From 287922eb0b186e2a5bf54fdd04b734c25c90035c Mon Sep 17 00:00:00 2001 +From: Christoph Hellwig +Date: Fri, 30 Oct 2015 20:57:30 +0800 +Subject: block: defer timeouts to a workqueue + +From: Christoph Hellwig + +commit 287922eb0b186e2a5bf54fdd04b734c25c90035c upstream. + +Timer context is not very useful for drivers to perform any meaningful abort +action from. So instead of calling the driver from this useless context +defer it to a workqueue as soon as possible. + +Note that while a delayed_work item would seem the right thing here I didn't +dare to use it due to the magic in blk_add_timer that pokes deep into timer +internals. But maybe this encourages Tejun to add a sensible API for that to +the workqueue API and we'll all be fine in the end :) + +Contains a major update from Keith Bush: + +"This patch removes synchronizing the timeout work so that the timer can + start a freeze on its own queue. The timer enters the queue, so timer + context can only start a freeze, but not wait for frozen." + +------------- +NOTE: Back-ported to 4.4.y. + +The only parts of the upstream commit that have been kept are various +locking changes, none of which were mentioned in the original commit +message which therefore describes this change not at all. + +Timeout callbacks continue to be run via a timer. Both blk_mq_rq_timer +and blk_rq_timed_out_timer will return without without doing any work +if they cannot acquire the queue (without waiting). +------------- + +Signed-off-by: Christoph Hellwig +Acked-by: Keith Busch +Signed-off-by: Jens Axboe +Signed-off-by: Giuliano Procida +Signed-off-by: Greg Kroah-Hartman +--- + block/blk-mq.c | 4 ++++ + block/blk-timeout.c | 3 +++ + 2 files changed, 7 insertions(+) + +--- a/block/blk-mq.c ++++ b/block/blk-mq.c +@@ -628,6 +628,9 @@ static void blk_mq_rq_timer(unsigned lon + }; + int i; + ++ if (blk_queue_enter(q, GFP_NOWAIT)) ++ return; ++ + blk_mq_queue_tag_busy_iter(q, blk_mq_check_expired, &data); + + if (data.next_set) { +@@ -642,6 +645,7 @@ static void blk_mq_rq_timer(unsigned lon + blk_mq_tag_idle(hctx); + } + } ++ blk_queue_exit(q); + } + + /* +--- a/block/blk-timeout.c ++++ b/block/blk-timeout.c +@@ -134,6 +134,8 @@ void blk_rq_timed_out_timer(unsigned lon + struct request *rq, *tmp; + int next_set = 0; + ++ if (blk_queue_enter(q, GFP_NOWAIT)) ++ return; + spin_lock_irqsave(q->queue_lock, flags); + + list_for_each_entry_safe(rq, tmp, &q->timeout_list, timeout_list) +@@ -143,6 +145,7 @@ void blk_rq_timed_out_timer(unsigned lon + mod_timer(&q->timeout, round_jiffies_up(next)); + + spin_unlock_irqrestore(q->queue_lock, flags); ++ blk_queue_exit(q); + } + + /** diff --git a/queue-4.4/series b/queue-4.4/series index 867a764728a..1872a7a1f51 100644 --- a/queue-4.4/series +++ b/queue-4.4/series @@ -60,3 +60,7 @@ gcc-10-disable-zero-length-bounds-warning-for-now.patch gcc-10-disable-array-bounds-warning-for-now.patch gcc-10-disable-stringop-overflow-warning-for-now.patch gcc-10-disable-restrict-warning-for-now.patch +block-defer-timeouts-to-a-workqueue.patch +blk-mq-allow-timeouts-to-run-while-queue-is-freezing.patch +blk-mq-sync-the-update-nr_hw_queues-with-blk_mq_queue_tag_busy_iter.patch +blk-mq-allow-blocking-queue-tag-iter-callbacks.patch