linux-kernel - Re: [PATCH for-6.12 3/4] block, bfq: don't break merge chain in bfq_split

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: <20240904122043.nyaouj7qt2fyp6xa@quack3>
Date: Wed, 4 Sep 2024 14:20:43 +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 3/4] block, bfq: don't break merge chain in
 bfq_split_bfqq()

On Mon 02-09-24 21:03:28, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@...wei.com>
> 
> Consider the following scenario:
> 
>     Process 1       Process 2       Process 3       Process 4
>      (BIC1)          (BIC2)          (BIC3)          (BIC4)
>       Λ               |               |                |
>        \-------------\ \-------------\ \--------------\|
>                       V               V                V
>       bfqq1--------->bfqq2---------->bfqq3----------->bfqq4
> ref    0              1               2                4
> 
> If Process 1 issue a new IO and bfqq2 is found, and then bfq_init_rq()
> decide to spilt bfqq2 by bfq_split_bfqq(). Howerver, procress reference
> of bfqq2 is 1 and bfq_split_bfqq() just clear the coop flag, which will
> break the merge chain.
> 
> Expected result: caller will allocate a new bfqq for BIC1
> 
>     Process 1       Process 2       Process 3       Process 4
>      (BIC1)          (BIC2)          (BIC3)          (BIC4)
>                       |               |                |
>                        \-------------\ \--------------\|
>                                       V                V
>       bfqq1--------->bfqq2---------->bfqq3----------->bfqq4
> ref    0              0               1                3
> 
> Since the condition is only used for the last bfqq4 when the previous
> bfqq2 and bfqq3 are already splited. Fix the problem by checking if
> bfqq is the last one in the merge chain as well.
> 
> Fixes: 36eca8948323 ("block, bfq: add Early Queue Merge (EQM)")
> Signed-off-by: Yu Kuai <yukuai3@...wei.com>

Looks good. Feel free to add:

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

								Honza

> ---
>  block/bfq-iosched.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index ffaa0d56328a..ca766b7d5560 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -6727,7 +6727,7 @@ bfq_split_bfqq(struct bfq_io_cq *bic, struct bfq_queue *bfqq)
>  {
>  	bfq_log_bfqq(bfqq->bfqd, bfqq, "splitting queue");
>  
> -	if (bfqq_process_refs(bfqq) == 1) {
> +	if (bfqq_process_refs(bfqq) == 1 && !bfqq->new_bfqq) {
>  		bfqq->pid = current->pid;
>  		bfq_clear_bfqq_coop(bfqq);
>  		bfq_clear_bfqq_split_coop(bfqq);
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR