Re: [PATCH] blk-mq: Fix two racy hctx->tags->rqs[] assignments
From: Ming Lei <hidden>
Date: 2021-04-25 00:13:21
On Fri, Apr 23, 2021 at 01:01:09PM -0700, Bart Van Assche wrote:
quoted hunk ↗ jump to hunk
hctx->tags->rqs[] must be cleared before releasing a request tag because otherwise clearing that pointer races with the following assignment in blk_mq_get_driver_tag(): rcu_assign_pointer(hctx->tags->rqs[rq->tag], rq); Reported-by: Ming Lei <redacted> Cc: Christoph Hellwig <hch@lst.de> Cc: Ming Lei <redacted> Cc: Hannes Reinecke <hare@suse.de> Cc: John Garry <redacted> Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- block/blk-mq.c | 2 +- block/blk-mq.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)diff --git a/block/blk-mq.c b/block/blk-mq.c index 06d204796c43..1ffaab7c9b11 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c@@ -501,8 +501,8 @@ static void __blk_mq_free_request(struct request *rq) blk_pm_mark_last_busy(rq); rq->mq_hctx = NULL; if (rq->tag != BLK_MQ_NO_TAG) { - blk_mq_put_tag(hctx->tags, ctx, rq->tag); rcu_assign_pointer(hctx->tags->rqs[rq->tag], NULL); + blk_mq_put_tag(hctx->tags, ctx, rq->tag); } if (sched_tag != BLK_MQ_NO_TAG) blk_mq_put_tag(hctx->sched_tags, ctx, sched_tag);diff --git a/block/blk-mq.h b/block/blk-mq.h index 9ccb1818303b..f73cd659eb81 100644 --- a/block/blk-mq.h +++ b/block/blk-mq.h@@ -225,8 +225,8 @@ static inline int __blk_mq_active_requests(struct blk_mq_hw_ctx *hctx) static inline void __blk_mq_put_driver_tag(struct blk_mq_hw_ctx *hctx, struct request *rq) { - blk_mq_put_tag(hctx->tags, rq->mq_ctx, rq->tag); rcu_assign_pointer(hctx->tags->rqs[rq->tag], NULL); + blk_mq_put_tag(hctx->tags, rq->mq_ctx, rq->tag); rq->tag = BLK_MQ_NO_TAG; if (rq->rq_flags & RQF_MQ_INFLIGHT) {
I'd suggest to document the memory order which is key to the usage's correctness, especially both memory barriers are implied in allocating & releasing bit tag. Thanks, Ming