|
| 1 | +blk-mq: fix potential io hang by wrong 'wake_batch' |
| 2 | + |
| 3 | +jira LE-4034 |
| 4 | +Rebuild_History Non-Buildable kernel-4.18.0-553.72.1.el8_10 |
| 5 | +commit-author Yu Kuai < [email protected]> |
| 6 | +commit 4f1731df60f9033669f024d06ae26a6301260b55 |
| 7 | +Empty-Commit: Cherry-Pick Conflicts during history rebuild. |
| 8 | +Will be included in final tarball splat. Ref for failed cherry-pick at: |
| 9 | +ciq/ciq_backports/kernel-4.18.0-553.72.1.el8_10/4f1731df.failed |
| 10 | + |
| 11 | +In __blk_mq_tag_busy/idle(), updating 'active_queues' and calculating |
| 12 | +'wake_batch' is not atomic: |
| 13 | + |
| 14 | +t1: t2: |
| 15 | +_blk_mq_tag_busy blk_mq_tag_busy |
| 16 | +inc active_queues |
| 17 | +// assume 1->2 |
| 18 | + inc active_queues |
| 19 | + // 2 -> 3 |
| 20 | + blk_mq_update_wake_batch |
| 21 | + // calculate based on 3 |
| 22 | +blk_mq_update_wake_batch |
| 23 | +/* calculate based on 2, while active_queues is actually 3. */ |
| 24 | + |
| 25 | +Fix this problem by protecting them wih 'tags->lock', this is not a hot |
| 26 | +path, so performance should not be concerned. And now that all writers |
| 27 | +are inside the lock, switch 'actives_queues' from atomic to unsigned |
| 28 | +int. |
| 29 | + |
| 30 | +Fixes: 180dccb0dba4 ("blk-mq: fix tag_get wait task can't be awakened") |
| 31 | + Signed-off-by: Yu Kuai < [email protected]> |
| 32 | + Reviewed-by: Jan Kara < [email protected]> |
| 33 | +Link: https://lore.kernel.org/r/ [email protected] |
| 34 | + Signed-off-by: Jens Axboe < [email protected]> |
| 35 | +(cherry picked from commit 4f1731df60f9033669f024d06ae26a6301260b55) |
| 36 | + Signed-off-by: Jonathan Maple < [email protected]> |
| 37 | + |
| 38 | +# Conflicts: |
| 39 | +# block/blk-mq-tag.c |
| 40 | +# block/blk-mq.h |
| 41 | +# include/linux/blk-mq.h |
| 42 | +diff --cc block/blk-mq-tag.c |
| 43 | +index 21f98a7c6b8a,426197312069..000000000000 |
| 44 | +--- a/block/blk-mq-tag.c |
| 45 | ++++ b/block/blk-mq-tag.c |
| 46 | +@@@ -21,22 -35,28 +21,37 @@@ |
| 47 | + * to get tag when first time, the other shared-tag users could reserve |
| 48 | + * budget for it. |
| 49 | + */ |
| 50 | + -void __blk_mq_tag_busy(struct blk_mq_hw_ctx *hctx) |
| 51 | + +bool __blk_mq_tag_busy(struct blk_mq_hw_ctx *hctx) |
| 52 | + { |
| 53 | +++<<<<<<< HEAD |
| 54 | + + if (blk_mq_is_sbitmap_shared(hctx->flags)) { |
| 55 | +++======= |
| 56 | ++ unsigned int users; |
| 57 | ++ struct blk_mq_tags *tags = hctx->tags; |
| 58 | ++ |
| 59 | ++ if (blk_mq_is_shared_tags(hctx->flags)) { |
| 60 | +++>>>>>>> 4f1731df60f9 (blk-mq: fix potential io hang by wrong 'wake_batch') |
| 61 | + struct request_queue *q = hctx->queue; |
| 62 | + + struct blk_mq_tag_set *set = q->tag_set; |
| 63 | + |
| 64 | + - if (test_bit(QUEUE_FLAG_HCTX_ACTIVE, &q->queue_flags)) |
| 65 | + - return; |
| 66 | + - set_bit(QUEUE_FLAG_HCTX_ACTIVE, &q->queue_flags); |
| 67 | + + if (!test_bit(QUEUE_FLAG_HCTX_ACTIVE, &q->queue_flags) && |
| 68 | + + !test_and_set_bit(QUEUE_FLAG_HCTX_ACTIVE, &q->queue_flags)) |
| 69 | + + atomic_inc(&set->active_queues_shared_sbitmap); |
| 70 | + } else { |
| 71 | + - if (test_bit(BLK_MQ_S_TAG_ACTIVE, &hctx->state)) |
| 72 | + - return; |
| 73 | + - set_bit(BLK_MQ_S_TAG_ACTIVE, &hctx->state); |
| 74 | + + if (!test_bit(BLK_MQ_S_TAG_ACTIVE, &hctx->state) && |
| 75 | + + !test_and_set_bit(BLK_MQ_S_TAG_ACTIVE, &hctx->state)) |
| 76 | + + atomic_inc(&hctx->tags->active_queues); |
| 77 | + } |
| 78 | + |
| 79 | +++<<<<<<< HEAD |
| 80 | + + return true; |
| 81 | +++======= |
| 82 | ++ spin_lock_irq(&tags->lock); |
| 83 | ++ users = tags->active_queues + 1; |
| 84 | ++ WRITE_ONCE(tags->active_queues, users); |
| 85 | ++ blk_mq_update_wake_batch(tags, users); |
| 86 | ++ spin_unlock_irq(&tags->lock); |
| 87 | +++>>>>>>> 4f1731df60f9 (blk-mq: fix potential io hang by wrong 'wake_batch') |
| 88 | + } |
| 89 | + |
| 90 | + /* |
| 91 | +@@@ -67,9 -87,14 +82,18 @@@ void __blk_mq_tag_idle(struct blk_mq_hw |
| 92 | + } else { |
| 93 | + if (!test_and_clear_bit(BLK_MQ_S_TAG_ACTIVE, &hctx->state)) |
| 94 | + return; |
| 95 | + + atomic_dec(&tags->active_queues); |
| 96 | + } |
| 97 | + |
| 98 | +++<<<<<<< HEAD |
| 99 | +++======= |
| 100 | ++ spin_lock_irq(&tags->lock); |
| 101 | ++ users = tags->active_queues - 1; |
| 102 | ++ WRITE_ONCE(tags->active_queues, users); |
| 103 | ++ blk_mq_update_wake_batch(tags, users); |
| 104 | ++ spin_unlock_irq(&tags->lock); |
| 105 | ++ |
| 106 | +++>>>>>>> 4f1731df60f9 (blk-mq: fix potential io hang by wrong 'wake_batch') |
| 107 | + blk_mq_tag_wakeup_all(tags, false); |
| 108 | + } |
| 109 | + |
| 110 | +diff --cc block/blk-mq.h |
| 111 | +index c12b86d8c632,1743857e0b01..000000000000 |
| 112 | +--- a/block/blk-mq.h |
| 113 | ++++ b/block/blk-mq.h |
| 114 | +@@@ -347,9 -410,9 +347,13 @@@ static inline bool hctx_may_queue(struc |
| 115 | + } else { |
| 116 | + if (!test_bit(BLK_MQ_S_TAG_ACTIVE, &hctx->state)) |
| 117 | + return true; |
| 118 | + + users = atomic_read(&hctx->tags->active_queues); |
| 119 | + } |
| 120 | + |
| 121 | +++<<<<<<< HEAD |
| 122 | +++======= |
| 123 | ++ users = READ_ONCE(hctx->tags->active_queues); |
| 124 | +++>>>>>>> 4f1731df60f9 (blk-mq: fix potential io hang by wrong 'wake_batch') |
| 125 | + if (!users) |
| 126 | + return true; |
| 127 | + |
| 128 | +diff --cc include/linux/blk-mq.h |
| 129 | +index 62c0a67b5e72,f401067ac03a..000000000000 |
| 130 | +--- a/include/linux/blk-mq.h |
| 131 | ++++ b/include/linux/blk-mq.h |
| 132 | +@@@ -371,16 -723,48 +371,52 @@@ enum |
| 133 | + BLK_MQ_REQ_NOWAIT = (__force blk_mq_req_flags_t)(1 << 0), |
| 134 | + /* allocate from reserved pool */ |
| 135 | + BLK_MQ_REQ_RESERVED = (__force blk_mq_req_flags_t)(1 << 1), |
| 136 | + - /* set RQF_PM */ |
| 137 | + - BLK_MQ_REQ_PM = (__force blk_mq_req_flags_t)(1 << 2), |
| 138 | + + /* set RQF_PREEMPT */ |
| 139 | + + BLK_MQ_REQ_PREEMPT = (__force blk_mq_req_flags_t)(1 << 3), |
| 140 | + }; |
| 141 | + |
| 142 | + -struct request *blk_mq_alloc_request(struct request_queue *q, blk_opf_t opf, |
| 143 | + +struct request *blk_mq_alloc_request(struct request_queue *q, unsigned int op, |
| 144 | + blk_mq_req_flags_t flags); |
| 145 | + struct request *blk_mq_alloc_request_hctx(struct request_queue *q, |
| 146 | + - blk_opf_t opf, blk_mq_req_flags_t flags, |
| 147 | + + unsigned int op, blk_mq_req_flags_t flags, |
| 148 | + unsigned int hctx_idx); |
| 149 | +++<<<<<<< HEAD |
| 150 | + +struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, unsigned int tag); |
| 151 | +++======= |
| 152 | ++ |
| 153 | ++ /* |
| 154 | ++ * Tag address space map. |
| 155 | ++ */ |
| 156 | ++ struct blk_mq_tags { |
| 157 | ++ unsigned int nr_tags; |
| 158 | ++ unsigned int nr_reserved_tags; |
| 159 | ++ unsigned int active_queues; |
| 160 | ++ |
| 161 | ++ struct sbitmap_queue bitmap_tags; |
| 162 | ++ struct sbitmap_queue breserved_tags; |
| 163 | ++ |
| 164 | ++ struct request **rqs; |
| 165 | ++ struct request **static_rqs; |
| 166 | ++ struct list_head page_list; |
| 167 | ++ |
| 168 | ++ /* |
| 169 | ++ * used to clear request reference in rqs[] before freeing one |
| 170 | ++ * request pool |
| 171 | ++ */ |
| 172 | ++ spinlock_t lock; |
| 173 | ++ }; |
| 174 | ++ |
| 175 | ++ static inline struct request *blk_mq_tag_to_rq(struct blk_mq_tags *tags, |
| 176 | ++ unsigned int tag) |
| 177 | ++ { |
| 178 | ++ if (tag < tags->nr_tags) { |
| 179 | ++ prefetch(tags->rqs[tag]); |
| 180 | ++ return tags->rqs[tag]; |
| 181 | ++ } |
| 182 | ++ |
| 183 | ++ return NULL; |
| 184 | ++ } |
| 185 | +++>>>>>>> 4f1731df60f9 (blk-mq: fix potential io hang by wrong 'wake_batch') |
| 186 | + |
| 187 | + enum { |
| 188 | + BLK_MQ_UNIQUE_TAG_BITS = 16, |
| 189 | +diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c |
| 190 | +index deea36cb6960..249cccf5675b 100644 |
| 191 | +--- a/block/blk-mq-debugfs.c |
| 192 | ++++ b/block/blk-mq-debugfs.c |
| 193 | +@@ -461,7 +461,7 @@ static void blk_mq_debugfs_tags_show(struct seq_file *m, |
| 194 | + seq_printf(m, "nr_tags=%u\n", tags->nr_tags); |
| 195 | + seq_printf(m, "nr_reserved_tags=%u\n", tags->nr_reserved_tags); |
| 196 | + seq_printf(m, "active_queues=%d\n", |
| 197 | +- atomic_read(&tags->active_queues)); |
| 198 | ++ READ_ONCE(tags->active_queues)); |
| 199 | + |
| 200 | + seq_puts(m, "\nbitmap_tags:\n"); |
| 201 | + sbitmap_queue_show(tags->bitmap_tags, m); |
| 202 | +* Unmerged path block/blk-mq-tag.c |
| 203 | +* Unmerged path block/blk-mq.h |
| 204 | +* Unmerged path include/linux/blk-mq.h |
0 commit comments