linux-kernel - Re: [PATCH for-6.12 2/4] block, bfq: choose the last bfqq from merge chain in bfq_setup

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240904121723.elseqvr277hzmnd6@quack3>
Date: Wed, 4 Sep 2024 14:17:23 +0200
From: Jan Kara <jack@...e.cz>
To: Yu Kuai <yukuai1@...weicloud.com>
Cc: jack@...e.cz, tj@...nel.org, josef@...icpanda.com, axboe@...nel.dk,
	paolo.valente@...more.it, mauro.andreolini@...more.it,
	avanzini.arianna@...il.com, cgroups@...r.kernel.org,
	linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
	yukuai3@...wei.com, yi.zhang@...wei.com, yangerkun@...wei.com
Subject: Re: [PATCH for-6.12 2/4] block, bfq: choose the last bfqq from merge
 chain in bfq_setup_cooperator()

On Mon 02-09-24 21:03:27, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@...wei.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@...wei.com>

Good catch. Feel free to add:

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