linux-kernel - Re: [PATCH RFC v2 2/6] blk-throttle: delay initialization until configuration

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: <Zhl2a2m3L3QEELtj@slm.duckdns.org>
Date: Fri, 12 Apr 2024 07:59:07 -1000
From: Tejun Heo <tj@...nel.org>
To: Yu Kuai <yukuai1@...weicloud.com>
Cc: axboe@...nel.dk, chenhuacai@...nel.org, josef@...icpanda.com,
	jhs@...atatu.com, svenjoac@....de, raven@...maw.net,
	pctammela@...atatu.com, yukuai3@...wei.com, qde@...cy.de,
	zhaotianrui@...ngson.cn, linux-block@...r.kernel.org,
	linux-kernel@...r.kernel.org, loongarch@...ts.linux.dev,
	cgroups@...r.kernel.org, yi.zhang@...wei.com, yangerkun@...wei.com
Subject: Re: [PATCH RFC v2 2/6] blk-throttle: delay initialization until
 configuration

Hello,

On Sat, Apr 06, 2024 at 04:00:55PM +0800, Yu Kuai wrote:
> @@ -1480,6 +1547,9 @@ void blk_throtl_cancel_bios(struct gendisk *disk)
>  	struct cgroup_subsys_state *pos_css;
>  	struct blkcg_gq *blkg;
>  
> +	if (!q->td)
> +		return;

So, this naked test is safe because the interface functions are shut down by
the time this function is called.

>  static inline bool blk_should_throtl(struct bio *bio)
>  {
> -	struct throtl_grp *tg = blkg_to_tg(bio->bi_blkg);
> +	struct throtl_grp *tg;
>  	int rw = bio_data_dir(bio);
>  
> +	if (!bio->bi_bdev->bd_queue->td)
> +		return false;

and this one because ->td is set while the queue is frozen and this path
shouldn't be running while it gets set, right?

Can you please add comments explaining why those are safe? Otherwise, the
patch looks generally sane to me on the first glance. Can you please also
add how you tested the change?

Thanks.

-- 
tejun