On 24/06/2019 09:46, Ming Lei wrote:
On Wed, Jun 05, 2019 at 03:10:51PM +0100, John Garry wrote:
On 31/05/2019 03:27, Ming Lei wrote:
index 32b8ad3d341b..49d73d979cb3 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2433,6 +2433,11 @@ static bool __blk_mq_alloc_rq_map(struct blk_mq_tag_set *set, int hctx_idx)
{
int ret = 0;
Hi Ming,
+ if ((set->flags & BLK_MQ_F_HOST_TAGS) && hctx_idx) {
+ set->tags[hctx_idx] = set->tags[0];
Here we set all tags same as that of hctx index 0.
+ return true;
As such, I think that the error handling in __blk_mq_alloc_rq_maps() is made
a little fragile:
__blk_mq_alloc_rq_maps(struct blk_mq_tag_set *set)
{
int i;
for (i = 0; i < set->nr_hw_queues; i++)
if (!__blk_mq_alloc_rq_map(set, i))
goto out_unwind;
return 0;
out_unwind:
while (--i >= 0)
blk_mq_free_rq_map(set->tags[i]);
return -ENOMEM;
}
If __blk_mq_alloc_rq_map(, i > 1) fails for when BLK_MQ_F_HOST_TAGS FLAG is
set (even though today it can't), then we would try to free set->tags[0]
multiple times.
Hi Ming,
Good catch, and the issue can be addressed easily by setting set->hctx[i] as
NULL, then check 'tags' in blk_mq_free_rq_map().
OK, so you could do that. But I just think that it's not a great
practice in general to have multiple pointers pointing at the same
dynamic memory.
Thanks,
John
Thanks,
Ming
.
