Re: [PATCH 2/8] loop: initialize the worker tracking fields once

From: Jan Kara <jack@suse.cz>
To: Christoph Hellwig <hch@lst.de>
Cc: Jens Axboe <axboe@kernel.dk>,
	Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	Jan Kara <jack@suse.cz>,
	linux-block@vger.kernel.org, Ming Lei <ming.lei@redhat.com>
Subject: Re: [PATCH 2/8] loop: initialize the worker tracking fields once
Date: Thu, 27 Jan 2022 10:45:30 +0100	[thread overview]
Message-ID: <20220127094530.wpneoub2nkdxzxmq@quack3.lan> (raw)
In-Reply-To: <20220126155040.1190842-3-hch@lst.de>

On Wed 26-01-22 16:50:34, Christoph Hellwig wrote:
> There is no need to reinitialize idle_worker_list, worker_tree and timer
> every time a loop device is configured.  Just initialize them once at
> allocation time.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good. Feel free to add:

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

								Honza

> ---
>  drivers/block/loop.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index b268bca6e4fb7..6ec55a5d9dfc4 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -1052,10 +1052,6 @@ static int loop_configure(struct loop_device *lo, fmode_t mode,
>  
>  	INIT_WORK(&lo->rootcg_work, loop_rootcg_workfn);
>  	INIT_LIST_HEAD(&lo->rootcg_cmd_list);
> -	INIT_LIST_HEAD(&lo->idle_worker_list);
> -	lo->worker_tree = RB_ROOT;
> -	timer_setup(&lo->timer, loop_free_idle_workers_timer,
> -		TIMER_DEFERRABLE);
>  	lo->use_dio = lo->lo_flags & LO_FLAGS_DIRECT_IO;
>  	lo->lo_device = bdev;
>  	lo->lo_backing_file = file;
> @@ -1957,6 +1953,9 @@ static int loop_add(int i)
>  	lo = kzalloc(sizeof(*lo), GFP_KERNEL);
>  	if (!lo)
>  		goto out;
> +	lo->worker_tree = RB_ROOT;
> +	INIT_LIST_HEAD(&lo->idle_worker_list);
> +	timer_setup(&lo->timer, loop_free_idle_workers_timer, TIMER_DEFERRABLE);
>  	lo->lo_state = Lo_unbound;
>  
>  	err = mutex_lock_killable(&loop_ctl_mutex);
> -- 
> 2.30.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR