Re: [PATCH for-6.12 2/4] block, bfq: choose the last bfqq from merge chain in bfq_setup_cooperator()

[Jan Kara <jack@suse.cz>]

On Mon 02-09-24 21:03:27, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> Consider the following merge chain:
> 
> Process 1       Process 2       Process 3	Process 4
>  (BIC1)          (BIC2)          (BIC3)		 (BIC4)
>   Λ                |               |               |
>    \--------------\ \-------------\ \-------------\|
>                    V               V		   V
>   bfqq1--------->bfqq2---------->bfqq3----------->bfqq4
> 
> IO from Process 1 will get bfqf2 from BIC1 first, then
> bfq_setup_cooperator() will found bfqq2 already merged to bfqq3 and then
> handle this IO from bfqq3. However, the merge chain can be much deeper
> and bfqq3 can be merged to other bfqq as well.
> 
> Fix this problem by iterating to the last bfqq in
> bfq_setup_cooperator().
> 
> Fixes: 36eca8948323 ("block, bfq: add Early Queue Merge (EQM)")
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>

Good catch. Feel free to add:

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

								Honza

> ---
>  block/bfq-iosched.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 83adac3e71db..ffaa0d56328a 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -2911,8 +2911,12 @@ bfq_setup_cooperator(struct bfq_data *bfqd, struct bfq_queue *bfqq,
>  	struct bfq_iocq_bfqq_data *bfqq_data = &bic->bfqq_data[a_idx];
>  
>  	/* if a merge has already been setup, then proceed with that first */
> -	if (bfqq->new_bfqq)
> -		return bfqq->new_bfqq;
> +	new_bfqq = bfqq->new_bfqq;
> +	if (new_bfqq) {
> +		while (new_bfqq->new_bfqq)
> +			new_bfqq = new_bfqq->new_bfqq;
> +		return new_bfqq;
> +	}
>  
>  	/*
>  	 * Check delayed stable merge for rotational or non-queueing
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR