linux-kernel - Re: [PATCH 2/7] blk-mq: count changed hctx as active in blk_mq_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20230209135539.mokecz63dsolmemj@quack3>
Date:   Thu, 9 Feb 2023 14:55:39 +0100
From:   Jan Kara <jack@...e.cz>
To:     Kemeng Shi <shikemeng@...weicloud.com>
Cc:     axboe@...nel.dk, hch@....de, jack@...e.cz,
        andriy.shevchenko@...ux.intel.com, qiulaibin@...wei.com,
        linux-block@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/7] blk-mq: count changed hctx as active in
 blk_mq_get_tag

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@...weicloud.com>

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

Reviewed-by: Jan Kara <jack@...e.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
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR