Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: Re: [PATCH -next] blk-wbt: call rq_qos_add() after wb_normal is
 initialized
To:     Yu Kuai <yukuai1@huaweicloud.com>, axboe@kernel.dk
Cc:     linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        yi.zhang@huawei.com, "yukuai (C)" <yukuai3@huawei.com>
References: <20220913105749.3086243-1-yukuai1@huaweicloud.com>
From:   Yu Kuai <yukuai1@huaweicloud.com>
Message-ID: <eef13e74-9f5b-23d9-07d8-7ee8f5e30012@huaweicloud.com>
Date:   Wed, 21 Sep 2022 17:45:08 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <20220913105749.3086243-1-yukuai1@huaweicloud.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


在 2022/09/13 18:57, Yu Kuai 写道:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> Our test found a problem that wbt inflight counter is negative, which
> will cause io hang(noted that this problem doesn't exist in mainline):
> 
> t1: device create	t2: issue io
> add_disk
>   blk_register_queue
>    wbt_enable_default
>     wbt_init
>      rq_qos_add
>      // wb_normal is still 0
> 			/*
> 			 * in mainline, disk can't be opened before
> 			 * bdev_add(), however, in old kernels, disk
> 			 * can be opened before blk_register_queue().
> 			 */
> 			blkdev_issue_flush
>                          // disk size is 0, however, it's not checked
>                           submit_bio_wait
>                            submit_bio
>                             blk_mq_submit_bio
>                              rq_qos_throttle
>                               wbt_wait
> 			      bio_to_wbt_flags
>                                 rwb_enabled
> 			       // wb_normal is 0, inflight is not increased
> 
>      wbt_queue_depth_changed(&rwb->rqos);
>       wbt_update_limits
>       // wb_normal is initialized
>                              rq_qos_track
>                               wbt_track
>                                rq->wbt_flags |= bio_to_wbt_flags(rwb, bio);
> 			      // wb_normal is not 0，wbt_flags will be set
> t3: io completion
> blk_mq_free_request
>   rq_qos_done
>    wbt_done
>     wbt_is_tracked
>     // return true
>     __wbt_done
>      wbt_rqw_done
>       atomic_dec_return(&rqw->inflight);
>       // inflight is decreased
> 
> commit 8235b5c1e8c1 ("block: call bdev_add later in device_add_disk") can
> avoid this problem, however it's better to fix this problem in wbt:
> 
> 1) Lower kernel can't backport this patch due to lots of refactor.
> 2) Root cause is that wbt call rq_qos_add() before wb_normal is
> initialized.
> 
friendly ping ...
> Fixes: e34cbd307477 ("blk-wbt: add general throttling mechanism")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
>   block/blk-wbt.c | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/block/blk-wbt.c b/block/blk-wbt.c
> index a9982000b667..246467926253 100644
> --- a/block/blk-wbt.c
> +++ b/block/blk-wbt.c
> @@ -843,6 +843,10 @@ int wbt_init(struct request_queue *q)
>   	rwb->enable_state = WBT_STATE_ON_DEFAULT;
>   	rwb->wc = 1;
>   	rwb->rq_depth.default_depth = RWB_DEF_DEPTH;
> +	rwb->min_lat_nsec = wbt_default_latency_nsec(q);
> +
> +	wbt_queue_depth_changed(&rwb->rqos);
> +	wbt_set_write_cache(q, test_bit(QUEUE_FLAG_WC, &q->queue_flags));
>   
>   	/*
>   	 * Assign rwb and add the stats callback.
> @@ -853,11 +857,6 @@ int wbt_init(struct request_queue *q)
>   
>   	blk_stat_add_callback(q, rwb->cb);
>   
> -	rwb->min_lat_nsec = wbt_default_latency_nsec(q);
> -
> -	wbt_queue_depth_changed(&rwb->rqos);
> -	wbt_set_write_cache(q, test_bit(QUEUE_FLAG_WC, &q->queue_flags));
> -
>   	return 0;
>   
>   err_free:
>