Re: [PATCH 1/9] blk-mq: allow hw queues to share hostwide tags

[John Garry <john.garry@xxxxxxxxxx>]

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.

> +	}
> +
>  	set->tags[hctx_idx] = blk_mq_alloc_rq_map(set, hctx_idx,
>  					set->queue_depth, set->reserved_tags);

Thanks,
John

>  	if (!set->tags[hctx_idx])
> @@ -2451,6 +2456,9 @@ static bool __blk_mq_alloc_rq_map(struct blk_mq_tag_set *set, int hctx_idx)
>  static void blk_mq_free_map_and_requests(struct blk_mq_tag_set *set,
>