Re: [PATCH 4/8] blk-mq: Facilitate a shared sbitmap per tagset

[Bart Van Assche <bvanassche@xxxxxxx>]

On 11/26/19 5:10 AM, Hannes Reinecke wrote:
> -struct blk_mq_tags *blk_mq_init_tags(unsigned int total_tags,
> +bool blk_mq_init_shared_sbitmap(struct blk_mq_tag_set *tag_set)
> +{
> +	unsigned int depth = tag_set->queue_depth -tag_set->reserved_tags;

A nit: please follow the coding style for spaces around the subtraction 
sign.

> +struct blk_mq_tags *blk_mq_init_tags(struct blk_mq_tag_set *set,
> +				     unsigned int total_tags,
>   				     unsigned int reserved_tags,
> -				     int node, int alloc_policy)
> +				     int node, int alloc_policy,
> +				     bool shared_tags)
>   {
>   	struct blk_mq_tags *tags;
>   
> @@ -488,9 +517,11 @@ struct blk_mq_tags *blk_mq_init_tags(unsigned int total_tags,
>   	tags->nr_tags = total_tags;
>   	tags->nr_reserved_tags = reserved_tags;
>   
> -	if (blk_mq_init_bitmap_tags(tags, node, alloc_policy) < 0) {
> -		kfree(tags);
> -		tags = NULL;
> +	if (shared_tags) {
> +		if (blk_mq_init_bitmap_tags(tags, node, alloc_policy) < 0) {
> +			kfree(tags);
> +			tags = NULL;
> +		}
>   	}
>   	return tags;
>   }

The above looks weird to me: the existing code path is only called if 
shared tags are enabled? Shouldn't "if (shared_tags)" be changed into 
"if (!shared_tags)"?

> +static inline bool blk_mq_is_sbitmap_shared(struct blk_mq_tag_set *tag_set)
> +{
> +	return !!(tag_set->flags & BLK_MQ_F_TAG_HCTX_SHARED);
> +}

Another nit: since the return type of this function is 'bool', the 
double exclamation mark and the parentheses are superfluous.

Bart.