Re: [PATCH RFC v7 07/12] blk-mq: Add support in hctx_tags_bitmap_show() for a shared sbitmap

Hannes Reinecke
2020-06-11T13:19:31Z

On 6/10/20 7:29 PM, John Garry wrote:
> Since a set-wide shared tag sbitmap may be used, it is no longer valid to
> examine the per-hctx tagset for getting the active requests for a hctx
> (when a shared sbitmap is used).
>
> As such, add support for the shared sbitmap by using an intermediate
> sbitmap per hctx, iterating all active tags for the specific hctx in the
> shared sbitmap.
>
> Originally-by: Bart Van Assche <bvanassche@acm.org>
> Reviewed-by: Hannes Reinecke <hare@suse.de> #earlier version
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
> block/blk-mq-debugfs.c | 62 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 62 insertions(+)
>
> diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c
> index 05b4be0c03d9..4da7e54adf3b 100644
> --- a/block/blk-mq-debugfs.c
> +++ b/block/blk-mq-debugfs.c
> @@ -495,6 +495,67 @@ static int hctx_tags_show(void *data, struct seq_file *m)
> return res;
> }
>
> +struct hctx_sb_data {
> + struct sbitmap *sb; /* output bitmap */
> + struct blk_mq_hw_ctx *hctx; /* input hctx */
> +};
> +
> +static bool hctx_filter_fn(struct blk_mq_hw_ctx *hctx, struct request *req,
> + void *priv, bool reserved)
> +{
> + struct hctx_sb_data *hctx_sb_data = priv;
> +
> + if (hctx == hctx_sb_data->hctx)
> + sbitmap_set_bit(hctx_sb_data->sb, req->tag);
> +
> + return true;
> +}
> +
> +static void hctx_filter_sb(struct sbitmap *sb, struct blk_mq_hw_ctx *hctx)
> +{
> + struct hctx_sb_data hctx_sb_data = { .sb = sb, .hctx = hctx };
> +
> + blk_mq_queue_tag_busy_iter(hctx->queue, hctx_filter_fn, &hctx_sb_data);
> +}
> +
> +static int hctx_tags_shared_sbitmap_bitmap_show(void *data, struct seq_file *m)
> +{
> + struct blk_mq_hw_ctx *hctx = data;
> + struct request_queue *q = hctx->queue;
> + struct blk_mq_tag_set *set = q->tag_set;
> + struct sbitmap shared_sb, *sb;
> + int res;
> +
> + if (!set)
> + return 0;
> +
> + /*
> + * We could use the allocated sbitmap for that hctx here, but
> + * that would mean that we would need to clean it prior to use.
> + */
> + res = sbitmap_init_node(&shared_sb,
> + set->__bitmap_tags.sb.depth,
> + set->__bitmap_tags.sb.shift,
> + GFP_KERNEL, NUMA_NO_NODE);
> + if (res)
> + return res;
> + sb = &shared_sb;
> +
> + res = mutex_lock_interruptible(&q->sysfs_lock);
> + if (res)
> + goto out;
> + if (hctx->tags) {
> + hctx_filter_sb(sb, hctx);
> + sbitmap_bitmap_show(sb, m);
> + }
> +
> + mutex_unlock(&q->sysfs_lock);
> +
> +out:
> + sbitmap_free(&shared_sb);
> + return res;
> +}
> +
> static int hctx_tags_bitmap_show(void *data, struct seq_file *m)
> {
> struct blk_mq_hw_ctx *hctx = data;
> @@ -823,6 +884,7 @@ static const struct blk_mq_debugfs_attr blk_mq_debugfs_hctx_shared_sbitmap_attrs
> {"busy", 0400, hctx_busy_show},
> {"ctx_map", 0400, hctx_ctx_map_show},
> {"tags", 0400, hctx_tags_show},
> + {"tags_bitmap", 0400, hctx_tags_shared_sbitmap_bitmap_show},
> {"sched_tags", 0400, hctx_sched_tags_show},
> {"sched_tags_bitmap", 0400, hctx_sched_tags_bitmap_show},
> {"io_poll", 0600, hctx_io_poll_show, hctx_io_poll_write},
>
Ah, right. Here it is.

But I don't get it; why do we have to allocate a temporary bitmap and
can't walk the existing shared sbitmap?

Cheers,

Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer