On Wed 26-01-22 16:50:33, Christoph Hellwig wrote:
> Use a common helper for both timer based and uncoditional freeing of idle
> workers.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Nice cleanup. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> drivers/block/loop.c | 73 +++++++++++++++++++++-----------------------
> 1 file changed, 35 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 01cbbfc4e9e24..b268bca6e4fb7 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -804,7 +804,6 @@ struct loop_worker {
>
> static void loop_workfn(struct work_struct *work);
> static void loop_rootcg_workfn(struct work_struct *work);
> -static void loop_free_idle_workers(struct timer_list *timer);
>
> #ifdef CONFIG_BLK_CGROUP
> static inline int queue_on_root_worker(struct cgroup_subsys_state *css)
> @@ -888,6 +887,39 @@ static void loop_queue_work(struct loop_device *lo, struct loop_cmd *cmd)
> spin_unlock_irq(&lo->lo_work_lock);
> }
>
> +static void loop_set_timer(struct loop_device *lo)
> +{
> + timer_reduce(&lo->timer, jiffies + LOOP_IDLE_WORKER_TIMEOUT);
> +}
> +
> +static void loop_free_idle_workers(struct loop_device *lo, bool delete_all)
> +{
> + struct loop_worker *pos, *worker;
> +
> + spin_lock_irq(&lo->lo_work_lock);
> + list_for_each_entry_safe(worker, pos, &lo->idle_worker_list,
> + idle_list) {
> + if (!delete_all &&
> + time_is_after_jiffies(worker->last_ran_at +
> + LOOP_IDLE_WORKER_TIMEOUT))
> + break;
> + list_del(&worker->idle_list);
> + rb_erase(&worker->rb_node, &lo->worker_tree);
> + css_put(worker->blkcg_css);
> + kfree(worker);
> + }
> + if (!list_empty(&lo->idle_worker_list))
> + loop_set_timer(lo);
> + spin_unlock_irq(&lo->lo_work_lock);
> +}
> +
> +static void loop_free_idle_workers_timer(struct timer_list *timer)
> +{
> + struct loop_device *lo = container_of(timer, struct loop_device, timer);
> +
> + return loop_free_idle_workers(lo, false);
> +}
> +
> static void loop_update_rotational(struct loop_device *lo)
> {
> struct file *file = lo->lo_backing_file;
> @@ -1022,7 +1054,7 @@ static int loop_configure(struct loop_device *lo, fmode_t mode,
> 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_setup(&lo->timer, loop_free_idle_workers_timer,
> TIMER_DEFERRABLE);
> lo->use_dio = lo->lo_flags & LO_FLAGS_DIRECT_IO;
> lo->lo_device = bdev;
> @@ -1086,7 +1118,6 @@ static void __loop_clr_fd(struct loop_device *lo)
> {
> struct file *filp;
> gfp_t gfp = lo->old_gfp_mask;
> - struct loop_worker *pos, *worker;
>
> /*
> * Flush loop_configure() and loop_change_fd(). It is acceptable for
> @@ -1116,15 +1147,7 @@ static void __loop_clr_fd(struct loop_device *lo)
> blk_mq_freeze_queue(lo->lo_queue);
>
> destroy_workqueue(lo->workqueue);
> - spin_lock_irq(&lo->lo_work_lock);
> - list_for_each_entry_safe(worker, pos, &lo->idle_worker_list,
> - idle_list) {
> - list_del(&worker->idle_list);
> - rb_erase(&worker->rb_node, &lo->worker_tree);
> - css_put(worker->blkcg_css);
> - kfree(worker);
> - }
> - spin_unlock_irq(&lo->lo_work_lock);
> + loop_free_idle_workers(lo, true);
> del_timer_sync(&lo->timer);
>
> spin_lock_irq(&lo->lo_lock);
> @@ -1871,11 +1894,6 @@ static void loop_handle_cmd(struct loop_cmd *cmd)
> }
> }
>
> -static void loop_set_timer(struct loop_device *lo)
> -{
> - timer_reduce(&lo->timer, jiffies + LOOP_IDLE_WORKER_TIMEOUT);
> -}
> -
> static void loop_process_work(struct loop_worker *worker,
> struct list_head *cmd_list, struct loop_device *lo)
> {
> @@ -1924,27 +1942,6 @@ static void loop_rootcg_workfn(struct work_struct *work)
> loop_process_work(NULL, &lo->rootcg_cmd_list, lo);
> }
>
> -static void loop_free_idle_workers(struct timer_list *timer)
> -{
> - struct loop_device *lo = container_of(timer, struct loop_device, timer);
> - struct loop_worker *pos, *worker;
> -
> - spin_lock_irq(&lo->lo_work_lock);
> - list_for_each_entry_safe(worker, pos, &lo->idle_worker_list,
> - idle_list) {
> - if (time_is_after_jiffies(worker->last_ran_at +
> - LOOP_IDLE_WORKER_TIMEOUT))
> - break;
> - list_del(&worker->idle_list);
> - rb_erase(&worker->rb_node, &lo->worker_tree);
> - css_put(worker->blkcg_css);
> - kfree(worker);
> - }
> - if (!list_empty(&lo->idle_worker_list))
> - loop_set_timer(lo);
> - spin_unlock_irq(&lo->lo_work_lock);
> -}
> -
> static const struct blk_mq_ops loop_mq_ops = {
> .queue_rq = loop_queue_rq,
> .complete = lo_complete_rq,
> --
> 2.30.2
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
