| From 1e041b11cf37f661ff430d3fb4a3fc0bcaf44ce9 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Wed, 9 Jun 2021 09:58:21 +0800 |
| Subject: block: fix race between adding/removing rq qos and normal IO |
| |
| From: Ming Lei <ming.lei@redhat.com> |
| |
| [ Upstream commit 2cafe29a8d03f02a3d16193bdaae2f3e82a423f9 ] |
| |
| Yi reported several kernel panics on: |
| |
| [16687.001777] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008 |
| ... |
| [16687.163549] pc : __rq_qos_track+0x38/0x60 |
| |
| or |
| |
| [ 997.690455] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000020 |
| ... |
| [ 997.850347] pc : __rq_qos_done+0x2c/0x50 |
| |
| Turns out it is caused by race between adding rq qos(wbt) and normal IO |
| because rq_qos_add can be run when IO is being submitted, fix this issue |
| by freezing queue before adding/deleting rq qos to queue. |
| |
| rq_qos_exit() needn't to freeze queue because it is called after queue |
| has been frozen. |
| |
| iolatency calls rq_qos_add() during allocating queue, so freezing won't |
| add delay because queue usage refcount works at atomic mode at that |
| time. |
| |
| iocost calls rq_qos_add() when writing cgroup attribute file, that is |
| fine to freeze queue at that time since we usually freeze queue when |
| storing to queue sysfs attribute, meantime iocost only exists on the |
| root cgroup. |
| |
| wbt_init calls it in blk_register_queue() and queue sysfs attribute |
| store(queue_wb_lat_store() when write it 1st time in case of !BLK_WBT_MQ), |
| the following patch will speedup the queue freezing in wbt_init. |
| |
| Reported-by: Yi Zhang <yi.zhang@redhat.com> |
| Cc: Bart Van Assche <bvanassche@acm.org> |
| Signed-off-by: Ming Lei <ming.lei@redhat.com> |
| Reviewed-by: Bart Van Assche <bvanassche@acm.org> |
| Tested-by: Yi Zhang <yi.zhang@redhat.com> |
| Link: https://lore.kernel.org/r/20210609015822.103433-2-ming.lei@redhat.com |
| Signed-off-by: Jens Axboe <axboe@kernel.dk> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| block/blk-rq-qos.h | 24 ++++++++++++++++++++++++ |
| 1 file changed, 24 insertions(+) |
| |
| diff --git a/block/blk-rq-qos.h b/block/blk-rq-qos.h |
| index 2bc43e94f4c4..2bcb3495e376 100644 |
| --- a/block/blk-rq-qos.h |
| +++ b/block/blk-rq-qos.h |
| @@ -7,6 +7,7 @@ |
| #include <linux/blk_types.h> |
| #include <linux/atomic.h> |
| #include <linux/wait.h> |
| +#include <linux/blk-mq.h> |
| |
| #include "blk-mq-debugfs.h" |
| |
| @@ -99,8 +100,21 @@ static inline void rq_wait_init(struct rq_wait *rq_wait) |
| |
| static inline void rq_qos_add(struct request_queue *q, struct rq_qos *rqos) |
| { |
| + /* |
| + * No IO can be in-flight when adding rqos, so freeze queue, which |
| + * is fine since we only support rq_qos for blk-mq queue. |
| + * |
| + * Reuse ->queue_lock for protecting against other concurrent |
| + * rq_qos adding/deleting |
| + */ |
| + blk_mq_freeze_queue(q); |
| + |
| + spin_lock_irq(&q->queue_lock); |
| rqos->next = q->rq_qos; |
| q->rq_qos = rqos; |
| + spin_unlock_irq(&q->queue_lock); |
| + |
| + blk_mq_unfreeze_queue(q); |
| |
| if (rqos->ops->debugfs_attrs) |
| blk_mq_debugfs_register_rqos(rqos); |
| @@ -110,12 +124,22 @@ static inline void rq_qos_del(struct request_queue *q, struct rq_qos *rqos) |
| { |
| struct rq_qos **cur; |
| |
| + /* |
| + * See comment in rq_qos_add() about freezing queue & using |
| + * ->queue_lock. |
| + */ |
| + blk_mq_freeze_queue(q); |
| + |
| + spin_lock_irq(&q->queue_lock); |
| for (cur = &q->rq_qos; *cur; cur = &(*cur)->next) { |
| if (*cur == rqos) { |
| *cur = rqos->next; |
| break; |
| } |
| } |
| + spin_unlock_irq(&q->queue_lock); |
| + |
| + blk_mq_unfreeze_queue(q); |
| |
| blk_mq_debugfs_unregister_rqos(rqos); |
| } |
| -- |
| 2.30.2 |
| |