[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y/uPFT7w+XQ9l0XE@slm.duckdns.org>
Date: Sun, 26 Feb 2023 06:55:49 -1000
From: Tejun Heo <tj@...nel.org>
To: Breno Leitao <leitao@...ian.org>
Cc: axboe@...nel.dk, josef@...icpanda.com, cgroups@...r.kernel.org,
linux-block@...r.kernel.org, aherrmann@...e.de,
linux-kernel@...r.kernel.org, hch@....de, leit@...com
Subject: Re: [PATCH] blk-iocost: initialize rqos before accessing it
Hello, Breno.
On Fri, Feb 24, 2023 at 08:07:14AM -0800, Breno Leitao wrote:
> diff --git a/block/blk-iocost.c b/block/blk-iocost.c
> index ff534e9d92dc..6cced8a76e9c 100644
> --- a/block/blk-iocost.c
> +++ b/block/blk-iocost.c
> @@ -2878,11 +2878,6 @@ static int blk_iocost_init(struct gendisk *disk)
> atomic64_set(&ioc->cur_period, 0);
> atomic_set(&ioc->hweight_gen, 0);
>
> - spin_lock_irq(&ioc->lock);
> - ioc->autop_idx = AUTOP_INVALID;
> - ioc_refresh_params(ioc, true);
> - spin_unlock_irq(&ioc->lock);
> -
> /*
> * rqos must be added before activation to allow ioc_pd_init() to
> * lookup the ioc from q. This means that the rqos methods may get
> @@ -2893,6 +2888,11 @@ static int blk_iocost_init(struct gendisk *disk)
> if (ret)
> goto err_free_ioc;
>
> + spin_lock_irq(&ioc->lock);
> + ioc->autop_idx = AUTOP_INVALID;
> + ioc_refresh_params(ioc, true);
> + spin_unlock_irq(&ioc->lock);
> +
I'm a bit worried about registering the rqos before ioc_refresh_params() as
that initializes all the internal parameters and letting IOs flow through
without initializing them can lead to subtle issues. Can you please instead
explicitly pass @q into ioc_refresh_params() (and explain why we need it
passed explicitly in the function comment)?
Thanks.
--
tejun
Powered by blists - more mailing lists