Message ID | 20180109152954.GA5512@kernel.dk (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 1/9/18 8:29 AM, Jens Axboe wrote: > On Tue, Jan 09 2018, Ming Lei wrote: >> HW queues may be unmapped in some cases, such as blk_mq_update_nr_hw_queues(), >> then we need to check it before calling blk_mq_tag_idle(), otherwise >> the following kernel oops can be triggered, so fix it by checking if >> the hw queue is unmapped since it doesn't make sense to idle the tags >> any more after hw queues are unmapped. > > Seems cleaner to just move the mapped check to the idling function, > especially since we already have the same check in the other spot where > we call the idling. Ho hum, I guess that requires shuffling some code/includes to actually do that. I'll just apply yours as-is.
On Tue, Jan 09, 2018 at 08:38:55AM -0700, Jens Axboe wrote: > On 1/9/18 8:29 AM, Jens Axboe wrote: > > On Tue, Jan 09 2018, Ming Lei wrote: > >> HW queues may be unmapped in some cases, such as blk_mq_update_nr_hw_queues(), > >> then we need to check it before calling blk_mq_tag_idle(), otherwise > >> the following kernel oops can be triggered, so fix it by checking if > >> the hw queue is unmapped since it doesn't make sense to idle the tags > >> any more after hw queues are unmapped. > > > > Seems cleaner to just move the mapped check to the idling function, > > especially since we already have the same check in the other spot where > > we call the idling. > > Ho hum, I guess that requires shuffling some code/includes to > actually do that. I'll just apply yours as-is. Yes, that need to move blk_mq_hw_queue_mapped() into blk-mq-tag.h since the two headers are included by each other, seem some cleanup are needed for headers. Thanks, Ming
diff --git a/block/blk-mq-tag.h b/block/blk-mq-tag.h index 61deab0b5a5a..10e7e1ef8297 100644 --- a/block/blk-mq-tag.h +++ b/block/blk-mq-tag.h @@ -63,6 +63,8 @@ static inline bool blk_mq_tag_busy(struct blk_mq_hw_ctx *hctx) static inline void blk_mq_tag_idle(struct blk_mq_hw_ctx *hctx) { + if (!blk_mq_hw_queue_mapped(hctx)) + return; if (!(hctx->flags & BLK_MQ_F_TAG_SHARED)) return; diff --git a/block/blk-mq.c b/block/blk-mq.c index 111e1aa5562f..4d9f79bfdca2 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -873,11 +873,8 @@ static void blk_mq_timeout_work(struct work_struct *work) } else { struct blk_mq_hw_ctx *hctx; - queue_for_each_hw_ctx(q, hctx, i) { - /* the hctx may be unmapped, so check it here */ - if (blk_mq_hw_queue_mapped(hctx)) - blk_mq_tag_idle(hctx); - } + queue_for_each_hw_ctx(q, hctx, i) + blk_mq_tag_idle(hctx); } blk_queue_exit(q); }