Messages in this thread | | | Subject | Re: [PATCH] blk-mq: Fix blk_mq_tagset_busy_iter() for shared tags | From | John Garry <> | Date | Wed, 13 Oct 2021 11:01:11 +0100 |
| |
On 13/10/2021 10:22, Ming Lei wrote: > On Wed, Oct 13, 2021 at 04:40:59PM +0800, John Garry wrote: >> Since it is now possible for a tagset to share a single set of tags, the >> iter function should not re-iter the tags for the count of #hw queues in >> that case. Rather it should just iter once. >> >> Fixes: e0fdf846c7bb ("blk-mq: Use shared tags for shared sbitmap support") >> Reported-by: Kashyap Desai<kashyap.desai@broadcom.com> >> Signed-off-by: John Garry<john.garry@huawei.com> >> >> diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c >> index 72a2724a4eee..c943b6529619 100644 >> --- a/block/blk-mq-tag.c >> +++ b/block/blk-mq-tag.c >> @@ -378,9 +378,12 @@ void blk_mq_all_tag_iter(struct blk_mq_tags *tags, busy_tag_iter_fn *fn, >> void blk_mq_tagset_busy_iter(struct blk_mq_tag_set *tagset, >> busy_tag_iter_fn *fn, void *priv) >> { >> - int i; >> + unsigned int flags = tagset->flags; >> + int i, nr_tags; >> + >> + nr_tags = blk_mq_is_shared_tags(flags) ? 1 : tagset->nr_hw_queues; >> >> - for (i = 0; i < tagset->nr_hw_queues; i++) { >> + for (i = 0; i < nr_tags; i++) { >> if (tagset->tags && tagset->tags[i]) >> __blk_mq_all_tag_iter(tagset->tags[i], fn, priv, >> BT_TAG_ITER_STARTED); > blk_mq_queue_tag_busy_iter() needn't such change?
I didn't think so.
blk_mq_queue_tag_busy_iter() will indeed re-iter the tags per hctx. However in bt_iter(), we check rq->mq_hctx == hctx for calling the iter callback:
static bool bt_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data) { ...
if (rq->q == hctx->queue && rq->mq_hctx == hctx) ret = iter_data->fn(hctx, rq, iter_data->data, reserved);
And this would only pass for the correct hctx which we're iter'ing for. Indeed, it would be nice not to iter excessive times, but I didn't see a straightforward way to change that.
There is also blk_mq_all_tag_iter():
void blk_mq_all_tag_iter(struct blk_mq_tags *tags, busy_tag_iter_fn *fn, void *priv) { __blk_mq_all_tag_iter(tags, fn, priv, BT_TAG_ITER_STATIC_RQS); }
But then the only user is blk_mq_hctx_has_requests():
static bool blk_mq_hctx_has_requests(struct blk_mq_hw_ctx *hctx) { struct blk_mq_tags *tags = hctx->sched_tags ? hctx->sched_tags : hctx->tags; struct rq_iter_data data = { .hctx = hctx, };
blk_mq_all_tag_iter(tags, blk_mq_has_request, &data); return data.has_rq; }
But, again like bt_iter(), blk_mq_has_request() will check the hctx matches:
static bool blk_mq_has_request(struct request *rq, void *data, bool reserved) { struct rq_iter_data *iter_data = data;
if (rq->mq_hctx != iter_data->hctx) return true; iter_data->has_rq = true; return false; }
Thanks, John
| |