On Thu, Aug 19, 2021 at 08:32:20AM +0100, John Garry wrote: > On 19/08/2021 01:39, Ming Lei wrote: > > > That's intentional, as we have from later patch: > > > > > > void blk_mq_free_rqs(struct blk_mq_tag_set *set, struct blk_mq_tags *tags, > > > unsigned int hctx_idx) > > > { > > > struct blk_mq_tags *drv_tags; > > > struct page *page; > > > > > > + if (blk_mq_is_sbitmap_shared(set->flags)) > > > + drv_tags = set->shared_sbitmap_tags; > > > + else > > > drv_tags = set->tags[hctx_idx]; > > > > > > ... > > > > > > blk_mq_clear_rq_mapping(drv_tags, tags); > > > > > > } > > > > > > And it's just nice to not re-indent later. > > But this way is weird, and I don't think checkpatch.pl is happy with > > it. > > There is the idea to try to not remove/change code earlier in a series - I > am taking it to an extreme! I can stop. > > On another related topic, how about this change also: > > ---8<--- > void blk_mq_clear_rq_mapping(struct blk_mq_tags *drv_tags, > struct blk_mq_tags *tags) > { > > + /* There is no need to clear a driver tags own mapping */ > + if (drv_tags == tags) > + return; > --->8--- The change itself is correct, and no need to clear driver tags ->rq[] since all request queues have been cleaned up when freeing tagset. Thanks, Ming