linux-kernel - Re: [PATCH block/for-linus] block: don't release bdi while request

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] [day] [month] [year] [list]

Date:	Thu, 15 Oct 2015 11:50:49 -0400
From:	Jeff Moyer <jmoyer@...hat.com>
To:	Tejun Heo <tj@...nel.org>
Cc:	Jens Axboe <axboe@...com>,
	Andrey Konovalov <andreyknvl@...gle.com>,
	Jan Kara <jack@...e.cz>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, Dmitry Vyukov <dvyukov@...gle.com>,
	Alexander Potapenko <glider@...gle.com>,
	Kostya Serebryany <kcc@...gle.com>, kernel-team@...com
Subject: Re: [PATCH block/for-linus] block: don't release bdi while request_queue has live references

Tejun Heo <tj@...nel.org> writes:

[snip]

Thanks for the nice write-up of the problem.  This looks good to me.
The only minor nit I have is that you might want to rename
cgwb_bdi_destroy to cgwb_bdi_unregister.

Reviewed-by: Jeff Moyer <jmoyer@...hat.com>


> Signed-off-by: Tejun Heo <tj@...nel.org>
> Fixes: a13f35e87140 ("writeback: don't embed root bdi_writeback_congested in bdi_writeback")
> Cc: stable@...r.kernel.org # v4.2+
> Reported-and-tested-by: Andrey Konovalov <andreyknvl@...gle.com>
> Link: http://lkml.kernel.org/g/CAAeHK+zUJ74Zn17=rOyxacHU18SgCfC6bsYW=6kCY5GXJBwGfQ@mail.gmail.com
> ---
>  block/blk-core.c            |    2 +-
>  block/blk-sysfs.c           |    1 +
>  include/linux/backing-dev.h |    6 +++++-
>  mm/backing-dev.c            |   12 +++++++++++-
>  4 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 60912e9..ae49240 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -576,7 +576,7 @@ void blk_cleanup_queue(struct request_queue *q)
>  		q->queue_lock = &q->__queue_lock;
>  	spin_unlock_irq(lock);
>  
> -	bdi_destroy(&q->backing_dev_info);
> +	bdi_unregister(&q->backing_dev_info);
>  
>  	/* @q is and will stay empty, shutdown and put */
>  	blk_put_queue(q);
> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
> index 3e44a9d..07b42f5 100644
> --- a/block/blk-sysfs.c
> +++ b/block/blk-sysfs.c
> @@ -540,6 +540,7 @@ static void blk_release_queue(struct kobject *kobj)
>  	struct request_queue *q =
>  		container_of(kobj, struct request_queue, kobj);
>  
> +	bdi_exit(&q->backing_dev_info);
>  	blkcg_exit_queue(q);
>  
>  	if (q->elevator) {
> diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
> index 0fe9df9..fe0ab98 100644
> --- a/include/linux/backing-dev.h
> +++ b/include/linux/backing-dev.h
> @@ -18,13 +18,17 @@
>  #include <linux/slab.h>
>  
>  int __must_check bdi_init(struct backing_dev_info *bdi);
> -void bdi_destroy(struct backing_dev_info *bdi);
> +void bdi_exit(struct backing_dev_info *bdi);
>  
>  __printf(3, 4)
>  int bdi_register(struct backing_dev_info *bdi, struct device *parent,
>  		const char *fmt, ...);
>  int bdi_register_dev(struct backing_dev_info *bdi, dev_t dev);
> +void bdi_unregister(struct backing_dev_info *bdi);
> +
>  int __must_check bdi_setup_and_register(struct backing_dev_info *, char *);
> +void bdi_destroy(struct backing_dev_info *bdi);
> +
>  void wb_start_writeback(struct bdi_writeback *wb, long nr_pages,
>  			bool range_cyclic, enum wb_reason reason);
>  void wb_start_background_writeback(struct bdi_writeback *wb);
> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> index dac5bf5..dc07d88 100644
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -823,7 +823,7 @@ static void bdi_remove_from_list(struct backing_dev_info *bdi)
>  	synchronize_rcu_expedited();
>  }
>  
> -void bdi_destroy(struct backing_dev_info *bdi)
> +void bdi_unregister(struct backing_dev_info *bdi)
>  {
>  	/* make sure nobody finds us on the bdi_list anymore */
>  	bdi_remove_from_list(bdi);
> @@ -835,9 +835,19 @@ void bdi_destroy(struct backing_dev_info *bdi)
>  		device_unregister(bdi->dev);
>  		bdi->dev = NULL;
>  	}
> +}
>  
> +void bdi_exit(struct backing_dev_info *bdi)
> +{
> +	WARN_ON_ONCE(bdi->dev);
>  	wb_exit(&bdi->wb);
>  }
> +
> +void bdi_destroy(struct backing_dev_info *bdi)
> +{
> +	bdi_unregister(bdi);
> +	bdi_exit(bdi);
> +}
>  EXPORT_SYMBOL(bdi_destroy);
>  
>  /*
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/