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

Bart Van Assche
2019-11-27T17:03:49Z

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.