diff mbox series

[2/7] blk-mq: count changed hctx as active in blk_mq_get_tag

Message ID 20230209201116.579809-3-shikemeng@huaweicloud.com (mailing list archive)
State New, archived
Headers show
Series A few bugfix and cleanup patches to blk-mq | expand

Commit Message

Kemeng Shi Feb. 9, 2023, 8:11 p.m. UTC
Commit d263ed9926823 ("blk-mq: count the hctx as active before allocating
tag") active hctx before blk_mq_get_tag to avoid petential starvation.
However, the hctx to alloc tag may change in blk_mq_get_tag if
BLK_MQ_REQ_NOWAIT is not set, then there are two problems:
1. The hctx without real allocation is marked active.
2. The starvation problem mentioned in Commit d263ed9926823 ("blk-mq:
count the hctx as active before allocating tag") still exists on the
changed hctx as it maybe not marked active before tag allocation.

For problem 1, the hctx which is marked active probably gets IO
soon or will be marked inactive after lazy detection of tag idle.
Mark changed hctx active to fix problem 2.

Fixes: d263ed992682 ("blk-mq: count the hctx as active before allocating tag")
Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
---
 block/blk-mq-tag.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Jan Kara Feb. 9, 2023, 1:55 p.m. UTC | #1
On Fri 10-02-23 04:11:11, Kemeng Shi wrote:
> Commit d263ed9926823 ("blk-mq: count the hctx as active before allocating
> tag") active hctx before blk_mq_get_tag to avoid petential starvation.
> However, the hctx to alloc tag may change in blk_mq_get_tag if
> BLK_MQ_REQ_NOWAIT is not set, then there are two problems:
> 1. The hctx without real allocation is marked active.
> 2. The starvation problem mentioned in Commit d263ed9926823 ("blk-mq:
> count the hctx as active before allocating tag") still exists on the
> changed hctx as it maybe not marked active before tag allocation.
> 
> For problem 1, the hctx which is marked active probably gets IO
> soon or will be marked inactive after lazy detection of tag idle.
> Mark changed hctx active to fix problem 2.
> 
> Fixes: d263ed992682 ("blk-mq: count the hctx as active before allocating tag")
> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>

Yeah, makes sense to me. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  block/blk-mq-tag.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
> index 1d3135acfc98..e566fd96dc26 100644
> --- a/block/blk-mq-tag.c
> +++ b/block/blk-mq-tag.c
> @@ -191,6 +191,9 @@ unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data)
>  		data->ctx = blk_mq_get_ctx(data->q);
>  		data->hctx = blk_mq_map_queue(data->q, data->cmd_flags,
>  						data->ctx);
> +		if (!(data->rq_flags & RQF_ELV))
> +			blk_mq_tag_busy(data->hctx);
> +
>  		tags = blk_mq_tags_from_data(data);
>  		if (data->flags & BLK_MQ_REQ_RESERVED)
>  			bt = &tags->breserved_tags;
> -- 
> 2.30.0
>
diff mbox series

Patch

diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index 1d3135acfc98..e566fd96dc26 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -191,6 +191,9 @@  unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data)
 		data->ctx = blk_mq_get_ctx(data->q);
 		data->hctx = blk_mq_map_queue(data->q, data->cmd_flags,
 						data->ctx);
+		if (!(data->rq_flags & RQF_ELV))
+			blk_mq_tag_busy(data->hctx);
+
 		tags = blk_mq_tags_from_data(data);
 		if (data->flags & BLK_MQ_REQ_RESERVED)
 			bt = &tags->breserved_tags;