Re: [PATCH 1/5] blk-mq: tags: define several fields of tags as pointer

[Bart Van Assche <Bart.VanAssche@wdc.com>]

On Sat, 2018-02-03 at 12:21 +0800, Ming Lei wrote:
> diff --git a/block/blk-mq-tag.h b/block/blk-mq-tag.h
> index 61deab0b5a5a..a68323fa0c02 100644
> --- a/block/blk-mq-tag.h
> +++ b/block/blk-mq-tag.h
> @@ -11,10 +11,14 @@ struct blk_mq_tags {
>  	unsigned int nr_tags;
>  	unsigned int nr_reserved_tags;
>  
> -	atomic_t active_queues;
> +	atomic_t *active_queues;
> +	atomic_t __active_queues;
>  
> -	struct sbitmap_queue bitmap_tags;
> -	struct sbitmap_queue breserved_tags;
> +	struct sbitmap_queue *bitmap_tags;
> +	struct sbitmap_queue *breserved_tags;
> +
> +	struct sbitmap_queue __bitmap_tags;
> +	struct sbitmap_queue __breserved_tags;
>  
>  	struct request **rqs;
>  	struct request **static_rqs;

This is getting ugly: multiple pointers that either all point at an element
inside struct blk_mq_tags or all point at a member outside blk_mq_tags. Have
you considered to introduce a new structure for these members (active_queues,
bitmap_tags and breserved_tags) such that only a single new pointer has to
be introduced in struct blk_mq_tags? Additionally, why does every
blk_mq_tags instance have its own static_rqs array if tags are shared
across hardware queues? blk_mq_get_tag() allocates a tag either from
bitmap_tags or from breserved_tags. So if these tag sets are shared I
don't think that it is necessary to have one static_rqs array per struct
blk_mq_tags instance.

Thanks,

Bart.