move the bdi from the request_queue to the gendisk

move the bdi from the request_queue to the gendisk

[Christoph Hellwig]

Hi Jens,

this series moves the pointer to the bdi from the request_queue
to the bdi, better matching the life time rules of the different
objects.

Diffstat:
 block/bfq-iosched.c           |    4 ++--
 block/blk-cgroup.c            |    7 +++----
 block/blk-core.c              |   18 +++---------------
 block/blk-mq.c                |    2 +-
 block/blk-settings.c          |   22 ++++++++++++++--------
 block/blk-sysfs.c             |   28 +++++++++++++---------------
 block/blk-wbt.c               |   10 +++++-----
 block/genhd.c                 |   23 ++++++++++++++---------
 block/ioctl.c                 |    7 ++++---
 drivers/block/drbd/drbd_nl.c  |    2 +-
 drivers/block/drbd/drbd_req.c |    5 ++---
 drivers/block/pktcdvd.c       |    8 +++-----
 drivers/md/dm-table.c         |    2 +-
 drivers/nvme/host/core.c      |    2 +-
 fs/block_dev.c                |   13 +------------
 fs/fat/fatent.c               |    1 +
 fs/nilfs2/super.c             |    2 +-
 fs/super.c                    |    2 +-
 fs/xfs/xfs_buf.c              |    2 +-
 include/linux/backing-dev.h   |    2 +-
 include/linux/blk_types.h     |    1 -
 include/linux/blkdev.h        |    6 ++----
 include/linux/genhd.h         |    1 +
 mm/backing-dev.c              |    3 +++
 mm/page-writeback.c           |    2 --
 25 files changed, 79 insertions(+), 96 deletions(-)

[PATCH 1/5] mm: hide laptop_mode_wb_timer entirely behind the BDI API

[Christoph Hellwig]

Don't leak the detaіls of the timer into the block layer, instead
initialize the timer in bdi_alloc and delete it in bdi_unregister.
Note that this means the timer is initialized (but not armed) for
non-block queues as well now.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-core.c    | 5 -----
 mm/backing-dev.c    | 3 +++
 mm/page-writeback.c | 2 --
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 04477697ee4b..5897bc37467d 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -394,10 +394,7 @@ void blk_cleanup_queue(struct request_queue *q)
 	/* for synchronous bio-based driver finish in-flight integrity i/o */
 	blk_flush_integrity();
 
-	/* @q won't process any more request, flush async actions */
-	del_timer_sync(&q->backing_dev_info->laptop_mode_wb_timer);
 	blk_sync_queue(q);
-
 	if (queue_is_mq(q))
 		blk_mq_exit_queue(q);
 
@@ -546,8 +543,6 @@ struct request_queue *blk_alloc_queue(int node_id)
 
 	atomic_set(&q->nr_active_requests_shared_sbitmap, 0);
 
-	timer_setup(&q->backing_dev_info->laptop_mode_wb_timer,
-		    laptop_mode_timer_fn, 0);
 	timer_setup(&q->timeout, blk_rq_timed_out_timer, 0);
 	INIT_WORK(&q->timeout_work, blk_timeout_work);
 	INIT_LIST_HEAD(&q->icq_list);
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index f5561ea7d90a..cd06dca232c3 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -807,6 +807,7 @@ struct backing_dev_info *bdi_alloc(int node_id)
 	bdi->capabilities = BDI_CAP_WRITEBACK | BDI_CAP_WRITEBACK_ACCT;
 	bdi->ra_pages = VM_READAHEAD_PAGES;
 	bdi->io_pages = VM_READAHEAD_PAGES;
+	timer_setup(&bdi->laptop_mode_wb_timer, laptop_mode_timer_fn, 0);
 	return bdi;
 }
 EXPORT_SYMBOL(bdi_alloc);
@@ -928,6 +929,8 @@ static void bdi_remove_from_list(struct backing_dev_info *bdi)
 
 void bdi_unregister(struct backing_dev_info *bdi)
 {
+	del_timer_sync(&bdi->laptop_mode_wb_timer);
+
 	/* make sure nobody finds us on the bdi_list anymore */
 	bdi_remove_from_list(bdi);
 	wb_shutdown(&bdi->wb);
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 9f63548f247c..c12f67cbfa19 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2010,7 +2010,6 @@ int dirty_writeback_centisecs_handler(struct ctl_table *table, int write,
 	return ret;
 }
 
-#ifdef CONFIG_BLOCK
 void laptop_mode_timer_fn(struct timer_list *t)
 {
 	struct backing_dev_info *backing_dev_info =
@@ -2045,7 +2044,6 @@ void laptop_sync_completion(void)
 
 	rcu_read_unlock();
 }
-#endif
 
 /*
  * If ratelimit_pages is too high then we can get into dirty-data overload
-- 
2.30.2

[PATCH 1/5] mm: hide laptop_mode_wb_timer entirely behind the BDI API

[Christoph Hellwig]

Don't leak the deta—ñls of the timer into the block layer, instead
initialize the timer in bdi_alloc and delete it in bdi_unregister.
Note that this means the timer is initialized (but not armed) for
non-block queues as well now.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-core.c    | 5 -----
 mm/backing-dev.c    | 3 +++
 mm/page-writeback.c | 2 --
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 04477697ee4b..5897bc37467d 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -394,10 +394,7 @@ void blk_cleanup_queue(struct request_queue *q)
 	/* for synchronous bio-based driver finish in-flight integrity i/o */
 	blk_flush_integrity();
 
-	/* @q won't process any more request, flush async actions */
-	del_timer_sync(&q->backing_dev_info->laptop_mode_wb_timer);
 	blk_sync_queue(q);
-
 	if (queue_is_mq(q))
 		blk_mq_exit_queue(q);
 
@@ -546,8 +543,6 @@ struct request_queue *blk_alloc_queue(int node_id)
 
 	atomic_set(&q->nr_active_requests_shared_sbitmap, 0);
 
-	timer_setup(&q->backing_dev_info->laptop_mode_wb_timer,
-		    laptop_mode_timer_fn, 0);
 	timer_setup(&q->timeout, blk_rq_timed_out_timer, 0);
 	INIT_WORK(&q->timeout_work, blk_timeout_work);
 	INIT_LIST_HEAD(&q->icq_list);
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index f5561ea7d90a..cd06dca232c3 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -807,6 +807,7 @@ struct backing_dev_info *bdi_alloc(int node_id)
 	bdi->capabilities = BDI_CAP_WRITEBACK | BDI_CAP_WRITEBACK_ACCT;
 	bdi->ra_pages = VM_READAHEAD_PAGES;
 	bdi->io_pages = VM_READAHEAD_PAGES;
+	timer_setup(&bdi->laptop_mode_wb_timer, laptop_mode_timer_fn, 0);
 	return bdi;
 }
 EXPORT_SYMBOL(bdi_alloc);
@@ -928,6 +929,8 @@ static void bdi_remove_from_list(struct backing_dev_info *bdi)
 
 void bdi_unregister(struct backing_dev_info *bdi)
 {
+	del_timer_sync(&bdi->laptop_mode_wb_timer);
+
 	/* make sure nobody finds us on the bdi_list anymore */
 	bdi_remove_from_list(bdi);
 	wb_shutdown(&bdi->wb);
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 9f63548f247c..c12f67cbfa19 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -2010,7 +2010,6 @@ int dirty_writeback_centisecs_handler(struct ctl_table *table, int write,
 	return ret;
 }
 
-#ifdef CONFIG_BLOCK
 void laptop_mode_timer_fn(struct timer_list *t)
 {
 	struct backing_dev_info *backing_dev_info =
@@ -2045,7 +2044,6 @@ void laptop_sync_completion(void)
 
 	rcu_read_unlock();
 }
-#endif
 
 /*
  * If ratelimit_pages is too high then we can get into dirty-data overload
-- 
2.30.2

Re: [PATCH 1/5] mm: hide laptop_mode_wb_timer entirely behind the BDI API

[Johannes Thumshirn]

On 09/08/2021 16:19, Christoph Hellwig wrote:
> Don't leak the detaіls of the timer into the block layer, instead
> initialize the timer in bdi_alloc and delete it in bdi_unregister.
> Note that this means the timer is initialized (but not armed) for
> non-block queues as well now.

And laptop_mode_timer_fn() is always present now.

Other than that,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 1/5] mm: hide laptop_mode_wb_timer entirely behind the BDI API

[Johannes Thumshirn]

On 09/08/2021 16:19, Christoph Hellwig wrote:
> Don't leak the deta¦ls of the timer into the block layer, instead
> initialize the timer in bdi_alloc and delete it in bdi_unregister.
> Note that this means the timer is initialized (but not armed) for
> non-block queues as well now.

And laptop_mode_timer_fn() is always present now.

Other than that,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 1/5] mm: hide laptop_mode_wb_timer entirely behind the BDI API

[Jan Kara]

On Mon 09-08-21 16:17:40, Christoph Hellwig wrote:
> Don't leak the detaіls of the timer into the block layer, instead
> initialize the timer in bdi_alloc and delete it in bdi_unregister.
> Note that this means the timer is initialized (but not armed) for
> non-block queues as well now.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good. Feel free to add:

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

								Honza

> ---
>  block/blk-core.c    | 5 -----
>  mm/backing-dev.c    | 3 +++
>  mm/page-writeback.c | 2 --
>  3 files changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 04477697ee4b..5897bc37467d 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -394,10 +394,7 @@ void blk_cleanup_queue(struct request_queue *q)
>  	/* for synchronous bio-based driver finish in-flight integrity i/o */
>  	blk_flush_integrity();
>  
> -	/* @q won't process any more request, flush async actions */
> -	del_timer_sync(&q->backing_dev_info->laptop_mode_wb_timer);
>  	blk_sync_queue(q);
> -
>  	if (queue_is_mq(q))
>  		blk_mq_exit_queue(q);
>  
> @@ -546,8 +543,6 @@ struct request_queue *blk_alloc_queue(int node_id)
>  
>  	atomic_set(&q->nr_active_requests_shared_sbitmap, 0);
>  
> -	timer_setup(&q->backing_dev_info->laptop_mode_wb_timer,
> -		    laptop_mode_timer_fn, 0);
>  	timer_setup(&q->timeout, blk_rq_timed_out_timer, 0);
>  	INIT_WORK(&q->timeout_work, blk_timeout_work);
>  	INIT_LIST_HEAD(&q->icq_list);
> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> index f5561ea7d90a..cd06dca232c3 100644
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -807,6 +807,7 @@ struct backing_dev_info *bdi_alloc(int node_id)
>  	bdi->capabilities = BDI_CAP_WRITEBACK | BDI_CAP_WRITEBACK_ACCT;
>  	bdi->ra_pages = VM_READAHEAD_PAGES;
>  	bdi->io_pages = VM_READAHEAD_PAGES;
> +	timer_setup(&bdi->laptop_mode_wb_timer, laptop_mode_timer_fn, 0);
>  	return bdi;
>  }
>  EXPORT_SYMBOL(bdi_alloc);
> @@ -928,6 +929,8 @@ static void bdi_remove_from_list(struct backing_dev_info *bdi)
>  
>  void bdi_unregister(struct backing_dev_info *bdi)
>  {
> +	del_timer_sync(&bdi->laptop_mode_wb_timer);
> +
>  	/* make sure nobody finds us on the bdi_list anymore */
>  	bdi_remove_from_list(bdi);
>  	wb_shutdown(&bdi->wb);
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index 9f63548f247c..c12f67cbfa19 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -2010,7 +2010,6 @@ int dirty_writeback_centisecs_handler(struct ctl_table *table, int write,
>  	return ret;
>  }
>  
> -#ifdef CONFIG_BLOCK
>  void laptop_mode_timer_fn(struct timer_list *t)
>  {
>  	struct backing_dev_info *backing_dev_info =
> @@ -2045,7 +2044,6 @@ void laptop_sync_completion(void)
>  
>  	rcu_read_unlock();
>  }
> -#endif
>  
>  /*
>   * If ratelimit_pages is too high then we can get into dirty-data overload
> -- 
> 2.30.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 1/5] mm: hide laptop_mode_wb_timer entirely behind the BDI API

[Jan Kara]

On Mon 09-08-21 16:17:40, Christoph Hellwig wrote:
> Don't leak the detaÑ–ls of the timer into the block layer, instead
> initialize the timer in bdi_alloc and delete it in bdi_unregister.
> Note that this means the timer is initialized (but not armed) for
> non-block queues as well now.
> 
> Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>

								Honza

> ---
>  block/blk-core.c    | 5 -----
>  mm/backing-dev.c    | 3 +++
>  mm/page-writeback.c | 2 --
>  3 files changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 04477697ee4b..5897bc37467d 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -394,10 +394,7 @@ void blk_cleanup_queue(struct request_queue *q)
>  	/* for synchronous bio-based driver finish in-flight integrity i/o */
>  	blk_flush_integrity();
>  
> -	/* @q won't process any more request, flush async actions */
> -	del_timer_sync(&q->backing_dev_info->laptop_mode_wb_timer);
>  	blk_sync_queue(q);
> -
>  	if (queue_is_mq(q))
>  		blk_mq_exit_queue(q);
>  
> @@ -546,8 +543,6 @@ struct request_queue *blk_alloc_queue(int node_id)
>  
>  	atomic_set(&q->nr_active_requests_shared_sbitmap, 0);
>  
> -	timer_setup(&q->backing_dev_info->laptop_mode_wb_timer,
> -		    laptop_mode_timer_fn, 0);
>  	timer_setup(&q->timeout, blk_rq_timed_out_timer, 0);
>  	INIT_WORK(&q->timeout_work, blk_timeout_work);
>  	INIT_LIST_HEAD(&q->icq_list);
> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> index f5561ea7d90a..cd06dca232c3 100644
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -807,6 +807,7 @@ struct backing_dev_info *bdi_alloc(int node_id)
>  	bdi->capabilities = BDI_CAP_WRITEBACK | BDI_CAP_WRITEBACK_ACCT;
>  	bdi->ra_pages = VM_READAHEAD_PAGES;
>  	bdi->io_pages = VM_READAHEAD_PAGES;
> +	timer_setup(&bdi->laptop_mode_wb_timer, laptop_mode_timer_fn, 0);
>  	return bdi;
>  }
>  EXPORT_SYMBOL(bdi_alloc);
> @@ -928,6 +929,8 @@ static void bdi_remove_from_list(struct backing_dev_info *bdi)
>  
>  void bdi_unregister(struct backing_dev_info *bdi)
>  {
> +	del_timer_sync(&bdi->laptop_mode_wb_timer);
> +
>  	/* make sure nobody finds us on the bdi_list anymore */
>  	bdi_remove_from_list(bdi);
>  	wb_shutdown(&bdi->wb);
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index 9f63548f247c..c12f67cbfa19 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -2010,7 +2010,6 @@ int dirty_writeback_centisecs_handler(struct ctl_table *table, int write,
>  	return ret;
>  }
>  
> -#ifdef CONFIG_BLOCK
>  void laptop_mode_timer_fn(struct timer_list *t)
>  {
>  	struct backing_dev_info *backing_dev_info =
> @@ -2045,7 +2044,6 @@ void laptop_sync_completion(void)
>  
>  	rcu_read_unlock();
>  }
> -#endif
>  
>  /*
>   * If ratelimit_pages is too high then we can get into dirty-data overload
> -- 
> 2.30.2
> 
-- 
Jan Kara <jack-IBi9RG/b67k@public.gmane.org>
SUSE Labs, CR

Re: [PATCH 1/5] mm: hide laptop_mode_wb_timer entirely behind the BDI API

[Guenter Roeck]

On Mon, Aug 09, 2021 at 04:17:40PM +0200, Christoph Hellwig wrote:
> Don't leak the detaіls of the timer into the block layer, instead
> initialize the timer in bdi_alloc and delete it in bdi_unregister.
> Note that this means the timer is initialized (but not armed) for
> non-block queues as well now.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Just in case this hasn't been reported yet.
This patch results in a widespread build failure. Example:

Building x86_64:tinyconfig ... failed
--------------
Error log:
mm/page-writeback.c:2044:6: error: redefinition of 'laptop_sync_completion'
 2044 | void laptop_sync_completion(void)
      |      ^~~~~~~~~~~~~~~~~~~~~~
In file included from include/linux/memcontrol.h:22,
                 from include/linux/swap.h:9,
                 from mm/page-writeback.c:20:
include/linux/writeback.h:345:20: note: previous definition of 'laptop_sync_completion' with type 'void(void)'
  345 | static inline void laptop_sync_completion(void) { }
      |                    ^~~~~~~~~~~~~~~~~~~~~~
make[2]: *** [scripts/Makefile.build:272: mm/page-writeback.o] Error 1

Guenter

---
bisect log:

# bad: [92d00774360dfd4151f15ab9905c643347b9f242] Add linux-next specific files for 20210810
# good: [36a21d51725af2ce0700c6ebcb6b9594aac658a6] Linux 5.14-rc5
git bisect start 'HEAD' 'v5.14-rc5'
# good: [01dda625c9b7cfd3bf5ac05f73da8c512792f94c] Merge remote-tracking branch 'crypto/master'
git bisect good 01dda625c9b7cfd3bf5ac05f73da8c512792f94c
# bad: [75cadd49361c6650764d35bcbb6c9cb9f0a9d9a3] Merge remote-tracking branch 'irqchip/irq/irqchip-next'
git bisect bad 75cadd49361c6650764d35bcbb6c9cb9f0a9d9a3
# good: [511b0c991c9d49fd6d8188f799b10aa0465cecf3] Merge remote-tracking branch 'drm-intel/for-linux-next'
git bisect good 511b0c991c9d49fd6d8188f799b10aa0465cecf3
# good: [f3b48aa06fb8b4384b90e41220da8be5a4013a6d] Merge remote-tracking branch 'input/next'
git bisect good f3b48aa06fb8b4384b90e41220da8be5a4013a6d
# bad: [87470038c43f9577a300a29ba6c2c95d28039464] Merge remote-tracking branch 'regulator/for-next'
git bisect bad 87470038c43f9577a300a29ba6c2c95d28039464
# bad: [e1796683109e4ba27c73f099486555a36820b175] Merge remote-tracking branch 'device-mapper/for-next'
git bisect bad e1796683109e4ba27c73f099486555a36820b175
# bad: [a11d7fc2d05fb509cd9e33d4093507d6eda3ad53] block: remove the bd_bdi in struct block_device
git bisect bad a11d7fc2d05fb509cd9e33d4093507d6eda3ad53
# good: [a291bb43e5c9fdedc4be3dfd496e64e7c5a78b1f] block: use the %pg format specifier in show_partition
git bisect good a291bb43e5c9fdedc4be3dfd496e64e7c5a78b1f
# good: [2112f5c1330a671fa852051d85cb9eadc05d7eb7] loop: Select I/O scheduler 'none' from inside add_disk()
git bisect good 2112f5c1330a671fa852051d85cb9eadc05d7eb7
# good: [ba30585936b0b88f0fb2b19be279b346a6cc87eb] dm: move setting md->type into dm_setup_md_queue
git bisect good ba30585936b0b88f0fb2b19be279b346a6cc87eb
# bad: [5ed964f8e54eb3191b8b7b45aeb52672a0c995dc] mm: hide laptop_mode_wb_timer entirely behind the BDI API
git bisect bad 5ed964f8e54eb3191b8b7b45aeb52672a0c995dc
# good: [d1254a8749711e0d7441036a74ce592341f89697] block: remove support for delayed queue registrations
git bisect good d1254a8749711e0d7441036a74ce592341f89697
# first bad commit: [5ed964f8e54eb3191b8b7b45aeb52672a0c995dc] mm: hide laptop_mode_wb_timer entirely behind the BDI API

Re: [PATCH 1/5] mm: hide laptop_mode_wb_timer entirely behind the BDI API

[Guenter Roeck]

On Mon, Aug 09, 2021 at 04:17:40PM +0200, Christoph Hellwig wrote:
> Don't leak the detaÑ–ls of the timer into the block layer, instead
> initialize the timer in bdi_alloc and delete it in bdi_unregister.
> Note that this means the timer is initialized (but not armed) for
> non-block queues as well now.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Just in case this hasn't been reported yet.
This patch results in a widespread build failure. Example:

Building x86_64:tinyconfig ... failed
--------------
Error log:
mm/page-writeback.c:2044:6: error: redefinition of 'laptop_sync_completion'
 2044 | void laptop_sync_completion(void)
      |      ^~~~~~~~~~~~~~~~~~~~~~
In file included from include/linux/memcontrol.h:22,
                 from include/linux/swap.h:9,
                 from mm/page-writeback.c:20:
include/linux/writeback.h:345:20: note: previous definition of 'laptop_sync_completion' with type 'void(void)'
  345 | static inline void laptop_sync_completion(void) { }
      |                    ^~~~~~~~~~~~~~~~~~~~~~
make[2]: *** [scripts/Makefile.build:272: mm/page-writeback.o] Error 1

Guenter

---
bisect log:

# bad: [92d00774360dfd4151f15ab9905c643347b9f242] Add linux-next specific files for 20210810
# good: [36a21d51725af2ce0700c6ebcb6b9594aac658a6] Linux 5.14-rc5
git bisect start 'HEAD' 'v5.14-rc5'
# good: [01dda625c9b7cfd3bf5ac05f73da8c512792f94c] Merge remote-tracking branch 'crypto/master'
git bisect good 01dda625c9b7cfd3bf5ac05f73da8c512792f94c
# bad: [75cadd49361c6650764d35bcbb6c9cb9f0a9d9a3] Merge remote-tracking branch 'irqchip/irq/irqchip-next'
git bisect bad 75cadd49361c6650764d35bcbb6c9cb9f0a9d9a3
# good: [511b0c991c9d49fd6d8188f799b10aa0465cecf3] Merge remote-tracking branch 'drm-intel/for-linux-next'
git bisect good 511b0c991c9d49fd6d8188f799b10aa0465cecf3
# good: [f3b48aa06fb8b4384b90e41220da8be5a4013a6d] Merge remote-tracking branch 'input/next'
git bisect good f3b48aa06fb8b4384b90e41220da8be5a4013a6d
# bad: [87470038c43f9577a300a29ba6c2c95d28039464] Merge remote-tracking branch 'regulator/for-next'
git bisect bad 87470038c43f9577a300a29ba6c2c95d28039464
# bad: [e1796683109e4ba27c73f099486555a36820b175] Merge remote-tracking branch 'device-mapper/for-next'
git bisect bad e1796683109e4ba27c73f099486555a36820b175
# bad: [a11d7fc2d05fb509cd9e33d4093507d6eda3ad53] block: remove the bd_bdi in struct block_device
git bisect bad a11d7fc2d05fb509cd9e33d4093507d6eda3ad53
# good: [a291bb43e5c9fdedc4be3dfd496e64e7c5a78b1f] block: use the %pg format specifier in show_partition
git bisect good a291bb43e5c9fdedc4be3dfd496e64e7c5a78b1f
# good: [2112f5c1330a671fa852051d85cb9eadc05d7eb7] loop: Select I/O scheduler 'none' from inside add_disk()
git bisect good 2112f5c1330a671fa852051d85cb9eadc05d7eb7
# good: [ba30585936b0b88f0fb2b19be279b346a6cc87eb] dm: move setting md->type into dm_setup_md_queue
git bisect good ba30585936b0b88f0fb2b19be279b346a6cc87eb
# bad: [5ed964f8e54eb3191b8b7b45aeb52672a0c995dc] mm: hide laptop_mode_wb_timer entirely behind the BDI API
git bisect bad 5ed964f8e54eb3191b8b7b45aeb52672a0c995dc
# good: [d1254a8749711e0d7441036a74ce592341f89697] block: remove support for delayed queue registrations
git bisect good d1254a8749711e0d7441036a74ce592341f89697
# first bad commit: [5ed964f8e54eb3191b8b7b45aeb52672a0c995dc] mm: hide laptop_mode_wb_timer entirely behind the BDI API

Re: [PATCH 1/5] mm: hide laptop_mode_wb_timer entirely behind the BDI API

[Christoph Hellwig]

On Tue, Aug 10, 2021 at 02:56:22PM -0700, Guenter Roeck wrote:
> On Mon, Aug 09, 2021 at 04:17:40PM +0200, Christoph Hellwig wrote:
> > Don't leak the detaіls of the timer into the block layer, instead
> > initialize the timer in bdi_alloc and delete it in bdi_unregister.
> > Note that this means the timer is initialized (but not armed) for
> > non-block queues as well now.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> 
> Just in case this hasn't been reported yet.
> This patch results in a widespread build failure. Example:

Sorry.  This was reported before and a fix is already queued up here:

https://git.kernel.dk/cgit/linux-block/commit/?h=for-5.15/block&id=99d26de2f6d79badc80f55b54bd90d4cb9d1ad90

Re: [PATCH 1/5] mm: hide laptop_mode_wb_timer entirely behind the BDI API

[Christoph Hellwig]

On Tue, Aug 10, 2021 at 02:56:22PM -0700, Guenter Roeck wrote:
> On Mon, Aug 09, 2021 at 04:17:40PM +0200, Christoph Hellwig wrote:
> > Don't leak the detaÑ–ls of the timer into the block layer, instead
> > initialize the timer in bdi_alloc and delete it in bdi_unregister.
> > Note that this means the timer is initialized (but not armed) for
> > non-block queues as well now.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> 
> Just in case this hasn't been reported yet.
> This patch results in a widespread build failure. Example:

Sorry.  This was reported before and a fix is already queued up here:

https://git.kernel.dk/cgit/linux-block/commit/?h=for-5.15/block&id=99d26de2f6d79badc80f55b54bd90d4cb9d1ad90

[PATCH 2/5] block: pass a gendisk to blk_queue_update_readahead

[Christoph Hellwig]

.. and rename the function to disk_update_readahead.  This is in
preparation for moving the BDI from the request_queue to the gendisk.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-settings.c         | 8 +++++---
 block/blk-sysfs.c            | 2 +-
 drivers/block/drbd/drbd_nl.c | 2 +-
 drivers/md/dm-table.c        | 2 +-
 drivers/nvme/host/core.c     | 2 +-
 include/linux/blkdev.h       | 2 +-
 6 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index 109012719aa0..44aaef9bf736 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -380,8 +380,10 @@ void blk_queue_alignment_offset(struct request_queue *q, unsigned int offset)
 }
 EXPORT_SYMBOL(blk_queue_alignment_offset);
 
-void blk_queue_update_readahead(struct request_queue *q)
+void disk_update_readahead(struct gendisk *disk)
 {
+	struct request_queue *q = disk->queue;
+
 	/*
 	 * For read-ahead of large files to be effective, we need to read ahead
 	 * at least twice the optimal I/O size.
@@ -391,7 +393,7 @@ void blk_queue_update_readahead(struct request_queue *q)
 	q->backing_dev_info->io_pages =
 		queue_max_sectors(q) >> (PAGE_SHIFT - 9);
 }
-EXPORT_SYMBOL_GPL(blk_queue_update_readahead);
+EXPORT_SYMBOL_GPL(disk_update_readahead);
 
 /**
  * blk_limits_io_min - set minimum request size for a device
@@ -665,7 +667,7 @@ void disk_stack_limits(struct gendisk *disk, struct block_device *bdev,
 		pr_notice("%s: Warning: Device %pg is misaligned\n",
 			disk->disk_name, bdev);
 
-	blk_queue_update_readahead(disk->queue);
+	disk_update_readahead(disk);
 }
 EXPORT_SYMBOL(disk_stack_limits);
 
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 370d83c18057..3af2ab7d5086 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -866,7 +866,7 @@ int blk_register_queue(struct gendisk *disk)
 		  "%s is registering an already registered queue\n",
 		  kobject_name(&dev->kobj));
 
-	blk_queue_update_readahead(q);
+	disk_update_readahead(disk);
 
 	ret = blk_trace_init_sysfs(dev);
 	if (ret)
diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
index e7d0e637e632..44ccf8b4f4b2 100644
--- a/drivers/block/drbd/drbd_nl.c
+++ b/drivers/block/drbd/drbd_nl.c
@@ -1364,7 +1364,7 @@ static void drbd_setup_queue_param(struct drbd_device *device, struct drbd_backi
 
 	if (b) {
 		blk_stack_limits(&q->limits, &b->limits, 0);
-		blk_queue_update_readahead(q);
+		disk_update_readahead(device->vdisk);
 	}
 	fixup_discard_if_not_supported(q);
 	fixup_write_zeroes(device, q);
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 0543cdf89e92..b03eabc1ed7c 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -2076,7 +2076,7 @@ int dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
 	}
 
 	dm_update_keyslot_manager(q, t);
-	blk_queue_update_readahead(q);
+	disk_update_readahead(t->md->disk);
 
 	return 0;
 }
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index dfd9dec0c1f6..f6c0a59c4b53 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1890,7 +1890,7 @@ static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_id_ns *id)
 		nvme_update_disk_info(ns->head->disk, ns, id);
 		blk_stack_limits(&ns->head->disk->queue->limits,
 				 &ns->queue->limits, 0);
-		blk_queue_update_readahead(ns->head->disk->queue);
+		disk_update_readahead(ns->head->disk);
 		blk_mq_unfreeze_queue(ns->head->disk->queue);
 	}
 	return 0;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index b5c033cf5f26..ac3642c88a4d 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1139,7 +1139,7 @@ void blk_queue_zone_write_granularity(struct request_queue *q,
 				      unsigned int size);
 extern void blk_queue_alignment_offset(struct request_queue *q,
 				       unsigned int alignment);
-void blk_queue_update_readahead(struct request_queue *q);
+void disk_update_readahead(struct gendisk *disk);
 extern void blk_limits_io_min(struct queue_limits *limits, unsigned int min);
 extern void blk_queue_io_min(struct request_queue *q, unsigned int min);
 extern void blk_limits_io_opt(struct queue_limits *limits, unsigned int opt);
-- 
2.30.2

[PATCH 2/5] block: pass a gendisk to blk_queue_update_readahead

[Christoph Hellwig]

.. and rename the function to disk_update_readahead.  This is in
preparation for moving the BDI from the request_queue to the gendisk.

Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
---
 block/blk-settings.c         | 8 +++++---
 block/blk-sysfs.c            | 2 +-
 drivers/block/drbd/drbd_nl.c | 2 +-
 drivers/md/dm-table.c        | 2 +-
 drivers/nvme/host/core.c     | 2 +-
 include/linux/blkdev.h       | 2 +-
 6 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index 109012719aa0..44aaef9bf736 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -380,8 +380,10 @@ void blk_queue_alignment_offset(struct request_queue *q, unsigned int offset)
 }
 EXPORT_SYMBOL(blk_queue_alignment_offset);
 
-void blk_queue_update_readahead(struct request_queue *q)
+void disk_update_readahead(struct gendisk *disk)
 {
+	struct request_queue *q = disk->queue;
+
 	/*
 	 * For read-ahead of large files to be effective, we need to read ahead
 	 * at least twice the optimal I/O size.
@@ -391,7 +393,7 @@ void blk_queue_update_readahead(struct request_queue *q)
 	q->backing_dev_info->io_pages =
 		queue_max_sectors(q) >> (PAGE_SHIFT - 9);
 }
-EXPORT_SYMBOL_GPL(blk_queue_update_readahead);
+EXPORT_SYMBOL_GPL(disk_update_readahead);
 
 /**
  * blk_limits_io_min - set minimum request size for a device
@@ -665,7 +667,7 @@ void disk_stack_limits(struct gendisk *disk, struct block_device *bdev,
 		pr_notice("%s: Warning: Device %pg is misaligned\n",
 			disk->disk_name, bdev);
 
-	blk_queue_update_readahead(disk->queue);
+	disk_update_readahead(disk);
 }
 EXPORT_SYMBOL(disk_stack_limits);
 
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 370d83c18057..3af2ab7d5086 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -866,7 +866,7 @@ int blk_register_queue(struct gendisk *disk)
 		  "%s is registering an already registered queue\n",
 		  kobject_name(&dev->kobj));
 
-	blk_queue_update_readahead(q);
+	disk_update_readahead(disk);
 
 	ret = blk_trace_init_sysfs(dev);
 	if (ret)
diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
index e7d0e637e632..44ccf8b4f4b2 100644
--- a/drivers/block/drbd/drbd_nl.c
+++ b/drivers/block/drbd/drbd_nl.c
@@ -1364,7 +1364,7 @@ static void drbd_setup_queue_param(struct drbd_device *device, struct drbd_backi
 
 	if (b) {
 		blk_stack_limits(&q->limits, &b->limits, 0);
-		blk_queue_update_readahead(q);
+		disk_update_readahead(device->vdisk);
 	}
 	fixup_discard_if_not_supported(q);
 	fixup_write_zeroes(device, q);
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 0543cdf89e92..b03eabc1ed7c 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -2076,7 +2076,7 @@ int dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
 	}
 
 	dm_update_keyslot_manager(q, t);
-	blk_queue_update_readahead(q);
+	disk_update_readahead(t->md->disk);
 
 	return 0;
 }
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index dfd9dec0c1f6..f6c0a59c4b53 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1890,7 +1890,7 @@ static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_id_ns *id)
 		nvme_update_disk_info(ns->head->disk, ns, id);
 		blk_stack_limits(&ns->head->disk->queue->limits,
 				 &ns->queue->limits, 0);
-		blk_queue_update_readahead(ns->head->disk->queue);
+		disk_update_readahead(ns->head->disk);
 		blk_mq_unfreeze_queue(ns->head->disk->queue);
 	}
 	return 0;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index b5c033cf5f26..ac3642c88a4d 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1139,7 +1139,7 @@ void blk_queue_zone_write_granularity(struct request_queue *q,
 				      unsigned int size);
 extern void blk_queue_alignment_offset(struct request_queue *q,
 				       unsigned int alignment);
-void blk_queue_update_readahead(struct request_queue *q);
+void disk_update_readahead(struct gendisk *disk);
 extern void blk_limits_io_min(struct queue_limits *limits, unsigned int min);
 extern void blk_queue_io_min(struct request_queue *q, unsigned int min);
 extern void blk_limits_io_opt(struct queue_limits *limits, unsigned int opt);
-- 
2.30.2

Re: [PATCH 2/5] block: pass a gendisk to blk_queue_update_readahead

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 2/5] block: pass a gendisk to blk_queue_update_readahead

[Jan Kara]

On Mon 09-08-21 16:17:41, Christoph Hellwig wrote:
> .. and rename the function to disk_update_readahead.  This is in
> preparation for moving the BDI from the request_queue to the gendisk.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good. Feel free to add:

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

								Honza

> ---
>  block/blk-settings.c         | 8 +++++---
>  block/blk-sysfs.c            | 2 +-
>  drivers/block/drbd/drbd_nl.c | 2 +-
>  drivers/md/dm-table.c        | 2 +-
>  drivers/nvme/host/core.c     | 2 +-
>  include/linux/blkdev.h       | 2 +-
>  6 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index 109012719aa0..44aaef9bf736 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -380,8 +380,10 @@ void blk_queue_alignment_offset(struct request_queue *q, unsigned int offset)
>  }
>  EXPORT_SYMBOL(blk_queue_alignment_offset);
>  
> -void blk_queue_update_readahead(struct request_queue *q)
> +void disk_update_readahead(struct gendisk *disk)
>  {
> +	struct request_queue *q = disk->queue;
> +
>  	/*
>  	 * For read-ahead of large files to be effective, we need to read ahead
>  	 * at least twice the optimal I/O size.
> @@ -391,7 +393,7 @@ void blk_queue_update_readahead(struct request_queue *q)
>  	q->backing_dev_info->io_pages =
>  		queue_max_sectors(q) >> (PAGE_SHIFT - 9);
>  }
> -EXPORT_SYMBOL_GPL(blk_queue_update_readahead);
> +EXPORT_SYMBOL_GPL(disk_update_readahead);
>  
>  /**
>   * blk_limits_io_min - set minimum request size for a device
> @@ -665,7 +667,7 @@ void disk_stack_limits(struct gendisk *disk, struct block_device *bdev,
>  		pr_notice("%s: Warning: Device %pg is misaligned\n",
>  			disk->disk_name, bdev);
>  
> -	blk_queue_update_readahead(disk->queue);
> +	disk_update_readahead(disk);
>  }
>  EXPORT_SYMBOL(disk_stack_limits);
>  
> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
> index 370d83c18057..3af2ab7d5086 100644
> --- a/block/blk-sysfs.c
> +++ b/block/blk-sysfs.c
> @@ -866,7 +866,7 @@ int blk_register_queue(struct gendisk *disk)
>  		  "%s is registering an already registered queue\n",
>  		  kobject_name(&dev->kobj));
>  
> -	blk_queue_update_readahead(q);
> +	disk_update_readahead(disk);
>  
>  	ret = blk_trace_init_sysfs(dev);
>  	if (ret)
> diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c
> index e7d0e637e632..44ccf8b4f4b2 100644
> --- a/drivers/block/drbd/drbd_nl.c
> +++ b/drivers/block/drbd/drbd_nl.c
> @@ -1364,7 +1364,7 @@ static void drbd_setup_queue_param(struct drbd_device *device, struct drbd_backi
>  
>  	if (b) {
>  		blk_stack_limits(&q->limits, &b->limits, 0);
> -		blk_queue_update_readahead(q);
> +		disk_update_readahead(device->vdisk);
>  	}
>  	fixup_discard_if_not_supported(q);
>  	fixup_write_zeroes(device, q);
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index 0543cdf89e92..b03eabc1ed7c 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -2076,7 +2076,7 @@ int dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
>  	}
>  
>  	dm_update_keyslot_manager(q, t);
> -	blk_queue_update_readahead(q);
> +	disk_update_readahead(t->md->disk);
>  
>  	return 0;
>  }
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index dfd9dec0c1f6..f6c0a59c4b53 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1890,7 +1890,7 @@ static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_id_ns *id)
>  		nvme_update_disk_info(ns->head->disk, ns, id);
>  		blk_stack_limits(&ns->head->disk->queue->limits,
>  				 &ns->queue->limits, 0);
> -		blk_queue_update_readahead(ns->head->disk->queue);
> +		disk_update_readahead(ns->head->disk);
>  		blk_mq_unfreeze_queue(ns->head->disk->queue);
>  	}
>  	return 0;
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index b5c033cf5f26..ac3642c88a4d 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1139,7 +1139,7 @@ void blk_queue_zone_write_granularity(struct request_queue *q,
>  				      unsigned int size);
>  extern void blk_queue_alignment_offset(struct request_queue *q,
>  				       unsigned int alignment);
> -void blk_queue_update_readahead(struct request_queue *q);
> +void disk_update_readahead(struct gendisk *disk);
>  extern void blk_limits_io_min(struct queue_limits *limits, unsigned int min);
>  extern void blk_queue_io_min(struct request_queue *q, unsigned int min);
>  extern void blk_limits_io_opt(struct queue_limits *limits, unsigned int opt);
> -- 
> 2.30.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 3/5] block: add a queue_has_disk helper

[Christoph Hellwig]

Add a helper to check if a gendisk is associated with a request_queue.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/blkdev.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index ac3642c88a4d..96f3d9617cd8 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -664,6 +664,7 @@ extern void blk_clear_pm_only(struct request_queue *q);
 	dma_map_page_attrs(dev, (bv)->bv_page, (bv)->bv_offset, (bv)->bv_len, \
 	(dir), (attrs))
 
+#define queue_has_disk(q)	((q)->kobj.parent != NULL)
 #define queue_to_disk(q)	(dev_to_disk(kobj_to_dev((q)->kobj.parent)))
 
 static inline bool queue_is_mq(struct request_queue *q)
-- 
2.30.2

[PATCH 3/5] block: add a queue_has_disk helper

[Christoph Hellwig]

Add a helper to check if a gendisk is associated with a request_queue.

Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
---
 include/linux/blkdev.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index ac3642c88a4d..96f3d9617cd8 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -664,6 +664,7 @@ extern void blk_clear_pm_only(struct request_queue *q);
 	dma_map_page_attrs(dev, (bv)->bv_page, (bv)->bv_offset, (bv)->bv_len, \
 	(dir), (attrs))
 
+#define queue_has_disk(q)	((q)->kobj.parent != NULL)
 #define queue_to_disk(q)	(dev_to_disk(kobj_to_dev((q)->kobj.parent)))
 
 static inline bool queue_is_mq(struct request_queue *q)
-- 
2.30.2

Re: [PATCH 3/5] block: add a queue_has_disk helper

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 3/5] block: add a queue_has_disk helper

[Jan Kara]

On Mon 09-08-21 16:17:42, Christoph Hellwig wrote:
> Add a helper to check if a gendisk is associated with a request_queue.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks fine. Feel free to add:

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

								Honza

> ---
>  include/linux/blkdev.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index ac3642c88a4d..96f3d9617cd8 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -664,6 +664,7 @@ extern void blk_clear_pm_only(struct request_queue *q);
>  	dma_map_page_attrs(dev, (bv)->bv_page, (bv)->bv_offset, (bv)->bv_len, \
>  	(dir), (attrs))
>  
> +#define queue_has_disk(q)	((q)->kobj.parent != NULL)
>  #define queue_to_disk(q)	(dev_to_disk(kobj_to_dev((q)->kobj.parent)))
>  
>  static inline bool queue_is_mq(struct request_queue *q)
> -- 
> 2.30.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 4/5] block: move the bdi from the request_queue to the gendisk

[Christoph Hellwig]

The backing device information only makes sense for file system I/O,
and thus belongs into the gendisk and not the lower level request_queue
structure.  Move it there.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/bfq-iosched.c           |  4 ++--
 block/blk-cgroup.c            |  7 +++----
 block/blk-core.c              | 13 +++----------
 block/blk-mq.c                |  2 +-
 block/blk-settings.c          | 14 +++++++++-----
 block/blk-sysfs.c             | 26 ++++++++++++--------------
 block/blk-wbt.c               | 10 +++++-----
 block/genhd.c                 | 23 ++++++++++++++---------
 drivers/block/drbd/drbd_req.c |  5 ++---
 drivers/block/pktcdvd.c       |  8 +++-----
 fs/block_dev.c                |  4 ++--
 fs/fat/fatent.c               |  1 +
 include/linux/blkdev.h        |  3 ---
 include/linux/genhd.h         |  1 +
 14 files changed, 58 insertions(+), 63 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 727955918563..1576e858d3a5 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -5266,8 +5266,8 @@ bfq_set_next_ioprio_data(struct bfq_queue *bfqq, struct bfq_io_cq *bic)
 	switch (ioprio_class) {
 	default:
 		pr_err("bdi %s: bfq: bad prio class %d\n",
-				bdi_dev_name(bfqq->bfqd->queue->backing_dev_info),
-				ioprio_class);
+			bdi_dev_name(queue_to_disk(bfqq->bfqd->queue)->bdi),
+			ioprio_class);
 		fallthrough;
 	case IOPRIO_CLASS_NONE:
 		/*
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 575d7a2e7203..db034e35ae20 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -489,10 +489,9 @@ static int blkcg_reset_stats(struct cgroup_subsys_state *css,
 
 const char *blkg_dev_name(struct blkcg_gq *blkg)
 {
-	/* some drivers (floppy) instantiate a queue w/o disk registered */
-	if (blkg->q->backing_dev_info->dev)
-		return bdi_dev_name(blkg->q->backing_dev_info);
-	return NULL;
+	if (!queue_has_disk(blkg->q) || !queue_to_disk(blkg->q)->bdi->dev)
+		return NULL;
+	return bdi_dev_name(queue_to_disk(blkg->q)->bdi);
 }
 
 /**
diff --git a/block/blk-core.c b/block/blk-core.c
index 5897bc37467d..0874bc2fcdb4 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -14,7 +14,6 @@
  */
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/backing-dev.h>
 #include <linux/bio.h>
 #include <linux/blkdev.h>
 #include <linux/blk-mq.h>
@@ -531,13 +530,9 @@ struct request_queue *blk_alloc_queue(int node_id)
 	if (ret)
 		goto fail_id;
 
-	q->backing_dev_info = bdi_alloc(node_id);
-	if (!q->backing_dev_info)
-		goto fail_split;
-
 	q->stats = blk_alloc_queue_stats();
 	if (!q->stats)
-		goto fail_stats;
+		goto fail_split;
 
 	q->node = node_id;
 
@@ -567,7 +562,7 @@ struct request_queue *blk_alloc_queue(int node_id)
 	if (percpu_ref_init(&q->q_usage_counter,
 				blk_queue_usage_counter_release,
 				PERCPU_REF_INIT_ATOMIC, GFP_KERNEL))
-		goto fail_bdi;
+		goto fail_stats;
 
 	if (blkcg_init_queue(q))
 		goto fail_ref;
@@ -580,10 +575,8 @@ struct request_queue *blk_alloc_queue(int node_id)
 
 fail_ref:
 	percpu_ref_exit(&q->q_usage_counter);
-fail_bdi:
-	blk_free_queue_stats(q->stats);
 fail_stats:
-	bdi_put(q->backing_dev_info);
+	blk_free_queue_stats(q->stats);
 fail_split:
 	bioset_exit(&q->bio_split);
 fail_id:
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 2c4ac51e54eb..d2725f94491d 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -525,7 +525,7 @@ void blk_mq_free_request(struct request *rq)
 		__blk_mq_dec_active_requests(hctx);
 
 	if (unlikely(laptop_mode && !blk_rq_is_passthrough(rq)))
-		laptop_io_completion(q->backing_dev_info);
+		laptop_io_completion(queue_to_disk(q)->bdi);
 
 	rq_qos_done(q, rq);
 
diff --git a/block/blk-settings.c b/block/blk-settings.c
index 44aaef9bf736..3613d2cc0688 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -8,6 +8,7 @@
 #include <linux/bio.h>
 #include <linux/blkdev.h>
 #include <linux/pagemap.h>
+#include <linux/backing-dev-defs.h>
 #include <linux/gcd.h>
 #include <linux/lcm.h>
 #include <linux/jiffies.h>
@@ -140,7 +141,9 @@ void blk_queue_max_hw_sectors(struct request_queue *q, unsigned int max_hw_secto
 				 limits->logical_block_size >> SECTOR_SHIFT);
 	limits->max_sectors = max_sectors;
 
-	q->backing_dev_info->io_pages = max_sectors >> (PAGE_SHIFT - 9);
+	if (!queue_has_disk(q))
+		return;
+	queue_to_disk(q)->bdi->io_pages = max_sectors >> (PAGE_SHIFT - 9);
 }
 EXPORT_SYMBOL(blk_queue_max_hw_sectors);
 
@@ -388,10 +391,9 @@ void disk_update_readahead(struct gendisk *disk)
 	 * For read-ahead of large files to be effective, we need to read ahead
 	 * at least twice the optimal I/O size.
 	 */
-	q->backing_dev_info->ra_pages =
+	disk->bdi->ra_pages =
 		max(queue_io_opt(q) * 2 / PAGE_SIZE, VM_READAHEAD_PAGES);
-	q->backing_dev_info->io_pages =
-		queue_max_sectors(q) >> (PAGE_SHIFT - 9);
+	disk->bdi->io_pages = queue_max_sectors(q) >> (PAGE_SHIFT - 9);
 }
 EXPORT_SYMBOL_GPL(disk_update_readahead);
 
@@ -473,7 +475,9 @@ EXPORT_SYMBOL(blk_limits_io_opt);
 void blk_queue_io_opt(struct request_queue *q, unsigned int opt)
 {
 	blk_limits_io_opt(&q->limits, opt);
-	q->backing_dev_info->ra_pages =
+	if (!queue_has_disk(q))
+		return;
+	queue_to_disk(q)->bdi->ra_pages =
 		max(queue_io_opt(q) * 2 / PAGE_SIZE, VM_READAHEAD_PAGES);
 }
 EXPORT_SYMBOL(blk_queue_io_opt);
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 3af2ab7d5086..1832587dce3a 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -88,9 +88,11 @@ queue_requests_store(struct request_queue *q, const char *page, size_t count)
 
 static ssize_t queue_ra_show(struct request_queue *q, char *page)
 {
-	unsigned long ra_kb = q->backing_dev_info->ra_pages <<
-					(PAGE_SHIFT - 10);
+	unsigned long ra_kb;
 
+	if (!queue_has_disk(q))
+		return -EINVAL;
+	ra_kb = queue_to_disk(q)->bdi->ra_pages << (PAGE_SHIFT - 10);
 	return queue_var_show(ra_kb, page);
 }
 
@@ -98,13 +100,14 @@ static ssize_t
 queue_ra_store(struct request_queue *q, const char *page, size_t count)
 {
 	unsigned long ra_kb;
-	ssize_t ret = queue_var_store(&ra_kb, page, count);
+	ssize_t ret;
 
+	if (!queue_has_disk(q))
+		return -EINVAL;
+	ret = queue_var_store(&ra_kb, page, count);
 	if (ret < 0)
 		return ret;
-
-	q->backing_dev_info->ra_pages = ra_kb >> (PAGE_SHIFT - 10);
-
+	queue_to_disk(q)->bdi->ra_pages = ra_kb >> (PAGE_SHIFT - 10);
 	return ret;
 }
 
@@ -251,7 +254,9 @@ queue_max_sectors_store(struct request_queue *q, const char *page, size_t count)
 
 	spin_lock_irq(&q->queue_lock);
 	q->limits.max_sectors = max_sectors_kb << 1;
-	q->backing_dev_info->io_pages = max_sectors_kb >> (PAGE_SHIFT - 10);
+	if (queue_has_disk(q))
+		queue_to_disk(q)->bdi->io_pages =
+			max_sectors_kb >> (PAGE_SHIFT - 10);
 	spin_unlock_irq(&q->queue_lock);
 
 	return ret;
@@ -766,13 +771,6 @@ static void blk_exit_queue(struct request_queue *q)
 	 * e.g. blkcg_print_blkgs() to crash.
 	 */
 	blkcg_exit_queue(q);
-
-	/*
-	 * Since the cgroup code may dereference the @q->backing_dev_info
-	 * pointer, only decrease its reference count after having removed the
-	 * association with the block cgroup controller.
-	 */
-	bdi_put(q->backing_dev_info);
 }
 
 /**
diff --git a/block/blk-wbt.c b/block/blk-wbt.c
index 3ed71b8da887..31086afaad9c 100644
--- a/block/blk-wbt.c
+++ b/block/blk-wbt.c
@@ -97,7 +97,7 @@ static void wb_timestamp(struct rq_wb *rwb, unsigned long *var)
  */
 static bool wb_recent_wait(struct rq_wb *rwb)
 {
-	struct bdi_writeback *wb = &rwb->rqos.q->backing_dev_info->wb;
+	struct bdi_writeback *wb = &queue_to_disk(rwb->rqos.q)->bdi->wb;
 
 	return time_before(jiffies, wb->dirty_sleep + HZ);
 }
@@ -234,7 +234,7 @@ enum {
 
 static int latency_exceeded(struct rq_wb *rwb, struct blk_rq_stat *stat)
 {
-	struct backing_dev_info *bdi = rwb->rqos.q->backing_dev_info;
+	struct backing_dev_info *bdi = queue_to_disk(rwb->rqos.q)->bdi;
 	struct rq_depth *rqd = &rwb->rq_depth;
 	u64 thislat;
 
@@ -287,7 +287,7 @@ static int latency_exceeded(struct rq_wb *rwb, struct blk_rq_stat *stat)
 
 static void rwb_trace_step(struct rq_wb *rwb, const char *msg)
 {
-	struct backing_dev_info *bdi = rwb->rqos.q->backing_dev_info;
+	struct backing_dev_info *bdi = queue_to_disk(rwb->rqos.q)->bdi;
 	struct rq_depth *rqd = &rwb->rq_depth;
 
 	trace_wbt_step(bdi, msg, rqd->scale_step, rwb->cur_win_nsec,
@@ -359,8 +359,8 @@ static void wb_timer_fn(struct blk_stat_callback *cb)
 
 	status = latency_exceeded(rwb, cb->stat);
 
-	trace_wbt_timer(rwb->rqos.q->backing_dev_info, status, rqd->scale_step,
-			inflight);
+	trace_wbt_timer(queue_to_disk(rwb->rqos.q)->bdi, status,
+			rqd->scale_step, inflight);
 
 	/*
 	 * If we exceeded the latency target, step down. If we did not,
diff --git a/block/genhd.c b/block/genhd.c
index a4817e42f3a3..4b212a620941 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -456,10 +456,9 @@ static void register_disk(struct device *parent, struct gendisk *disk,
 	dev_set_uevent_suppress(ddev, 0);
 	disk_uevent(disk, KOBJ_ADD);
 
-	if (disk->queue->backing_dev_info->dev) {
-		err = sysfs_create_link(&ddev->kobj,
-			  &disk->queue->backing_dev_info->dev->kobj,
-			  "bdi");
+	if (disk->bdi->dev) {
+		err = sysfs_create_link(&ddev->kobj, &disk->bdi->dev->kobj,
+					"bdi");
 		WARN_ON(err);
 	}
 }
@@ -531,15 +530,14 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,
 		disk->flags |= GENHD_FL_SUPPRESS_PARTITION_INFO;
 		disk->flags |= GENHD_FL_NO_PART_SCAN;
 	} else {
-		struct backing_dev_info *bdi = disk->queue->backing_dev_info;
 		struct device *dev = disk_to_dev(disk);
 
 		/* Register BDI before referencing it from bdev */
 		dev->devt = MKDEV(disk->major, disk->first_minor);
-		ret = bdi_register(bdi, "%u:%u",
+		ret = bdi_register(disk->bdi, "%u:%u",
 				   disk->major, disk->first_minor);
 		WARN_ON(ret);
-		bdi_set_owner(bdi, dev);
+		bdi_set_owner(disk->bdi, dev);
 		bdev_add(disk->part0, dev->devt);
 	}
 	register_disk(parent, disk, groups);
@@ -620,7 +618,7 @@ void del_gendisk(struct gendisk *disk)
 		 * Unregister bdi before releasing device numbers (as they can
 		 * get reused and we'd get clashes in sysfs).
 		 */
-		bdi_unregister(disk->queue->backing_dev_info);
+		bdi_unregister(disk->bdi);
 	}
 
 	blk_unregister_queue(disk);
@@ -1093,6 +1091,7 @@ static void disk_release(struct device *dev)
 
 	might_sleep();
 
+	bdi_put(disk->bdi);
 	if (MAJOR(dev->devt) == BLOCK_EXT_MAJOR)
 		blk_free_ext_minor(MINOR(dev->devt));
 	disk_release_events(disk);
@@ -1273,9 +1272,13 @@ struct gendisk *__alloc_disk_node(int minors, int node_id)
 	if (!disk)
 		return NULL;
 
+	disk->bdi = bdi_alloc(node_id);
+	if (!disk->bdi)
+		goto out_free_disk;
+
 	disk->part0 = bdev_alloc(disk, 0);
 	if (!disk->part0)
-		goto out_free_disk;
+		goto out_free_bdi;
 
 	disk->node_id = node_id;
 	mutex_init(&disk->open_mutex);
@@ -1295,6 +1298,8 @@ struct gendisk *__alloc_disk_node(int minors, int node_id)
 out_destroy_part_tbl:
 	xa_destroy(&disk->part_tbl);
 	iput(disk->part0->bd_inode);
+out_free_bdi:
+	bdi_put(disk->bdi);
 out_free_disk:
 	kfree(disk);
 	return NULL;
diff --git a/drivers/block/drbd/drbd_req.c b/drivers/block/drbd/drbd_req.c
index 13beb98a7c5a..5ca233644d70 100644
--- a/drivers/block/drbd/drbd_req.c
+++ b/drivers/block/drbd/drbd_req.c
@@ -905,13 +905,12 @@ static bool drbd_may_do_local_read(struct drbd_device *device, sector_t sector,
 static bool remote_due_to_read_balancing(struct drbd_device *device, sector_t sector,
 		enum drbd_read_balancing rbm)
 {
-	struct backing_dev_info *bdi;
 	int stripe_shift;
 
 	switch (rbm) {
 	case RB_CONGESTED_REMOTE:
-		bdi = device->ldev->backing_bdev->bd_disk->queue->backing_dev_info;
-		return bdi_read_congested(bdi);
+		return bdi_read_congested(
+			device->ldev->backing_bdev->bd_disk->bdi);
 	case RB_LEAST_PENDING:
 		return atomic_read(&device->local_cnt) >
 			atomic_read(&device->ap_pending_cnt) + atomic_read(&device->rs_pending_cnt);
diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 538446b652de..0f26b2510a75 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -1183,10 +1183,8 @@ static int pkt_handle_queue(struct pktcdvd_device *pd)
 	wakeup = (pd->write_congestion_on > 0
 	 		&& pd->bio_queue_size <= pd->write_congestion_off);
 	spin_unlock(&pd->lock);
-	if (wakeup) {
-		clear_bdi_congested(pd->disk->queue->backing_dev_info,
-					BLK_RW_ASYNC);
-	}
+	if (wakeup)
+		clear_bdi_congested(pd->disk->bdi, BLK_RW_ASYNC);
 
 	pkt->sleep_time = max(PACKET_WAIT_TIME, 1);
 	pkt_set_state(pkt, PACKET_WAITING_STATE);
@@ -2366,7 +2364,7 @@ static void pkt_make_request_write(struct request_queue *q, struct bio *bio)
 	spin_lock(&pd->lock);
 	if (pd->write_congestion_on > 0
 	    && pd->bio_queue_size >= pd->write_congestion_on) {
-		set_bdi_congested(q->backing_dev_info, BLK_RW_ASYNC);
+		set_bdi_congested(bio->bi_bdev->bd_disk->bdi, BLK_RW_ASYNC);
 		do {
 			spin_unlock(&pd->lock);
 			congestion_wait(BLK_RW_ASYNC, HZ);
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 6658f40ae492..de8c3d9cbdb1 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1232,7 +1232,7 @@ static int blkdev_get_whole(struct block_device *bdev, fmode_t mode)
 	if (!bdev->bd_openers) {
 		set_init_blocksize(bdev);
 		if (bdev->bd_bdi == &noop_backing_dev_info)
-			bdev->bd_bdi = bdi_get(disk->queue->backing_dev_info);
+			bdev->bd_bdi = bdi_get(disk->bdi);
 	}
 	if (test_bit(GD_NEED_PART_SCAN, &disk->state))
 		bdev_disk_changed(disk, false);
@@ -1267,7 +1267,7 @@ static int blkdev_get_part(struct block_device *part, fmode_t mode)
 	disk->open_partitions++;
 	set_init_blocksize(part);
 	if (part->bd_bdi == &noop_backing_dev_info)
-		part->bd_bdi = bdi_get(disk->queue->backing_dev_info);
+		part->bd_bdi = bdi_get(disk->bdi);
 done:
 	part->bd_openers++;
 	return 0;
diff --git a/fs/fat/fatent.c b/fs/fat/fatent.c
index 860e884e56e8..978ac6751aeb 100644
--- a/fs/fat/fatent.c
+++ b/fs/fat/fatent.c
@@ -5,6 +5,7 @@
 
 #include <linux/blkdev.h>
 #include <linux/sched/signal.h>
+#include <linux/backing-dev-defs.h>
 #include "fat.h"
 
 struct fatent_operations {
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 96f3d9617cd8..23e1253a8d88 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -11,7 +11,6 @@
 #include <linux/minmax.h>
 #include <linux/timer.h>
 #include <linux/workqueue.h>
-#include <linux/backing-dev-defs.h>
 #include <linux/wait.h>
 #include <linux/mempool.h>
 #include <linux/pfn.h>
@@ -398,8 +397,6 @@ struct request_queue {
 	struct blk_mq_hw_ctx	**queue_hw_ctx;
 	unsigned int		nr_hw_queues;
 
-	struct backing_dev_info	*backing_dev_info;
-
 	/*
 	 * The queue owner gets to use this for whatever they like.
 	 * ll_rw_blk doesn't touch it.
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 849486de81c6..ef94b770d696 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -158,6 +158,7 @@ struct gendisk {
 	struct mutex open_mutex;	/* open/close mutex */
 	unsigned open_partitions;	/* number of open partitions */
 
+	struct backing_dev_info	*bdi;
 	struct kobject *slave_dir;
 
 	struct timer_rand_state *random;
-- 
2.30.2

Re: [PATCH 4/5] block: move the bdi from the request_queue to the gendisk

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 4/5] block: move the bdi from the request_queue to the gendisk

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn-Sjgp3cTcYWE@public.gmane.org>

Re: [PATCH 4/5] block: move the bdi from the request_queue to the gendisk

[Jan Kara]

On Mon 09-08-21 16:17:43, Christoph Hellwig wrote:
> The backing device information only makes sense for file system I/O,
> and thus belongs into the gendisk and not the lower level request_queue
> structure.  Move it there.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks mostly good. I'm just unsure whether some queue_to_disk() calls are
safe.

> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 2c4ac51e54eb..d2725f94491d 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -525,7 +525,7 @@ void blk_mq_free_request(struct request *rq)
>  		__blk_mq_dec_active_requests(hctx);
>  
>  	if (unlikely(laptop_mode && !blk_rq_is_passthrough(rq)))
> -		laptop_io_completion(q->backing_dev_info);
> +		laptop_io_completion(queue_to_disk(q)->bdi);
> 

E.g. cannot this get called for a queue that is without a disk?

> diff --git a/block/blk-wbt.c b/block/blk-wbt.c
> index 3ed71b8da887..31086afaad9c 100644
> --- a/block/blk-wbt.c
> +++ b/block/blk-wbt.c
> @@ -97,7 +97,7 @@ static void wb_timestamp(struct rq_wb *rwb, unsigned long *var)
>   */
>  static bool wb_recent_wait(struct rq_wb *rwb)
>  {
> -	struct bdi_writeback *wb = &rwb->rqos.q->backing_dev_info->wb;
> +	struct bdi_writeback *wb = &queue_to_disk(rwb->rqos.q)->bdi->wb;
>  
>  	return time_before(jiffies, wb->dirty_sleep + HZ);
>  }
> @@ -234,7 +234,7 @@ enum {
>  
>  static int latency_exceeded(struct rq_wb *rwb, struct blk_rq_stat *stat)
>  {
> -	struct backing_dev_info *bdi = rwb->rqos.q->backing_dev_info;
> +	struct backing_dev_info *bdi = queue_to_disk(rwb->rqos.q)->bdi;
>  	struct rq_depth *rqd = &rwb->rq_depth;
>  	u64 thislat;
>  
> @@ -287,7 +287,7 @@ static int latency_exceeded(struct rq_wb *rwb, struct blk_rq_stat *stat)
>  
>  static void rwb_trace_step(struct rq_wb *rwb, const char *msg)
>  {
> -	struct backing_dev_info *bdi = rwb->rqos.q->backing_dev_info;
> +	struct backing_dev_info *bdi = queue_to_disk(rwb->rqos.q)->bdi;
>  	struct rq_depth *rqd = &rwb->rq_depth;
>  
>  	trace_wbt_step(bdi, msg, rqd->scale_step, rwb->cur_win_nsec,
> @@ -359,8 +359,8 @@ static void wb_timer_fn(struct blk_stat_callback *cb)
>  
>  	status = latency_exceeded(rwb, cb->stat);
>  
> -	trace_wbt_timer(rwb->rqos.q->backing_dev_info, status, rqd->scale_step,
> -			inflight);
> +	trace_wbt_timer(queue_to_disk(rwb->rqos.q)->bdi, status,
> +			rqd->scale_step, inflight);
>  
>  	/*
>  	 * If we exceeded the latency target, step down. If we did not,

Or all these calls - is wbt guaranteed to only be setup for a queue with
disk?

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 4/5] block: move the bdi from the request_queue to the gendisk

[Jens Axboe]

On 8/9/21 9:47 AM, Jan Kara wrote:
> On Mon 09-08-21 16:17:43, Christoph Hellwig wrote:
>> The backing device information only makes sense for file system I/O,
>> and thus belongs into the gendisk and not the lower level request_queue
>> structure.  Move it there.
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Looks mostly good. I'm just unsure whether some queue_to_disk() calls are
> safe.
> 
>> diff --git a/block/blk-mq.c b/block/blk-mq.c
>> index 2c4ac51e54eb..d2725f94491d 100644
>> --- a/block/blk-mq.c
>> +++ b/block/blk-mq.c
>> @@ -525,7 +525,7 @@ void blk_mq_free_request(struct request *rq)
>>  		__blk_mq_dec_active_requests(hctx);
>>  
>>  	if (unlikely(laptop_mode && !blk_rq_is_passthrough(rq)))
>> -		laptop_io_completion(q->backing_dev_info);
>> +		laptop_io_completion(queue_to_disk(q)->bdi);
>>
> 
> E.g. cannot this get called for a queue that is without a disk?

Should be fine, as it's checking for passthrough. Maybe famous last
words, but we should not be seeing regular IO before disk is setup.

>> @@ -359,8 +359,8 @@ static void wb_timer_fn(struct blk_stat_callback *cb)
>>  
>>  	status = latency_exceeded(rwb, cb->stat);
>>  
>> -	trace_wbt_timer(rwb->rqos.q->backing_dev_info, status, rqd->scale_step,
>> -			inflight);
>> +	trace_wbt_timer(queue_to_disk(rwb->rqos.q)->bdi, status,
>> +			rqd->scale_step, inflight);
>>  
>>  	/*
>>  	 * If we exceeded the latency target, step down. If we did not,
> 
> Or all these calls - is wbt guaranteed to only be setup for a queue with
> disk?

Same for this one.

-- 
Jens Axboe

Re: [PATCH 4/5] block: move the bdi from the request_queue to the gendisk

[Jens Axboe]

On 8/9/21 9:47 AM, Jan Kara wrote:
> On Mon 09-08-21 16:17:43, Christoph Hellwig wrote:
>> The backing device information only makes sense for file system I/O,
>> and thus belongs into the gendisk and not the lower level request_queue
>> structure.  Move it there.
>>
>> Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
> 
> Looks mostly good. I'm just unsure whether some queue_to_disk() calls are
> safe.
> 
>> diff --git a/block/blk-mq.c b/block/blk-mq.c
>> index 2c4ac51e54eb..d2725f94491d 100644
>> --- a/block/blk-mq.c
>> +++ b/block/blk-mq.c
>> @@ -525,7 +525,7 @@ void blk_mq_free_request(struct request *rq)
>>  		__blk_mq_dec_active_requests(hctx);
>>  
>>  	if (unlikely(laptop_mode && !blk_rq_is_passthrough(rq)))
>> -		laptop_io_completion(q->backing_dev_info);
>> +		laptop_io_completion(queue_to_disk(q)->bdi);
>>
> 
> E.g. cannot this get called for a queue that is without a disk?

Should be fine, as it's checking for passthrough. Maybe famous last
words, but we should not be seeing regular IO before disk is setup.

>> @@ -359,8 +359,8 @@ static void wb_timer_fn(struct blk_stat_callback *cb)
>>  
>>  	status = latency_exceeded(rwb, cb->stat);
>>  
>> -	trace_wbt_timer(rwb->rqos.q->backing_dev_info, status, rqd->scale_step,
>> -			inflight);
>> +	trace_wbt_timer(queue_to_disk(rwb->rqos.q)->bdi, status,
>> +			rqd->scale_step, inflight);
>>  
>>  	/*
>>  	 * If we exceeded the latency target, step down. If we did not,
> 
> Or all these calls - is wbt guaranteed to only be setup for a queue with
> disk?

Same for this one.

-- 
Jens Axboe

Re: [PATCH 4/5] block: move the bdi from the request_queue to the gendisk

[Jan Kara]

On Mon 09-08-21 11:57:42, Jens Axboe wrote:
> On 8/9/21 9:47 AM, Jan Kara wrote:
> > On Mon 09-08-21 16:17:43, Christoph Hellwig wrote:
> >> The backing device information only makes sense for file system I/O,
> >> and thus belongs into the gendisk and not the lower level request_queue
> >> structure.  Move it there.
> >>
> >> Signed-off-by: Christoph Hellwig <hch@lst.de>
> > 
> > Looks mostly good. I'm just unsure whether some queue_to_disk() calls are
> > safe.
> > 
> >> diff --git a/block/blk-mq.c b/block/blk-mq.c
> >> index 2c4ac51e54eb..d2725f94491d 100644
> >> --- a/block/blk-mq.c
> >> +++ b/block/blk-mq.c
> >> @@ -525,7 +525,7 @@ void blk_mq_free_request(struct request *rq)
> >>  		__blk_mq_dec_active_requests(hctx);
> >>  
> >>  	if (unlikely(laptop_mode && !blk_rq_is_passthrough(rq)))
> >> -		laptop_io_completion(q->backing_dev_info);
> >> +		laptop_io_completion(queue_to_disk(q)->bdi);
> >>
> > 
> > E.g. cannot this get called for a queue that is without a disk?
> 
> Should be fine, as it's checking for passthrough. Maybe famous last
> words, but we should not be seeing regular IO before disk is setup.
> 
> >> @@ -359,8 +359,8 @@ static void wb_timer_fn(struct blk_stat_callback *cb)
> >>  
> >>  	status = latency_exceeded(rwb, cb->stat);
> >>  
> >> -	trace_wbt_timer(rwb->rqos.q->backing_dev_info, status, rqd->scale_step,
> >> -			inflight);
> >> +	trace_wbt_timer(queue_to_disk(rwb->rqos.q)->bdi, status,
> >> +			rqd->scale_step, inflight);
> >>  
> >>  	/*
> >>  	 * If we exceeded the latency target, step down. If we did not,
> > 
> > Or all these calls - is wbt guaranteed to only be setup for a queue with
> > disk?
> 
> Same for this one.

OK, fair enough then. Feel free to add:

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

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 4/5] block: move the bdi from the request_queue to the gendisk

[Christoph Hellwig]

On Mon, Aug 09, 2021 at 05:47:28PM +0200, Jan Kara wrote:
> > diff --git a/block/blk-mq.c b/block/blk-mq.c
> > index 2c4ac51e54eb..d2725f94491d 100644
> > --- a/block/blk-mq.c
> > +++ b/block/blk-mq.c
> > @@ -525,7 +525,7 @@ void blk_mq_free_request(struct request *rq)
> >  		__blk_mq_dec_active_requests(hctx);
> >  
> >  	if (unlikely(laptop_mode && !blk_rq_is_passthrough(rq)))
> > -		laptop_io_completion(q->backing_dev_info);
> > +		laptop_io_completion(queue_to_disk(q)->bdi);
> > 
> 
> E.g. cannot this get called for a queue that is without a disk?

As Jens already explained we need the gendisk for non-passthrough
commands.  Same for the wbt case.

[sparc64] kernel OOPS (was: [PATCH 4/5] block: move the bdi from the request_queue to the gendisk)

[Anatoly Pugachev]

On Mon, Aug 09, 2021 at 04:17:43PM +0200, Christoph Hellwig wrote:
> The backing device information only makes sense for file system I/O,
> and thus belongs into the gendisk and not the lower level request_queue
> structure.  Move it there.



Hello!

Using util-linux/ test suite, got the following kernel OOPS:

root@ttip:/home/mator# cd /1/mator/util-linux/tests
root@ttip:/1/mator/util-linux/tests# uname -a
Linux ttip 5.15.0-rc5 #280 SMP Mon Oct 11 12:50:27 MSK 2021 sparc64 GNU/Linux

root@ttip:/1/mator/util-linux/tests# for i in {1..5}; do echo run $i; ./run.sh fdisk; done
run 1

-------------------- util-linux regression tests --------------------

                    For development purpose only.
                 Don't execute on production system!

       kernel: 5.15.0-rc5

      options: --srcdir=/1/mator/util-linux/tests/.. \
               --builddir=/1/mator/util-linux/tests/..

        fdisk: align 512/4K                   ... OK
        fdisk: align 512/4K +alignment_offset ...

^^ hangs

(using bash 'for' cycle, since it's not always OOPS on the first run, but with probability of about 95%)

console/kernel logs (5.15.0-rc5):

[  151.232610] scsi_debug:sdebug_driver_probe: scsi_debug: trim poll_queues to 0. poll_q/nr_hw = (0/1)
[  151.232680] scsi host0: scsi_debug: version 0190 [20200710]
[  151.232680]   dev_size_mb=50, opts=0x0, submit_queues=1, statistics=0
[  151.238292] scsi 0:0:0:0: Direct-Access     Linux    scsi_debug       0190 PQ: 0 ANSI: 7
[  151.239514] sd 0:0:0:0: Attached scsi generic sg0 type 0
[  151.240739] sd 0:0:0:0: Power-on or device reset occurred
[  151.249191] sd 0:0:0:0: [sda] 102400 512-byte logical blocks: (52.4 MB/50.0 MiB)
[  151.249226] sd 0:0:0:0: [sda] 4096-byte physical blocks
[  151.253293] sd 0:0:0:0: [sda] Write Protect is off
[  151.261403] sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, supports DPO and FUA
[  151.273553] sd 0:0:0:0: [sda] Optimal transfer size 524288 bytes
[  151.443048] sd 0:0:0:0: [sda] Attached SCSI disk
[  152.500133]  sda: sda1 sda2 sda3 sda4 < sda5 sda6 sda7 >
[  152.772862] sd 0:0:0:0: [sda] Synchronizing SCSI cache
[  153.136441] scsi_debug:sdebug_driver_probe: scsi_debug: trim poll_queues to 0. poll_q/nr_hw = (0/1)
[  153.136509] scsi host0: scsi_debug: version 0190 [20200710]
[  153.136509]   dev_size_mb=50, opts=0x0, submit_queues=1, statistics=0
[  153.141634] scsi 0:0:0:0: Direct-Access     Linux    scsi_debug       0190 PQ: 0 ANSI: 7
[  153.142777] sd 0:0:0:0: Attached scsi generic sg0 type 0
[  153.143829] sd 0:0:0:0: Power-on or device reset occurred
[  153.152034] sd 0:0:0:0: [sda] physical block alignment offset: 3584
[  153.152246] sd 0:0:0:0: [sda] 102400 512-byte logical blocks: (52.4 MB/50.0 MiB)
[  153.152277] sd 0:0:0:0: [sda] 4096-byte physical blocks
[  153.156347] sd 0:0:0:0: [sda] Write Protect is off
[  153.164454] sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, supports DPO and FUA
[  153.176605] sd 0:0:0:0: [sda] Optimal transfer size 524288 bytes
[  153.334960] sd 0:0:0:0: [sda] Attached SCSI disk
[  154.435981]  sda: sda1 sda2 sda3 sda4 < sda5 sda6 sda7 >
[  154.708897] sd 0:0:0:0: [sda] Synchronizing SCSI cache
[  154.766352] Unable to handle kernel NULL pointer dereference
[  154.766391] tsk->{mm,active_mm}->context = 0000000000000206
[  154.766418] tsk->{mm,active_mm}->pgd = fff800003cf98000
[  154.766440]               \|/ ____ \|/
[  154.766440]               "@'/ .. \`@"
[  154.766440]               /_| \__/ |_\
[  154.766440]                  \__U_/
[  154.766488] swapper/0(0): Oops [#1]
[  154.766508] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.15.0-rc5 #280
[  154.766536] TSTATE: 0000004480001603 TPC: 00000000009b8b1c TNPC: 00000000009b8ec4 Y: 00000000    Not tainted
[  154.766573] TPC: <latency_exceeded+0x1c/0x3e0>
[  154.766600] g0: 8226a448beb34e3c g1: 0000000000000000 g2: 0000000000000000 g3: 0000000000000000
[  154.766632] g4: 0000000000faf680 g5: fff800042963c000 g6: 0000000000f90000 g7: 000000000000ffff
[  154.766663] o0: 0000000000000000 o1: 000000000000ffff o2: 000000000000000e o3: 0000000000f0c350
[  154.766694] o4: 0000000001200768 o5: 000000000124b508 sp: fff800042f8671a1 ret_pc: 00000000009ca4a0
[  154.766727] RPC: <_find_next_bit+0x160/0x1c0>
[  154.766751] l0: 7fffffffffffffff l1: fff800042a803ea8 l2: fff800042a803f28 l3: fff800042a803fa8
[  154.766783] l4: f6b5b5b4df3cd40c l5: 00000000014b5800 l6: 00000000014b5800 l7: fff800042f867c78
[  154.766815] i0: fff8000047b97400 i1: fff800004fcd1980 i2: 0000000000000100 i3: 0000000000000100
[  154.766846] i4: 0000000000000000 i5: 0000000000000000 i6: fff800042f867251 i7: 00000000009b8f10
[  154.766877] I7: <wb_timer_fn+0x30/0x1c0>
[  154.766897] Call Trace:
[  154.766911] [<00000000009b8f10>] wb_timer_fn+0x30/0x1c0
[  154.766934] [<000000000099b564>] blk_stat_timer_fn+0x184/0x1a0
[  154.766962] [<00000000004fe90c>] call_timer_fn+0xec/0x200
[  154.766987] [<00000000004fec6c>] __run_timers+0x24c/0x340
[  154.767010] [<00000000004fed74>] run_timer_softirq+0x14/0x40
[  154.767033] [<0000000000cada0c>] __do_softirq+0x1ac/0x3c0
[  154.767061] [<000000000042bb74>] do_softirq_own_stack+0x34/0x60
[  154.767090] [<000000000046c97c>] irq_exit+0x7c/0x120
[  154.767116] [<0000000000cad6b8>] timer_interrupt+0x98/0xc0
[  154.767140] [<00000000004209d4>] tl0_irq14+0x14/0x20
[  154.767162] [<000000000042c384>] arch_cpu_idle+0xa4/0xc0
[  154.767185] [<0000000000cab0d4>] default_idle_call+0xb4/0x160
[  154.767208] [<00000000004a7414>] do_idle+0x114/0x1a0
[  154.767234] [<00000000004a771c>] cpu_startup_entry+0x1c/0xa0
[  154.767258] [<0000000000ca1f2c>] rest_init+0x14c/0x15c
[  154.767285] [<00000000011469a8>] arch_call_rest_init+0xc/0x1c
[  154.767314] Disabling lock debugging due to kernel taint
[  154.767321] Caller[00000000009b8f10]: wb_timer_fn+0x30/0x1c0
[  154.767330] Caller[000000000099b564]: blk_stat_timer_fn+0x184/0x1a0
[  154.767339] Caller[00000000004fe90c]: call_timer_fn+0xec/0x200
[  154.767347] Caller[00000000004fec6c]: __run_timers+0x24c/0x340
[  154.767354] Caller[00000000004fed74]: run_timer_softirq+0x14/0x40
[  154.767362] Caller[0000000000cada0c]: __do_softirq+0x1ac/0x3c0
[  154.767371] Caller[000000000042bb74]: do_softirq_own_stack+0x34/0x60
[  154.767379] Caller[000000000046c97c]: irq_exit+0x7c/0x120
[  154.767388] Caller[0000000000cad6b8]: timer_interrupt+0x98/0xc0
[  154.767396] Caller[00000000004209d4]: tl0_irq14+0x14/0x20
[  154.767404] Caller[000000000042c370]: arch_cpu_idle+0x90/0xc0
[  154.767412] Caller[0000000000cab0d4]: default_idle_call+0xb4/0x160
[  154.767420] Caller[00000000004a7414]: do_idle+0x114/0x1a0
[  154.767428] Caller[00000000004a771c]: cpu_startup_entry+0x1c/0xa0
[  154.767436] Caller[0000000000ca1f2c]: rest_init+0x14c/0x15c
[  154.767444] Caller[00000000011469a8]: arch_call_rest_init+0xc/0x1c
[  154.767453] Caller[0000000001146f5c]: start_kernel+0x52c/0x544
[  154.767462] Caller[0000000001149ff0]: start_early_boot+0x68/0x78
[  154.767470] Caller[0000000000ca1dc0]: tlb_fixup_done+0x4c/0x6c
[  154.767479] Caller[0000000000027414]: 0x27414
[  154.767486] Instruction DUMP:
[  154.767488]  fa5e2028
[  154.767494]  c25860b0
[  154.767499]  02c740eb
[  154.767504] <f6586148>
[  154.767509]  c25e2030
[  154.767514]  22c040e9
[  154.767519]  c45e2050
[  154.767523]  7fed2bd5
[  154.767528]  01000000
[  154.767533]
[  154.767542] Kernel panic - not syncing: Aiee, killing interrupt handler!
[  154.768930] Press Stop-A (L1-A) from sun keyboard or send break
[  154.768930] twice on console to return to the boot prom
[  154.768940] ---[ end Kernel panic - not syncing: Aiee, killing interrupt handler! ]---


bisected to the following kernel commit:

linux-2.6$ git bisect bad
edb0872f44ec9976ea6d052cb4b93cd2d23ac2ba is the first bad commit
commit edb0872f44ec9976ea6d052cb4b93cd2d23ac2ba
Author: Christoph Hellwig <hch@lst.de>
Date:   Mon Aug 9 16:17:43 2021 +0200

    block: move the bdi from the request_queue to the gendisk

    The backing device information only makes sense for file system I/O,
    and thus belongs into the gendisk and not the lower level request_queue
    structure.  Move it there.

    Signed-off-by: Christoph Hellwig <hch@lst.de>
    Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
    Link: https://lore.kernel.org/r/20210809141744.1203023-5-hch@lst.de
    Signed-off-by: Jens Axboe <axboe@kernel.dk>

 block/bfq-iosched.c           |  4 ++--
 block/blk-cgroup.c            |  7 +++----
 block/blk-core.c              | 13 +++----------
 block/blk-mq.c                |  2 +-
 block/blk-settings.c          | 14 +++++++++-----
 block/blk-sysfs.c             | 26 ++++++++++++--------------
 block/blk-wbt.c               | 10 +++++-----
 block/genhd.c                 | 23 ++++++++++++++---------
 drivers/block/drbd/drbd_req.c |  5 ++---
 drivers/block/pktcdvd.c       |  8 +++-----
 fs/block_dev.c                |  4 ++--
 fs/fat/fatent.c               |  1 +
 include/linux/blkdev.h        |  3 ---
 include/linux/genhd.h         |  1 +
 14 files changed, 58 insertions(+), 63 deletions(-)


linux-2.6$ git bisect log
git bisect start
# good: [7d2a07b769330c34b4deabeed939325c77a7ec2f] Linux 5.14
git bisect good 7d2a07b769330c34b4deabeed939325c77a7ec2f
# bad: [6880fa6c56601bb8ed59df6c30fd390cc5f6dd8f] Linux 5.15-rc1
git bisect bad 6880fa6c56601bb8ed59df6c30fd390cc5f6dd8f
# bad: [1b4f3dfb4792f03b139edf10124fcbeb44e608e6] Merge tag 'usb-serial-5.15-rc1' of https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial into usb-next
git bisect bad 1b4f3dfb4792f03b139edf10124fcbeb44e608e6
# good: [29ce8f9701072fc221d9c38ad952de1a9578f95c] Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net
git bisect good 29ce8f9701072fc221d9c38ad952de1a9578f95c
# bad: [e7c1bbcf0c315c56cd970642214aa1df3d8cf61d] Merge tag 'hwmon-for-v5.15' of git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging
git bisect bad e7c1bbcf0c315c56cd970642214aa1df3d8cf61d
# bad: [679369114e55f422dc593d0628cfde1d04ae59b3] Merge tag 'for-5.15/block-2021-08-30' of git://git.kernel.dk/linux-block
git bisect bad 679369114e55f422dc593d0628cfde1d04ae59b3
# good: [c7a5238ef68b98130fe36716bb3fa44502f56001] Merge tag 's390-5.15-1' of git://git.kernel.org/pub/scm/linux/kernel/git/s390/linux
git bisect good c7a5238ef68b98130fe36716bb3fa44502f56001
# good: [e5e726f7bb9f711102edea7e5bd511835640e3b4] Merge tag 'locking-core-2021-08-30' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect good e5e726f7bb9f711102edea7e5bd511835640e3b4
# bad: [158ee7b65653d9f841823c249014c2d0dfdeeb8f] block: mark blkdev_fsync static
git bisect bad 158ee7b65653d9f841823c249014c2d0dfdeeb8f
# bad: [b75f4aed88febe903bd40a6128b74edd2388417e] bcache: move the del_gendisk call out of bcache_device_free
git bisect bad b75f4aed88febe903bd40a6128b74edd2388417e
# good: [cf179948554a2e0d2b622317bf6bf33138ac36e5] block: add disk sequence number
git bisect good cf179948554a2e0d2b622317bf6bf33138ac36e5
# good: [89f871af1b26d98d983cba7ed0e86effa45ba5f8] dm: delay registering the gendisk
git bisect good 89f871af1b26d98d983cba7ed0e86effa45ba5f8
# bad: [99d26de2f6d79badc80f55b54bd90d4cb9d1ad90] writeback: make the laptop_mode prototypes available unconditionally
git bisect bad 99d26de2f6d79badc80f55b54bd90d4cb9d1ad90
# good: [1008162b2782a3624d12b0aee8da58bc75d12e19] block: add a queue_has_disk helper
git bisect good 1008162b2782a3624d12b0aee8da58bc75d12e19
# bad: [a11d7fc2d05fb509cd9e33d4093507d6eda3ad53] block: remove the bd_bdi in struct block_device
git bisect bad a11d7fc2d05fb509cd9e33d4093507d6eda3ad53
# bad: [edb0872f44ec9976ea6d052cb4b93cd2d23ac2ba] block: move the bdi from the request_queue to the gendisk
git bisect bad edb0872f44ec9976ea6d052cb4b93cd2d23ac2ba
# first bad commit: [edb0872f44ec9976ea6d052cb4b93cd2d23ac2ba] block: move the bdi from the request_queue to the gendisk


kernel/console logs for 5.14.0-rc4-00051-gedb0872f44ec :

ttip login: [   27.667716] SCSI subsystem initialized
[   27.680003] scsi_debug:sdebug_driver_probe: scsi_debug: trim poll_queues to 0. poll_q/nr_hw = (0/1)
[   27.680069] scsi host0: scsi_debug: version 0190 [20200710]
[   27.680069]   dev_size_mb=50, opts=0x0, submit_queues=1, statistics=0
[   27.685444] scsi 0:0:0:0: Direct-Access     Linux    scsi_debug       0190 PQ: 0 ANSI: 7
[   27.692723] scsi 0:0:0:0: Attached scsi generic sg0 type 0
[   27.699206] sd 0:0:0:0: Power-on or device reset occurred
[   27.707641] sd 0:0:0:0: [sda] 102400 512-byte logical blocks: (52.4 MB/50.0 MiB)
[   27.707676] sd 0:0:0:0: [sda] 4096-byte physical blocks
[   27.711747] sd 0:0:0:0: [sda] Write Protect is off
[   27.719862] sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, supports DPO and FUA
[   27.732012] sd 0:0:0:0: [sda] Optimal transfer size 524288 bytes
[   27.882257] sd 0:0:0:0: [sda] Attached SCSI disk
[   29.203392]  sda: sda1 sda2 sda3 sda4 < sda5 sda6 sda7 >
[   29.486552] sd 0:0:0:0: [sda] Synchronizing SCSI cache
[   29.517694] Unable to handle kernel NULL pointer dereference
[   29.517722] tsk->{mm,active_mm}->context = 0000000000000422
[   29.517748] tsk->{mm,active_mm}->pgd = fff800004b1c4000
[   29.517770]               \|/ ____ \|/
[   29.517770]               "@'/ .. \`@"
[   29.517770]               /_| \__/ |_\
[   29.517770]                  \__U_/
[   29.517820] swapper/15(0): Oops [#1]
[   29.517841] CPU: 15 PID: 0 Comm: swapper/15 Not tainted 5.14.0-rc4-00051-gedb0872f44ec #308
[   29.517876] TSTATE: 0000004480001600 TPC: 00000000009af598 TNPC: 00000000009af59c Y: 00000000    Not tainted
[   29.517913] TPC: <latency_exceeded+0x18/0x400>
[   29.517942] g0: fff800003fdd86c0 g1: 0000000000000000 g2: 0000000000000000 g3: 0000000000000000
[   29.517974] g4: fff8000034f013c0 g5: fff8000429a12000 g6: fff8000034f3c000 g7: 000000000000ffff
[   29.518006] o0: 0000000000000000 o1: 000000000000ffff o2: 0000000000000000 o3: 0000000000ef8a50
[   29.518038] o4: 00000000011ee728 o5: 0000000001238c88 sp: fff800042efef1a1 ret_pc: 00000000009c0d40
[   29.518071] RPC: <_find_next_bit+0x160/0x1c0>
[   29.518093] l0: fff800042abc7d88 l1: fff800042abc7ea8 l2: fff800042abc7f28 l3: fff800042abc7fa8
[   29.518126] l4: f6b5b5b4df3cd40c l5: 00000000014a3800 l6: 00000000014a3800 l7: fff800042efefc78
[   29.518158] i0: fff800004b071a00 i1: fff8000042589080 i2: 0000000000000100 i3: 0000000000000100
[   29.518190] i4: 0000000000000000 i5: 0000000000000000 i6: fff800042efef251 i7: 00000000009af9b0
[   29.518221] I7: <wb_timer_fn+0x30/0x1c0>
[   29.518241] Call Trace:
[   29.518255] [<00000000009af9b0>] wb_timer_fn+0x30/0x1c0
[   29.518278] [<0000000000990024>] blk_stat_timer_fn+0x184/0x1a0
[   29.518304] [<00000000004fdcec>] call_timer_fn+0xec/0x200
[   29.518329] [<00000000004fe050>] __run_timers+0x250/0x340
[   29.518352] [<00000000004fe154>] run_timer_softirq+0x14/0x40
[   29.518376] [<0000000000c9e9ac>] __do_softirq+0x1ac/0x3c0
[   29.518403] [<000000000042bbb4>] do_softirq_own_stack+0x34/0x60
[   29.518432] [<000000000046c63c>] irq_exit+0x7c/0x120
[   29.518456] [<0000000000c9e658>] timer_interrupt+0x98/0xc0
[   29.518481] [<00000000004209d4>] tl0_irq14+0x14/0x20
[   29.518504] [<000000000042c3c4>] arch_cpu_idle+0xa4/0xc0
[   29.518528] [<0000000000c9c034>] default_idle_call+0xb4/0x160
[   29.518552] [<00000000004a6bf8>] do_idle+0x118/0x1a0
[   29.518575] [<00000000004a6efc>] cpu_startup_entry+0x1c/0xa0
[   29.518599] [<000000000043fe2c>] smp_callin+0x10c/0x120
[   29.518622] [<0000000000fa4fb4>] after_lock_tlb+0x1a8/0x1bc
[   29.518649] Disabling lock debugging due to kernel taint
[   29.518656] Caller[00000000009af9b0]: wb_timer_fn+0x30/0x1c0
[   29.518664] Caller[0000000000990024]: blk_stat_timer_fn+0x184/0x1a0
[   29.518673] Caller[00000000004fdcec]: call_timer_fn+0xec/0x200
[   29.518681] Caller[00000000004fe050]: __run_timers+0x250/0x340
[   29.518689] Caller[00000000004fe154]: run_timer_softirq+0x14/0x40
[   29.518697] Caller[0000000000c9e9ac]: __do_softirq+0x1ac/0x3c0
[   29.518706] Caller[000000000042bbb4]: do_softirq_own_stack+0x34/0x60
[   29.518714] Caller[000000000046c63c]: irq_exit+0x7c/0x120
[   29.518723] Caller[0000000000c9e658]: timer_interrupt+0x98/0xc0
[   29.518731] Caller[00000000004209d4]: tl0_irq14+0x14/0x20
[   29.518739] Caller[000000000042c3b0]: arch_cpu_idle+0x90/0xc0
[   29.518747] Caller[0000000000c9c034]: default_idle_call+0xb4/0x160
[   29.518756] Caller[00000000004a6bf8]: do_idle+0x118/0x1a0
[   29.518764] Caller[00000000004a6efc]: cpu_startup_entry+0x1c/0xa0
[   29.518772] Caller[000000000043fe2c]: smp_callin+0x10c/0x120
[   29.518780] Caller[0000000000fa4fb4]: after_lock_tlb+0x1a8/0x1bc
[   29.518788] Caller[0000000000000000]: 0x0
[   29.518796] Instruction DUMP:
[   29.518798]  c25e2060
[   29.518804]  fa5e2028
[   29.518809]  c25860c8
[   29.518814] <c2586320>
[   29.518819]  02c740ec
[   29.518825]  f6586148
[   29.518829]  c25e2030
[   29.518834]  22c040ea
[   29.518839]  c45e2050
[   29.518844]
[   29.518853] Kernel panic - not syncing: Aiee, killing interrupt handler!
[   29.520474] Press Stop-A (L1-A) from sun keyboard or send break
[   29.520474] twice on console to return to the boot prom
[   29.520485] ---[ end Kernel panic - not syncing: Aiee, killing interrupt handler! ]---

sparc64 kernel 5.14.0-rc4-00050-g1008162b2782 never hangs/OOPS on this
util-linux fdisk test.

PS: compiled with gcc-10 
$ gcc --version
gcc (Debian 10.3.0-11) 10.3.0

Re: [sparc64] kernel OOPS (was: [PATCH 4/5] block: move the bdi from the request_queue to the gendisk)

[Christoph Hellwig]

Hi Anatoly,

please try this patchset:

https://lore.kernel.org/linux-block/CAHj4cs8tYY-ShH=QdrVirwXqX4Uze6ewZAGew_oRKLL_CCLNJg@mail.gmail.com/T/#m6591be7882bf30f3538a8baafbac1712f0763ebb

Re: [sparc64] kernel OOPS (was: [PATCH 4/5] block: move the bdi from the request_queue to the gendisk)

[Christoph Hellwig]

Hi Anatoly,

please try this patchset:

https://lore.kernel.org/linux-block/CAHj4cs8tYY-ShH=QdrVirwXqX4Uze6ewZAGew_oRKLL_CCLNJg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org/T/#m6591be7882bf30f3538a8baafbac1712f0763ebb

Re: [sparc64] kernel OOPS (was: [PATCH 4/5] block: move the bdi from the request_queue to the gendisk)

[Anatoly Pugachev]

On Thu, Oct 14, 2021 at 5:32 PM Christoph Hellwig <hch@lst.de> wrote:
>
> Hi Anatoly,
>
> please try this patchset:
>
> https://lore.kernel.org/linux-block/CAHj4cs8tYY-ShH=QdrVirwXqX4Uze6ewZAGew_oRKLL_CCLNJg@mail.gmail.com/T/#m6591be7882bf30f3538a8baafbac1712f0763ebb

Christoph,

thanks. Tested (with 5.15.0-rc5 + patchset) and no-more hangs with the
test-suite. Thanks again.

[PATCH 5/5] block: remove the bd_bdi in struct block_device

[Christoph Hellwig]

Just retrieve the bdi from the disk.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/ioctl.c               |  7 ++++---
 fs/block_dev.c              | 13 +------------
 fs/nilfs2/super.c           |  2 +-
 fs/super.c                  |  2 +-
 fs/xfs/xfs_buf.c            |  2 +-
 include/linux/backing-dev.h |  2 +-
 include/linux/blk_types.h   |  1 -
 7 files changed, 9 insertions(+), 20 deletions(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index 0c3a4a53fa11..fff161eaab42 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -506,7 +506,7 @@ static int blkdev_common_ioctl(struct block_device *bdev, fmode_t mode,
 	case BLKFRASET:
 		if(!capable(CAP_SYS_ADMIN))
 			return -EACCES;
-		bdev->bd_bdi->ra_pages = (arg * 512) / PAGE_SIZE;
+		bdev->bd_disk->bdi->ra_pages = (arg * 512) / PAGE_SIZE;
 		return 0;
 	case BLKRRPART:
 		return blkdev_reread_part(bdev, mode);
@@ -556,7 +556,8 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
 	case BLKFRAGET:
 		if (!argp)
 			return -EINVAL;
-		return put_long(argp, (bdev->bd_bdi->ra_pages*PAGE_SIZE) / 512);
+		return put_long(argp,
+			(bdev->bd_disk->bdi->ra_pages * PAGE_SIZE) / 512);
 	case BLKGETSIZE:
 		size = i_size_read(bdev->bd_inode);
 		if ((size >> 9) > ~0UL)
@@ -628,7 +629,7 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
 		if (!argp)
 			return -EINVAL;
 		return compat_put_long(argp,
-			       (bdev->bd_bdi->ra_pages * PAGE_SIZE) / 512);
+			(bdev->bd_disk->bdi->ra_pages * PAGE_SIZE) / 512);
 	case BLKGETSIZE:
 		size = i_size_read(bdev->bd_inode);
 		if ((size >> 9) > ~0UL)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index de8c3d9cbdb1..65fc0efca26b 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -801,7 +801,6 @@ static struct inode *bdev_alloc_inode(struct super_block *sb)
 	if (!ei)
 		return NULL;
 	memset(&ei->bdev, 0, sizeof(ei->bdev));
-	ei->bdev.bd_bdi = &noop_backing_dev_info;
 	return &ei->vfs_inode;
 }
 
@@ -826,16 +825,11 @@ static void init_once(void *data)
 
 static void bdev_evict_inode(struct inode *inode)
 {
-	struct block_device *bdev = &BDEV_I(inode)->bdev;
 	truncate_inode_pages_final(&inode->i_data);
 	invalidate_inode_buffers(inode); /* is it needed here? */
 	clear_inode(inode);
 	/* Detach inode from wb early as bdi_put() may free bdi->wb */
 	inode_detach_wb(inode);
-	if (bdev->bd_bdi != &noop_backing_dev_info) {
-		bdi_put(bdev->bd_bdi);
-		bdev->bd_bdi = &noop_backing_dev_info;
-	}
 }
 
 static const struct super_operations bdev_sops = {
@@ -1229,11 +1223,8 @@ static int blkdev_get_whole(struct block_device *bdev, fmode_t mode)
 		}
 	}
 
-	if (!bdev->bd_openers) {
+	if (!bdev->bd_openers)
 		set_init_blocksize(bdev);
-		if (bdev->bd_bdi == &noop_backing_dev_info)
-			bdev->bd_bdi = bdi_get(disk->bdi);
-	}
 	if (test_bit(GD_NEED_PART_SCAN, &disk->state))
 		bdev_disk_changed(disk, false);
 	bdev->bd_openers++;
@@ -1266,8 +1257,6 @@ static int blkdev_get_part(struct block_device *part, fmode_t mode)
 
 	disk->open_partitions++;
 	set_init_blocksize(part);
-	if (part->bd_bdi == &noop_backing_dev_info)
-		part->bd_bdi = bdi_get(disk->bdi);
 done:
 	part->bd_openers++;
 	return 0;
diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index 4abd928b0bc8..f6b2d280aab5 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -1053,7 +1053,7 @@ nilfs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_time_gran = 1;
 	sb->s_max_links = NILFS_LINK_MAX;
 
-	sb->s_bdi = bdi_get(sb->s_bdev->bd_bdi);
+	sb->s_bdi = bdi_get(sb->s_bdev->bd_disk->bdi);
 
 	err = load_nilfs(nilfs, sb);
 	if (err)
diff --git a/fs/super.c b/fs/super.c
index 91b7f156735b..bcef3a6f4c4b 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1203,7 +1203,7 @@ static int set_bdev_super(struct super_block *s, void *data)
 {
 	s->s_bdev = data;
 	s->s_dev = s->s_bdev->bd_dev;
-	s->s_bdi = bdi_get(s->s_bdev->bd_bdi);
+	s->s_bdi = bdi_get(s->s_bdev->bd_disk->bdi);
 
 	if (blk_queue_stable_writes(s->s_bdev->bd_disk->queue))
 		s->s_iflags |= SB_I_STABLE_WRITES;
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 8ff42b3585e0..3ab73567a0f5 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -844,7 +844,7 @@ xfs_buf_readahead_map(
 {
 	struct xfs_buf		*bp;
 
-	if (bdi_read_congested(target->bt_bdev->bd_bdi))
+	if (bdi_read_congested(target->bt_bdev->bd_disk->bdi))
 		return;
 
 	xfs_buf_read_map(target, map, nmaps,
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index 44df4fcef65c..29530859d9ff 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -143,7 +143,7 @@ static inline struct backing_dev_info *inode_to_bdi(struct inode *inode)
 	sb = inode->i_sb;
 #ifdef CONFIG_BLOCK
 	if (sb_is_blkdev_sb(sb))
-		return I_BDEV(inode)->bd_bdi;
+		return I_BDEV(inode)->bd_disk->bdi;
 #endif
 	return sb->s_bdi;
 }
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 290f9061b29a..a6c015cedaf7 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -41,7 +41,6 @@ struct block_device {
 	u8			bd_partno;
 	spinlock_t		bd_size_lock; /* for bd_inode->i_size updates */
 	struct gendisk *	bd_disk;
-	struct backing_dev_info *bd_bdi;
 
 	/* The counter of freeze processes */
 	int			bd_fsfreeze_count;
-- 
2.30.2

[PATCH 5/5] block: remove the bd_bdi in struct block_device

[Christoph Hellwig]

Just retrieve the bdi from the disk.

Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
---
 block/ioctl.c               |  7 ++++---
 fs/block_dev.c              | 13 +------------
 fs/nilfs2/super.c           |  2 +-
 fs/super.c                  |  2 +-
 fs/xfs/xfs_buf.c            |  2 +-
 include/linux/backing-dev.h |  2 +-
 include/linux/blk_types.h   |  1 -
 7 files changed, 9 insertions(+), 20 deletions(-)

diff --git a/block/ioctl.c b/block/ioctl.c
index 0c3a4a53fa11..fff161eaab42 100644
--- a/block/ioctl.c
+++ b/block/ioctl.c
@@ -506,7 +506,7 @@ static int blkdev_common_ioctl(struct block_device *bdev, fmode_t mode,
 	case BLKFRASET:
 		if(!capable(CAP_SYS_ADMIN))
 			return -EACCES;
-		bdev->bd_bdi->ra_pages = (arg * 512) / PAGE_SIZE;
+		bdev->bd_disk->bdi->ra_pages = (arg * 512) / PAGE_SIZE;
 		return 0;
 	case BLKRRPART:
 		return blkdev_reread_part(bdev, mode);
@@ -556,7 +556,8 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
 	case BLKFRAGET:
 		if (!argp)
 			return -EINVAL;
-		return put_long(argp, (bdev->bd_bdi->ra_pages*PAGE_SIZE) / 512);
+		return put_long(argp,
+			(bdev->bd_disk->bdi->ra_pages * PAGE_SIZE) / 512);
 	case BLKGETSIZE:
 		size = i_size_read(bdev->bd_inode);
 		if ((size >> 9) > ~0UL)
@@ -628,7 +629,7 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
 		if (!argp)
 			return -EINVAL;
 		return compat_put_long(argp,
-			       (bdev->bd_bdi->ra_pages * PAGE_SIZE) / 512);
+			(bdev->bd_disk->bdi->ra_pages * PAGE_SIZE) / 512);
 	case BLKGETSIZE:
 		size = i_size_read(bdev->bd_inode);
 		if ((size >> 9) > ~0UL)
diff --git a/fs/block_dev.c b/fs/block_dev.c
index de8c3d9cbdb1..65fc0efca26b 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -801,7 +801,6 @@ static struct inode *bdev_alloc_inode(struct super_block *sb)
 	if (!ei)
 		return NULL;
 	memset(&ei->bdev, 0, sizeof(ei->bdev));
-	ei->bdev.bd_bdi = &noop_backing_dev_info;
 	return &ei->vfs_inode;
 }
 
@@ -826,16 +825,11 @@ static void init_once(void *data)
 
 static void bdev_evict_inode(struct inode *inode)
 {
-	struct block_device *bdev = &BDEV_I(inode)->bdev;
 	truncate_inode_pages_final(&inode->i_data);
 	invalidate_inode_buffers(inode); /* is it needed here? */
 	clear_inode(inode);
 	/* Detach inode from wb early as bdi_put() may free bdi->wb */
 	inode_detach_wb(inode);
-	if (bdev->bd_bdi != &noop_backing_dev_info) {
-		bdi_put(bdev->bd_bdi);
-		bdev->bd_bdi = &noop_backing_dev_info;
-	}
 }
 
 static const struct super_operations bdev_sops = {
@@ -1229,11 +1223,8 @@ static int blkdev_get_whole(struct block_device *bdev, fmode_t mode)
 		}
 	}
 
-	if (!bdev->bd_openers) {
+	if (!bdev->bd_openers)
 		set_init_blocksize(bdev);
-		if (bdev->bd_bdi == &noop_backing_dev_info)
-			bdev->bd_bdi = bdi_get(disk->bdi);
-	}
 	if (test_bit(GD_NEED_PART_SCAN, &disk->state))
 		bdev_disk_changed(disk, false);
 	bdev->bd_openers++;
@@ -1266,8 +1257,6 @@ static int blkdev_get_part(struct block_device *part, fmode_t mode)
 
 	disk->open_partitions++;
 	set_init_blocksize(part);
-	if (part->bd_bdi == &noop_backing_dev_info)
-		part->bd_bdi = bdi_get(disk->bdi);
 done:
 	part->bd_openers++;
 	return 0;
diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index 4abd928b0bc8..f6b2d280aab5 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -1053,7 +1053,7 @@ nilfs_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_time_gran = 1;
 	sb->s_max_links = NILFS_LINK_MAX;
 
-	sb->s_bdi = bdi_get(sb->s_bdev->bd_bdi);
+	sb->s_bdi = bdi_get(sb->s_bdev->bd_disk->bdi);
 
 	err = load_nilfs(nilfs, sb);
 	if (err)
diff --git a/fs/super.c b/fs/super.c
index 91b7f156735b..bcef3a6f4c4b 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1203,7 +1203,7 @@ static int set_bdev_super(struct super_block *s, void *data)
 {
 	s->s_bdev = data;
 	s->s_dev = s->s_bdev->bd_dev;
-	s->s_bdi = bdi_get(s->s_bdev->bd_bdi);
+	s->s_bdi = bdi_get(s->s_bdev->bd_disk->bdi);
 
 	if (blk_queue_stable_writes(s->s_bdev->bd_disk->queue))
 		s->s_iflags |= SB_I_STABLE_WRITES;
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 8ff42b3585e0..3ab73567a0f5 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -844,7 +844,7 @@ xfs_buf_readahead_map(
 {
 	struct xfs_buf		*bp;
 
-	if (bdi_read_congested(target->bt_bdev->bd_bdi))
+	if (bdi_read_congested(target->bt_bdev->bd_disk->bdi))
 		return;
 
 	xfs_buf_read_map(target, map, nmaps,
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index 44df4fcef65c..29530859d9ff 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -143,7 +143,7 @@ static inline struct backing_dev_info *inode_to_bdi(struct inode *inode)
 	sb = inode->i_sb;
 #ifdef CONFIG_BLOCK
 	if (sb_is_blkdev_sb(sb))
-		return I_BDEV(inode)->bd_bdi;
+		return I_BDEV(inode)->bd_disk->bdi;
 #endif
 	return sb->s_bdi;
 }
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 290f9061b29a..a6c015cedaf7 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -41,7 +41,6 @@ struct block_device {
 	u8			bd_partno;
 	spinlock_t		bd_size_lock; /* for bd_inode->i_size updates */
 	struct gendisk *	bd_disk;
-	struct backing_dev_info *bd_bdi;
 
 	/* The counter of freeze processes */
 	int			bd_fsfreeze_count;
-- 
2.30.2

Re: [PATCH 5/5] block: remove the bd_bdi in struct block_device

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 5/5] block: remove the bd_bdi in struct block_device

[Jan Kara]

On Mon 09-08-21 16:17:44, Christoph Hellwig wrote:
> Just retrieve the bdi from the disk.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good to me. Feel free to add:

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

								Honza

> ---
>  block/ioctl.c               |  7 ++++---
>  fs/block_dev.c              | 13 +------------
>  fs/nilfs2/super.c           |  2 +-
>  fs/super.c                  |  2 +-
>  fs/xfs/xfs_buf.c            |  2 +-
>  include/linux/backing-dev.h |  2 +-
>  include/linux/blk_types.h   |  1 -
>  7 files changed, 9 insertions(+), 20 deletions(-)
> 
> diff --git a/block/ioctl.c b/block/ioctl.c
> index 0c3a4a53fa11..fff161eaab42 100644
> --- a/block/ioctl.c
> +++ b/block/ioctl.c
> @@ -506,7 +506,7 @@ static int blkdev_common_ioctl(struct block_device *bdev, fmode_t mode,
>  	case BLKFRASET:
>  		if(!capable(CAP_SYS_ADMIN))
>  			return -EACCES;
> -		bdev->bd_bdi->ra_pages = (arg * 512) / PAGE_SIZE;
> +		bdev->bd_disk->bdi->ra_pages = (arg * 512) / PAGE_SIZE;
>  		return 0;
>  	case BLKRRPART:
>  		return blkdev_reread_part(bdev, mode);
> @@ -556,7 +556,8 @@ int blkdev_ioctl(struct block_device *bdev, fmode_t mode, unsigned cmd,
>  	case BLKFRAGET:
>  		if (!argp)
>  			return -EINVAL;
> -		return put_long(argp, (bdev->bd_bdi->ra_pages*PAGE_SIZE) / 512);
> +		return put_long(argp,
> +			(bdev->bd_disk->bdi->ra_pages * PAGE_SIZE) / 512);
>  	case BLKGETSIZE:
>  		size = i_size_read(bdev->bd_inode);
>  		if ((size >> 9) > ~0UL)
> @@ -628,7 +629,7 @@ long compat_blkdev_ioctl(struct file *file, unsigned cmd, unsigned long arg)
>  		if (!argp)
>  			return -EINVAL;
>  		return compat_put_long(argp,
> -			       (bdev->bd_bdi->ra_pages * PAGE_SIZE) / 512);
> +			(bdev->bd_disk->bdi->ra_pages * PAGE_SIZE) / 512);
>  	case BLKGETSIZE:
>  		size = i_size_read(bdev->bd_inode);
>  		if ((size >> 9) > ~0UL)
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index de8c3d9cbdb1..65fc0efca26b 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -801,7 +801,6 @@ static struct inode *bdev_alloc_inode(struct super_block *sb)
>  	if (!ei)
>  		return NULL;
>  	memset(&ei->bdev, 0, sizeof(ei->bdev));
> -	ei->bdev.bd_bdi = &noop_backing_dev_info;
>  	return &ei->vfs_inode;
>  }
>  
> @@ -826,16 +825,11 @@ static void init_once(void *data)
>  
>  static void bdev_evict_inode(struct inode *inode)
>  {
> -	struct block_device *bdev = &BDEV_I(inode)->bdev;
>  	truncate_inode_pages_final(&inode->i_data);
>  	invalidate_inode_buffers(inode); /* is it needed here? */
>  	clear_inode(inode);
>  	/* Detach inode from wb early as bdi_put() may free bdi->wb */
>  	inode_detach_wb(inode);
> -	if (bdev->bd_bdi != &noop_backing_dev_info) {
> -		bdi_put(bdev->bd_bdi);
> -		bdev->bd_bdi = &noop_backing_dev_info;
> -	}
>  }
>  
>  static const struct super_operations bdev_sops = {
> @@ -1229,11 +1223,8 @@ static int blkdev_get_whole(struct block_device *bdev, fmode_t mode)
>  		}
>  	}
>  
> -	if (!bdev->bd_openers) {
> +	if (!bdev->bd_openers)
>  		set_init_blocksize(bdev);
> -		if (bdev->bd_bdi == &noop_backing_dev_info)
> -			bdev->bd_bdi = bdi_get(disk->bdi);
> -	}
>  	if (test_bit(GD_NEED_PART_SCAN, &disk->state))
>  		bdev_disk_changed(disk, false);
>  	bdev->bd_openers++;
> @@ -1266,8 +1257,6 @@ static int blkdev_get_part(struct block_device *part, fmode_t mode)
>  
>  	disk->open_partitions++;
>  	set_init_blocksize(part);
> -	if (part->bd_bdi == &noop_backing_dev_info)
> -		part->bd_bdi = bdi_get(disk->bdi);
>  done:
>  	part->bd_openers++;
>  	return 0;
> diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
> index 4abd928b0bc8..f6b2d280aab5 100644
> --- a/fs/nilfs2/super.c
> +++ b/fs/nilfs2/super.c
> @@ -1053,7 +1053,7 @@ nilfs_fill_super(struct super_block *sb, void *data, int silent)
>  	sb->s_time_gran = 1;
>  	sb->s_max_links = NILFS_LINK_MAX;
>  
> -	sb->s_bdi = bdi_get(sb->s_bdev->bd_bdi);
> +	sb->s_bdi = bdi_get(sb->s_bdev->bd_disk->bdi);
>  
>  	err = load_nilfs(nilfs, sb);
>  	if (err)
> diff --git a/fs/super.c b/fs/super.c
> index 91b7f156735b..bcef3a6f4c4b 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -1203,7 +1203,7 @@ static int set_bdev_super(struct super_block *s, void *data)
>  {
>  	s->s_bdev = data;
>  	s->s_dev = s->s_bdev->bd_dev;
> -	s->s_bdi = bdi_get(s->s_bdev->bd_bdi);
> +	s->s_bdi = bdi_get(s->s_bdev->bd_disk->bdi);
>  
>  	if (blk_queue_stable_writes(s->s_bdev->bd_disk->queue))
>  		s->s_iflags |= SB_I_STABLE_WRITES;
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 8ff42b3585e0..3ab73567a0f5 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -844,7 +844,7 @@ xfs_buf_readahead_map(
>  {
>  	struct xfs_buf		*bp;
>  
> -	if (bdi_read_congested(target->bt_bdev->bd_bdi))
> +	if (bdi_read_congested(target->bt_bdev->bd_disk->bdi))
>  		return;
>  
>  	xfs_buf_read_map(target, map, nmaps,
> diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
> index 44df4fcef65c..29530859d9ff 100644
> --- a/include/linux/backing-dev.h
> +++ b/include/linux/backing-dev.h
> @@ -143,7 +143,7 @@ static inline struct backing_dev_info *inode_to_bdi(struct inode *inode)
>  	sb = inode->i_sb;
>  #ifdef CONFIG_BLOCK
>  	if (sb_is_blkdev_sb(sb))
> -		return I_BDEV(inode)->bd_bdi;
> +		return I_BDEV(inode)->bd_disk->bdi;
>  #endif
>  	return sb->s_bdi;
>  }
> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
> index 290f9061b29a..a6c015cedaf7 100644
> --- a/include/linux/blk_types.h
> +++ b/include/linux/blk_types.h
> @@ -41,7 +41,6 @@ struct block_device {
>  	u8			bd_partno;
>  	spinlock_t		bd_size_lock; /* for bd_inode->i_size updates */
>  	struct gendisk *	bd_disk;
> -	struct backing_dev_info *bd_bdi;
>  
>  	/* The counter of freeze processes */
>  	int			bd_fsfreeze_count;
> -- 
> 2.30.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: move the bdi from the request_queue to the gendisk

[Jens Axboe]

On 8/9/21 8:17 AM, Christoph Hellwig wrote:
> Hi Jens,
> 
> this series moves the pointer to the bdi from the request_queue
> to the bdi, better matching the life time rules of the different
> objects.

Applied for 5.15, thanks.

-- 
Jens Axboe

Re: move the bdi from the request_queue to the gendisk

[Jens Axboe]

On 8/9/21 8:17 AM, Christoph Hellwig wrote:
> Hi Jens,
> 
> this series moves the pointer to the bdi from the request_queue
> to the bdi, better matching the life time rules of the different
> objects.

Applied for 5.15, thanks.

-- 
Jens Axboe

Re: move the bdi from the request_queue to the gendisk

[Qian Cai]

On 8/9/2021 10:17 AM, Christoph Hellwig wrote:
> Hi Jens,
> 
> this series moves the pointer to the bdi from the request_queue
> to the bdi, better matching the life time rules of the different
> objects.

Reverting this series fixed an use-after-free in bdev_evict_inode().

[ 3710.755078][    T1] BUG: KASAN: use-after-free in bdev_evict_inode+0x454/0x4d0
wb_put_many at /root/linux-next/./include/linux/backing-dev-defs.h:250
(inlined by) wb_put at /root/linux-next/./include/linux/backing-dev-defs.h:268
(inlined by) inode_detach_wb at /root/linux-next/./include/linux/writeback.h:251
(inlined by) bdev_evict_inode at /root/linux-next/fs/block_dev.c:832
[ 3710.762312][    T1] Read of size 8 at addr ffff000859ff6060 by task shutdown/1
[ 3710.769533][    T1] 
[ 3710.771721][    T1] CPU: 29 PID: 1 Comm: shutdown Not tainted 5.14.0-rc5-next-20210810+ #88
[ 3710.780073][    T1] Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 1.6 06/28/2020
[ 3710.788600][    T1] Call trace:
[ 3710.791741][    T1]  dump_backtrace+0x0/0x3b8
[ 3710.796103][    T1]  show_stack+0x20/0x30
[ 3710.800115][    T1]  dump_stack_lvl+0x8c/0xb8
[ 3710.804472][    T1]  print_address_description.constprop.0+0x74/0x3c8
[ 3710.810913][    T1]  kasan_report+0x1f0/0x208
[ 3710.815270][    T1]  __asan_report_load8_noabort+0x34/0x60
[ 3710.820755][    T1]  bdev_evict_inode+0x454/0x4d0
[ 3710.825459][    T1]  evict+0x20c/0x400
evict at /root/linux-next/fs/inode.c:595
[ 3710.829208][    T1]  iput.part.0+0x53c/0x7a8
[ 3710.833477][    T1]  iput+0x48/0x68
[ 3710.836964][    T1]  disk_release+0x168/0x1d8
[ 3710.841322][    T1]  device_release+0xec/0x1f0
[ 3710.845766][    T1]  kobject_release+0xe4/0x360
[ 3710.850299][    T1]  kobject_put+0x7c/0x138
[ 3710.854481][    T1]  put_device+0x1c/0x30
[ 3710.858489][    T1]  blk_cleanup_disk+0x64/0x88
[ 3710.863021][    T1]  cleanup_mapped_device+0x128/0x1e8 [dm_mod]
[ 3710.868974][    T1]  __dm_destroy+0x314/0x618 [dm_mod]
[ 3710.874140][    T1]  dm_destroy+0x1c/0x28 [dm_mod]
[ 3710.878955][    T1]  dev_remove+0x214/0x2f8 [dm_mod]
[ 3710.883947][    T1]  ctl_ioctl+0x490/0xb58 [dm_mod]
[ 3710.888850][    T1]  dm_ctl_ioctl+0x18/0x28 [dm_mod]
[ 3710.893842][    T1]  __arm64_sys_ioctl+0x114/0x180
[ 3710.898636][    T1]  invoke_syscall.constprop.0+0xdc/0x1d8
[ 3710.904123][    T1]  do_el0_svc+0xe4/0x2a8
[ 3710.908219][    T1]  el0_svc+0x64/0x130
[ 3710.912057][    T1]  el0t_64_sync_handler+0xb0/0xb8
[ 3710.916934][    T1]  el0t_64_sync+0x180/0x184
[ 3711.007417][    T1] 
[ 3711.009600][    T1] Freed by task 1:
[ 3711.013172][    T1]  kasan_save_stack+0x28/0x58
[ 3711.017702][    T1]  kasan_set_track+0x28/0x40
[ 3711.022144][    T1]  kasan_set_free_info+0x28/0x50
[ 3711.026933][    T1]  __kasan_slab_free+0xfc/0x150
[ 3711.031636][    T1]  slab_free_freelist_hook+0x108/0x208
[ 3711.036947][    T1]  kfree+0x154/0x3c8
[ 3711.040695][    T1]  release_bdi+0x80/0xc0
[ 3711.044790][    T1]  bdi_put+0x54/0xb0
[ 3711.048537][    T1]  disk_release+0x70/0x1d8
[ 3711.052807][    T1]  device_release+0xec/0x1f0
[ 3711.057251][    T1]  kobject_release+0xe4/0x360
[ 3711.061782][    T1]  kobject_put+0x7c/0x138
[ 3711.065964][    T1]  put_device+0x1c/0x30
[ 3711.069974][    T1]  blk_cleanup_disk+0x64/0x88
blk_cleanup_disk at /root/linux-next/block/genhd.c:1355
[ 3711.074503][    T1]  cleanup_mapped_device+0x128/0x1e8 [dm_mod]
[ 3711.080451][    T1]  __dm_destroy+0x314/0x618 [dm_mod]
[ 3711.085617][    T1]  dm_destroy+0x1c/0x28 [dm_mod]
[ 3711.090434][    T1]  dev_remove+0x214/0x2f8 [dm_mod]
[ 3711.095424][    T1]  ctl_ioctl+0x490/0xb58 [dm_mod]
[ 3711.100328][    T1]  dm_ctl_ioctl+0x18/0x28 [dm_mod]
[ 3711.105317][    T1]  __arm64_sys_ioctl+0x114/0x180
[ 3711.110108][    T1]  invoke_syscall.constprop.0+0xdc/0x1d8
[ 3711.115594][    T1]  do_el0_svc+0xe4/0x2a8
[ 3711.119691][    T1]  el0_svc+0x64/0x130
[ 3711.123527][    T1]  el0t_64_sync_handler+0xb0/0xb8
[ 3711.128402][    T1]  el0t_64_sync+0x180/0x184
[ 3711.132759][    T1] 
[ 3711.134941][    T1] Last potentially related work creation:
[ 3711.140511][    T1]  kasan_save_stack+0x28/0x58
[ 3711.145041][    T1]  kasan_record_aux_stack+0xf4/0x128
[ 3711.150179][    T1]  insert_work+0x58/0x2c0
[ 3711.154361][    T1]  __queue_work+0x644/0x18d0
[ 3711.158802][    T1]  __queue_delayed_work+0x14c/0x228
[ 3711.163853][    T1]  mod_delayed_work_on+0xc0/0x128
[ 3711.168729][    T1]  wb_shutdown+0x174/0x230
[ 3711.172999][    T1]  bdi_unregister+0x158/0x480
[ 3711.177527][    T1]  del_gendisk+0x410/0x548
[ 3711.181797][    T1]  cleanup_mapped_device+0x190/0x1e8 [dm_mod]
[ 3711.187745][    T1]  __dm_destroy+0x314/0x618 [dm_mod]
[ 3711.192909][    T1]  dm_destroy+0x1c/0x28 [dm_mod]
[ 3711.197725][    T1]  dev_remove+0x214/0x2f8 [dm_mod]
[ 3711.202716][    T1]  ctl_ioctl+0x490/0xb58 [dm_mod]
[ 3711.207620][    T1]  dm_ctl_ioctl+0x18/0x28 [dm_mod]
[ 3711.212611][    T1]  __arm64_sys_ioctl+0x114/0x180
[ 3711.217402][    T1]  invoke_syscall.constprop.0+0xdc/0x1d8
[ 3711.222889][    T1]  do_el0_svc+0xe4/0x2a8
[ 3711.226985][    T1]  el0_svc+0x64/0x130
[ 3711.230823][    T1]  el0t_64_sync_handler+0xb0/0xb8
[ 3711.235699][    T1]  el0t_64_sync+0x180/0x184
[ 3711.240055][    T1] 
[ 3711.242237][    T1] Second to last potentially related work creation:
[ 3711.248673][    T1]  kasan_save_stack+0x28/0x58
[ 3711.253203][    T1]  kasan_record_aux_stack+0xf4/0x128
[ 3711.258340][    T1]  insert_work+0x58/0x2c0
[ 3711.262522][    T1]  __queue_work+0x644/0x18d0
[ 3711.266964][    T1]  delayed_work_timer_fn+0x6c/0xa0
[ 3711.271927][    T1]  call_timer_fn+0x224/0xbb0
[ 3711.276371][    T1]  __run_timers.part.0+0x548/0xb58
[ 3711.281336][    T1]  run_timer_softirq+0x80/0x118
[ 3711.286039][    T1]  _stext+0x2d4/0x11ac
[ 3711.289961][    T1] 
[ 3711.292142][    T1] The buggy address belongs to the object at ffff000859ff6000
[ 3711.292142][    T1]  which belongs to the cache kmalloc-4k of size 4096
[ 3711.306045][    T1] The buggy address is located 96 bytes inside of
[ 3711.306045][    T1]  4096-byte region [ffff000859ff6000, ffff000859ff7000)
[ 3711.319169][    T1] The buggy address belongs to the page:
[ 3711.324653][    T1] page:ffffffc002167e00 refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff000859fd2000 pfn:0x8d9f8
[ 3711.335954][    T1] head:ffffffc002167e00 order:3 compound_mapcount:0 compound_pincount:0
[ 3711.344129][    T1] flags: 0x7ffff800010200(slab|head|node=0|zone=0|lastcpupid=0xfffff)
[ 3711.352137][    T1] raw: 007ffff800010200 ffffffc002168208 ffffffc002140e08 ffff000012911580
[ 3711.360574][    T1] raw: ffff000859fd2000 00000000002a0002 00000001ffffffff 0000000000000000
[ 3711.369008][    T1] page dumped because: kasan: bad access detected
[ 3711.375272][    T1] 
[ 3711.377454][    T1] Memory state around the buggy address:
[ 3711.382936][    T1]  ffff000859ff5f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 3711.390848][    T1]  ffff000859ff5f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 3711.398762][    T1] >ffff000859ff6000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 3711.406674][    T1]                                                        ^
[ 3711.413719][    T1]  ffff000859ff6080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 3711.421632][    T1]  ffff000859ff6100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb

> 
> Diffstat:
>  block/bfq-iosched.c           |    4 ++--
>  block/blk-cgroup.c            |    7 +++----
>  block/blk-core.c              |   18 +++---------------
>  block/blk-mq.c                |    2 +-
>  block/blk-settings.c          |   22 ++++++++++++++--------
>  block/blk-sysfs.c             |   28 +++++++++++++---------------
>  block/blk-wbt.c               |   10 +++++-----
>  block/genhd.c                 |   23 ++++++++++++++---------
>  block/ioctl.c                 |    7 ++++---
>  drivers/block/drbd/drbd_nl.c  |    2 +-
>  drivers/block/drbd/drbd_req.c |    5 ++---
>  drivers/block/pktcdvd.c       |    8 +++-----
>  drivers/md/dm-table.c         |    2 +-
>  drivers/nvme/host/core.c      |    2 +-
>  fs/block_dev.c                |   13 +------------
>  fs/fat/fatent.c               |    1 +
>  fs/nilfs2/super.c             |    2 +-
>  fs/super.c                    |    2 +-
>  fs/xfs/xfs_buf.c              |    2 +-
>  include/linux/backing-dev.h   |    2 +-
>  include/linux/blk_types.h     |    1 -
>  include/linux/blkdev.h        |    6 ++----
>  include/linux/genhd.h         |    1 +
>  mm/backing-dev.c              |    3 +++
>  mm/page-writeback.c           |    2 --
>  25 files changed, 79 insertions(+), 96 deletions(-)
> 

Re: move the bdi from the request_queue to the gendisk

[Qian Cai]

On 8/9/2021 10:17 AM, Christoph Hellwig wrote:
> Hi Jens,
> 
> this series moves the pointer to the bdi from the request_queue
> to the bdi, better matching the life time rules of the different
> objects.

Reverting this series fixed an use-after-free in bdev_evict_inode().

[ 3710.755078][    T1] BUG: KASAN: use-after-free in bdev_evict_inode+0x454/0x4d0
wb_put_many at /root/linux-next/./include/linux/backing-dev-defs.h:250
(inlined by) wb_put at /root/linux-next/./include/linux/backing-dev-defs.h:268
(inlined by) inode_detach_wb at /root/linux-next/./include/linux/writeback.h:251
(inlined by) bdev_evict_inode at /root/linux-next/fs/block_dev.c:832
[ 3710.762312][    T1] Read of size 8 at addr ffff000859ff6060 by task shutdown/1
[ 3710.769533][    T1] 
[ 3710.771721][    T1] CPU: 29 PID: 1 Comm: shutdown Not tainted 5.14.0-rc5-next-20210810+ #88
[ 3710.780073][    T1] Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 1.6 06/28/2020
[ 3710.788600][    T1] Call trace:
[ 3710.791741][    T1]  dump_backtrace+0x0/0x3b8
[ 3710.796103][    T1]  show_stack+0x20/0x30
[ 3710.800115][    T1]  dump_stack_lvl+0x8c/0xb8
[ 3710.804472][    T1]  print_address_description.constprop.0+0x74/0x3c8
[ 3710.810913][    T1]  kasan_report+0x1f0/0x208
[ 3710.815270][    T1]  __asan_report_load8_noabort+0x34/0x60
[ 3710.820755][    T1]  bdev_evict_inode+0x454/0x4d0
[ 3710.825459][    T1]  evict+0x20c/0x400
evict at /root/linux-next/fs/inode.c:595
[ 3710.829208][    T1]  iput.part.0+0x53c/0x7a8
[ 3710.833477][    T1]  iput+0x48/0x68
[ 3710.836964][    T1]  disk_release+0x168/0x1d8
[ 3710.841322][    T1]  device_release+0xec/0x1f0
[ 3710.845766][    T1]  kobject_release+0xe4/0x360
[ 3710.850299][    T1]  kobject_put+0x7c/0x138
[ 3710.854481][    T1]  put_device+0x1c/0x30
[ 3710.858489][    T1]  blk_cleanup_disk+0x64/0x88
[ 3710.863021][    T1]  cleanup_mapped_device+0x128/0x1e8 [dm_mod]
[ 3710.868974][    T1]  __dm_destroy+0x314/0x618 [dm_mod]
[ 3710.874140][    T1]  dm_destroy+0x1c/0x28 [dm_mod]
[ 3710.878955][    T1]  dev_remove+0x214/0x2f8 [dm_mod]
[ 3710.883947][    T1]  ctl_ioctl+0x490/0xb58 [dm_mod]
[ 3710.888850][    T1]  dm_ctl_ioctl+0x18/0x28 [dm_mod]
[ 3710.893842][    T1]  __arm64_sys_ioctl+0x114/0x180
[ 3710.898636][    T1]  invoke_syscall.constprop.0+0xdc/0x1d8
[ 3710.904123][    T1]  do_el0_svc+0xe4/0x2a8
[ 3710.908219][    T1]  el0_svc+0x64/0x130
[ 3710.912057][    T1]  el0t_64_sync_handler+0xb0/0xb8
[ 3710.916934][    T1]  el0t_64_sync+0x180/0x184
[ 3711.007417][    T1] 
[ 3711.009600][    T1] Freed by task 1:
[ 3711.013172][    T1]  kasan_save_stack+0x28/0x58
[ 3711.017702][    T1]  kasan_set_track+0x28/0x40
[ 3711.022144][    T1]  kasan_set_free_info+0x28/0x50
[ 3711.026933][    T1]  __kasan_slab_free+0xfc/0x150
[ 3711.031636][    T1]  slab_free_freelist_hook+0x108/0x208
[ 3711.036947][    T1]  kfree+0x154/0x3c8
[ 3711.040695][    T1]  release_bdi+0x80/0xc0
[ 3711.044790][    T1]  bdi_put+0x54/0xb0
[ 3711.048537][    T1]  disk_release+0x70/0x1d8
[ 3711.052807][    T1]  device_release+0xec/0x1f0
[ 3711.057251][    T1]  kobject_release+0xe4/0x360
[ 3711.061782][    T1]  kobject_put+0x7c/0x138
[ 3711.065964][    T1]  put_device+0x1c/0x30
[ 3711.069974][    T1]  blk_cleanup_disk+0x64/0x88
blk_cleanup_disk at /root/linux-next/block/genhd.c:1355
[ 3711.074503][    T1]  cleanup_mapped_device+0x128/0x1e8 [dm_mod]
[ 3711.080451][    T1]  __dm_destroy+0x314/0x618 [dm_mod]
[ 3711.085617][    T1]  dm_destroy+0x1c/0x28 [dm_mod]
[ 3711.090434][    T1]  dev_remove+0x214/0x2f8 [dm_mod]
[ 3711.095424][    T1]  ctl_ioctl+0x490/0xb58 [dm_mod]
[ 3711.100328][    T1]  dm_ctl_ioctl+0x18/0x28 [dm_mod]
[ 3711.105317][    T1]  __arm64_sys_ioctl+0x114/0x180
[ 3711.110108][    T1]  invoke_syscall.constprop.0+0xdc/0x1d8
[ 3711.115594][    T1]  do_el0_svc+0xe4/0x2a8
[ 3711.119691][    T1]  el0_svc+0x64/0x130
[ 3711.123527][    T1]  el0t_64_sync_handler+0xb0/0xb8
[ 3711.128402][    T1]  el0t_64_sync+0x180/0x184
[ 3711.132759][    T1] 
[ 3711.134941][    T1] Last potentially related work creation:
[ 3711.140511][    T1]  kasan_save_stack+0x28/0x58
[ 3711.145041][    T1]  kasan_record_aux_stack+0xf4/0x128
[ 3711.150179][    T1]  insert_work+0x58/0x2c0
[ 3711.154361][    T1]  __queue_work+0x644/0x18d0
[ 3711.158802][    T1]  __queue_delayed_work+0x14c/0x228
[ 3711.163853][    T1]  mod_delayed_work_on+0xc0/0x128
[ 3711.168729][    T1]  wb_shutdown+0x174/0x230
[ 3711.172999][    T1]  bdi_unregister+0x158/0x480
[ 3711.177527][    T1]  del_gendisk+0x410/0x548
[ 3711.181797][    T1]  cleanup_mapped_device+0x190/0x1e8 [dm_mod]
[ 3711.187745][    T1]  __dm_destroy+0x314/0x618 [dm_mod]
[ 3711.192909][    T1]  dm_destroy+0x1c/0x28 [dm_mod]
[ 3711.197725][    T1]  dev_remove+0x214/0x2f8 [dm_mod]
[ 3711.202716][    T1]  ctl_ioctl+0x490/0xb58 [dm_mod]
[ 3711.207620][    T1]  dm_ctl_ioctl+0x18/0x28 [dm_mod]
[ 3711.212611][    T1]  __arm64_sys_ioctl+0x114/0x180
[ 3711.217402][    T1]  invoke_syscall.constprop.0+0xdc/0x1d8
[ 3711.222889][    T1]  do_el0_svc+0xe4/0x2a8
[ 3711.226985][    T1]  el0_svc+0x64/0x130
[ 3711.230823][    T1]  el0t_64_sync_handler+0xb0/0xb8
[ 3711.235699][    T1]  el0t_64_sync+0x180/0x184
[ 3711.240055][    T1] 
[ 3711.242237][    T1] Second to last potentially related work creation:
[ 3711.248673][    T1]  kasan_save_stack+0x28/0x58
[ 3711.253203][    T1]  kasan_record_aux_stack+0xf4/0x128
[ 3711.258340][    T1]  insert_work+0x58/0x2c0
[ 3711.262522][    T1]  __queue_work+0x644/0x18d0
[ 3711.266964][    T1]  delayed_work_timer_fn+0x6c/0xa0
[ 3711.271927][    T1]  call_timer_fn+0x224/0xbb0
[ 3711.276371][    T1]  __run_timers.part.0+0x548/0xb58
[ 3711.281336][    T1]  run_timer_softirq+0x80/0x118
[ 3711.286039][    T1]  _stext+0x2d4/0x11ac
[ 3711.289961][    T1] 
[ 3711.292142][    T1] The buggy address belongs to the object at ffff000859ff6000
[ 3711.292142][    T1]  which belongs to the cache kmalloc-4k of size 4096
[ 3711.306045][    T1] The buggy address is located 96 bytes inside of
[ 3711.306045][    T1]  4096-byte region [ffff000859ff6000, ffff000859ff7000)
[ 3711.319169][    T1] The buggy address belongs to the page:
[ 3711.324653][    T1] page:ffffffc002167e00 refcount:1 mapcount:0 mapping:0000000000000000 index:0xffff000859fd2000 pfn:0x8d9f8
[ 3711.335954][    T1] head:ffffffc002167e00 order:3 compound_mapcount:0 compound_pincount:0
[ 3711.344129][    T1] flags: 0x7ffff800010200(slab|head|node=0|zone=0|lastcpupid=0xfffff)
[ 3711.352137][    T1] raw: 007ffff800010200 ffffffc002168208 ffffffc002140e08 ffff000012911580
[ 3711.360574][    T1] raw: ffff000859fd2000 00000000002a0002 00000001ffffffff 0000000000000000
[ 3711.369008][    T1] page dumped because: kasan: bad access detected
[ 3711.375272][    T1] 
[ 3711.377454][    T1] Memory state around the buggy address:
[ 3711.382936][    T1]  ffff000859ff5f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 3711.390848][    T1]  ffff000859ff5f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 3711.398762][    T1] >ffff000859ff6000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 3711.406674][    T1]                                                        ^
[ 3711.413719][    T1]  ffff000859ff6080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 3711.421632][    T1]  ffff000859ff6100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb

> 
> Diffstat:
>  block/bfq-iosched.c           |    4 ++--
>  block/blk-cgroup.c            |    7 +++----
>  block/blk-core.c              |   18 +++---------------
>  block/blk-mq.c                |    2 +-
>  block/blk-settings.c          |   22 ++++++++++++++--------
>  block/blk-sysfs.c             |   28 +++++++++++++---------------
>  block/blk-wbt.c               |   10 +++++-----
>  block/genhd.c                 |   23 ++++++++++++++---------
>  block/ioctl.c                 |    7 ++++---
>  drivers/block/drbd/drbd_nl.c  |    2 +-
>  drivers/block/drbd/drbd_req.c |    5 ++---
>  drivers/block/pktcdvd.c       |    8 +++-----
>  drivers/md/dm-table.c         |    2 +-
>  drivers/nvme/host/core.c      |    2 +-
>  fs/block_dev.c                |   13 +------------
>  fs/fat/fatent.c               |    1 +
>  fs/nilfs2/super.c             |    2 +-
>  fs/super.c                    |    2 +-
>  fs/xfs/xfs_buf.c              |    2 +-
>  include/linux/backing-dev.h   |    2 +-
>  include/linux/blk_types.h     |    1 -
>  include/linux/blkdev.h        |    6 ++----
>  include/linux/genhd.h         |    1 +
>  mm/backing-dev.c              |    3 +++
>  mm/page-writeback.c           |    2 --
>  25 files changed, 79 insertions(+), 96 deletions(-)
> 

Re: move the bdi from the request_queue to the gendisk

[Christoph Hellwig]

On Tue, Aug 10, 2021 at 03:36:39PM -0400, Qian Cai wrote:
> 
> 
> On 8/9/2021 10:17 AM, Christoph Hellwig wrote:
> > Hi Jens,
> > 
> > this series moves the pointer to the bdi from the request_queue
> > to the bdi, better matching the life time rules of the different
> > objects.
> 
> Reverting this series fixed an use-after-free in bdev_evict_inode().

Please try the patch below as a band-aid.  Although the proper fix is
that non-default bdi_writeback structures grab a reference to the bdi,
as this was a landmine that might have already caused spurious issues
before.

diff --git a/block/genhd.c b/block/genhd.c
index f8def1129501..2e4a9d187196 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1086,7 +1086,6 @@ static void disk_release(struct device *dev)
 
 	might_sleep();
 
-	bdi_put(disk->bdi);
 	if (MAJOR(dev->devt) == BLOCK_EXT_MAJOR)
 		blk_free_ext_minor(MINOR(dev->devt));
 	disk_release_events(disk);
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 7c969f81327a..c6087dbae6cf 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -849,11 +849,15 @@ static void init_once(void *data)
 
 static void bdev_evict_inode(struct inode *inode)
 {
+	struct block_device *bdev = I_BDEV(inode);
+
 	truncate_inode_pages_final(&inode->i_data);
 	invalidate_inode_buffers(inode); /* is it needed here? */
 	clear_inode(inode);
 	/* Detach inode from wb early as bdi_put() may free bdi->wb */
 	inode_detach_wb(inode);
+	if (!bdev_is_partition(bdev))
+		bdi_put(bdev->bd_disk->bdi);
 }
 
 static const struct super_operations bdev_sops = {

Re: move the bdi from the request_queue to the gendisk

[Christoph Hellwig]

On Tue, Aug 10, 2021 at 03:36:39PM -0400, Qian Cai wrote:
> 
> 
> On 8/9/2021 10:17 AM, Christoph Hellwig wrote:
> > Hi Jens,
> > 
> > this series moves the pointer to the bdi from the request_queue
> > to the bdi, better matching the life time rules of the different
> > objects.
> 
> Reverting this series fixed an use-after-free in bdev_evict_inode().

Please try the patch below as a band-aid.  Although the proper fix is
that non-default bdi_writeback structures grab a reference to the bdi,
as this was a landmine that might have already caused spurious issues
before.

diff --git a/block/genhd.c b/block/genhd.c
index f8def1129501..2e4a9d187196 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -1086,7 +1086,6 @@ static void disk_release(struct device *dev)
 
 	might_sleep();
 
-	bdi_put(disk->bdi);
 	if (MAJOR(dev->devt) == BLOCK_EXT_MAJOR)
 		blk_free_ext_minor(MINOR(dev->devt));
 	disk_release_events(disk);
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 7c969f81327a..c6087dbae6cf 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -849,11 +849,15 @@ static void init_once(void *data)
 
 static void bdev_evict_inode(struct inode *inode)
 {
+	struct block_device *bdev = I_BDEV(inode);
+
 	truncate_inode_pages_final(&inode->i_data);
 	invalidate_inode_buffers(inode); /* is it needed here? */
 	clear_inode(inode);
 	/* Detach inode from wb early as bdi_put() may free bdi->wb */
 	inode_detach_wb(inode);
+	if (!bdev_is_partition(bdev))
+		bdi_put(bdev->bd_disk->bdi);
 }
 
 static const struct super_operations bdev_sops = {

Re: move the bdi from the request_queue to the gendisk

[Qian Cai]

On 8/10/2021 4:02 PM, Christoph Hellwig wrote:
> On Tue, Aug 10, 2021 at 03:36:39PM -0400, Qian Cai wrote:
>>
>>
>> On 8/9/2021 10:17 AM, Christoph Hellwig wrote:
>>> Hi Jens,
>>>
>>> this series moves the pointer to the bdi from the request_queue
>>> to the bdi, better matching the life time rules of the different
>>> objects.
>>
>> Reverting this series fixed an use-after-free in bdev_evict_inode().
> 
> Please try the patch below as a band-aid.  Although the proper fix is
> that non-default bdi_writeback structures grab a reference to the bdi,
> as this was a landmine that might have already caused spurious issues
> before.

This works fine with a quick test.

Re: move the bdi from the request_queue to the gendisk

[Qian Cai]

On 8/10/2021 4:02 PM, Christoph Hellwig wrote:
> On Tue, Aug 10, 2021 at 03:36:39PM -0400, Qian Cai wrote:
>>
>>
>> On 8/9/2021 10:17 AM, Christoph Hellwig wrote:
>>> Hi Jens,
>>>
>>> this series moves the pointer to the bdi from the request_queue
>>> to the bdi, better matching the life time rules of the different
>>> objects.
>>
>> Reverting this series fixed an use-after-free in bdev_evict_inode().
> 
> Please try the patch below as a band-aid.  Although the proper fix is
> that non-default bdi_writeback structures grab a reference to the bdi,
> as this was a landmine that might have already caused spurious issues
> before.

This works fine with a quick test.

Re: move the bdi from the request_queue to the gendisk

[Jan Kara]

On Tue 10-08-21 22:02:56, Christoph Hellwig wrote:
> On Tue, Aug 10, 2021 at 03:36:39PM -0400, Qian Cai wrote:
> > 
> > 
> > On 8/9/2021 10:17 AM, Christoph Hellwig wrote:
> > > Hi Jens,
> > > 
> > > this series moves the pointer to the bdi from the request_queue
> > > to the bdi, better matching the life time rules of the different
> > > objects.
> > 
> > Reverting this series fixed an use-after-free in bdev_evict_inode().
> 
> Please try the patch below as a band-aid.  Although the proper fix is
> that non-default bdi_writeback structures grab a reference to the bdi,
> as this was a landmine that might have already caused spurious issues
> before.

Well, non-default bdi_writeback structures do hold bdi reference - see
wb_exit() which drops the reference. I think the problem rather was that a
block device's inode->i_wb was pointing to the default bdi_writeback
structure and that got freed after bdi_put() before block device inode was
shutdown through bdput()... So what I think we need is that if the inode
references the default writeback structure, it actually holds a reference
to the bdi.

								Honza
> 
> diff --git a/block/genhd.c b/block/genhd.c
> index f8def1129501..2e4a9d187196 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -1086,7 +1086,6 @@ static void disk_release(struct device *dev)
>  
>  	might_sleep();
>  
> -	bdi_put(disk->bdi);
>  	if (MAJOR(dev->devt) == BLOCK_EXT_MAJOR)
>  		blk_free_ext_minor(MINOR(dev->devt));
>  	disk_release_events(disk);
> diff --git a/fs/block_dev.c b/fs/block_dev.c
> index 7c969f81327a..c6087dbae6cf 100644
> --- a/fs/block_dev.c
> +++ b/fs/block_dev.c
> @@ -849,11 +849,15 @@ static void init_once(void *data)
>  
>  static void bdev_evict_inode(struct inode *inode)
>  {
> +	struct block_device *bdev = I_BDEV(inode);
> +
>  	truncate_inode_pages_final(&inode->i_data);
>  	invalidate_inode_buffers(inode); /* is it needed here? */
>  	clear_inode(inode);
>  	/* Detach inode from wb early as bdi_put() may free bdi->wb */
>  	inode_detach_wb(inode);
> +	if (!bdev_is_partition(bdev))
> +		bdi_put(bdev->bd_disk->bdi);
>  }
>  
>  static const struct super_operations bdev_sops = {
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: move the bdi from the request_queue to the gendisk

[Christoph Hellwig]

On Wed, Aug 11, 2021 at 01:25:14PM +0200, Jan Kara wrote:
> Well, non-default bdi_writeback structures do hold bdi reference - see
> wb_exit() which drops the reference. I think the problem rather was that a
> block device's inode->i_wb was pointing to the default bdi_writeback
> structure and that got freed after bdi_put() before block device inode was
> shutdown through bdput()... So what I think we need is that if the inode
> references the default writeback structure, it actually holds a reference
> to the bdi.

Qian, can you test the patch below instead of the one I sent yesterday?

diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index cd06dca232c3..edfb7ce2cc93 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -283,8 +283,7 @@ static int wb_init(struct bdi_writeback *wb, struct backing_dev_info *bdi,
 
 	memset(wb, 0, sizeof(*wb));
 
-	if (wb != &bdi->wb)
-		bdi_get(bdi);
+	bdi_get(bdi);
 	wb->bdi = bdi;
 	wb->last_old_flush = jiffies;
 	INIT_LIST_HEAD(&wb->b_dirty);
@@ -362,8 +361,7 @@ static void wb_exit(struct bdi_writeback *wb)
 		percpu_counter_destroy(&wb->stat[i]);
 
 	fprop_local_destroy_percpu(&wb->completions);
-	if (wb != &wb->bdi->wb)
-		bdi_put(wb->bdi);
+	bdi_put(wb->bdi);
 }
 
 #ifdef CONFIG_CGROUP_WRITEBACK

Re: move the bdi from the request_queue to the gendisk

[Christoph Hellwig]

On Wed, Aug 11, 2021 at 01:25:14PM +0200, Jan Kara wrote:
> Well, non-default bdi_writeback structures do hold bdi reference - see
> wb_exit() which drops the reference. I think the problem rather was that a
> block device's inode->i_wb was pointing to the default bdi_writeback
> structure and that got freed after bdi_put() before block device inode was
> shutdown through bdput()... So what I think we need is that if the inode
> references the default writeback structure, it actually holds a reference
> to the bdi.

Qian, can you test the patch below instead of the one I sent yesterday?

diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index cd06dca232c3..edfb7ce2cc93 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -283,8 +283,7 @@ static int wb_init(struct bdi_writeback *wb, struct backing_dev_info *bdi,
 
 	memset(wb, 0, sizeof(*wb));
 
-	if (wb != &bdi->wb)
-		bdi_get(bdi);
+	bdi_get(bdi);
 	wb->bdi = bdi;
 	wb->last_old_flush = jiffies;
 	INIT_LIST_HEAD(&wb->b_dirty);
@@ -362,8 +361,7 @@ static void wb_exit(struct bdi_writeback *wb)
 		percpu_counter_destroy(&wb->stat[i]);
 
 	fprop_local_destroy_percpu(&wb->completions);
-	if (wb != &wb->bdi->wb)
-		bdi_put(wb->bdi);
+	bdi_put(wb->bdi);
 }
 
 #ifdef CONFIG_CGROUP_WRITEBACK

Re: move the bdi from the request_queue to the gendisk

[Jan Kara]

On Wed 11-08-21 13:51:47, Christoph Hellwig wrote:
> On Wed, Aug 11, 2021 at 01:25:14PM +0200, Jan Kara wrote:
> > Well, non-default bdi_writeback structures do hold bdi reference - see
> > wb_exit() which drops the reference. I think the problem rather was that a
> > block device's inode->i_wb was pointing to the default bdi_writeback
> > structure and that got freed after bdi_put() before block device inode was
> > shutdown through bdput()... So what I think we need is that if the inode
> > references the default writeback structure, it actually holds a reference
> > to the bdi.
> 
> Qian, can you test the patch below instead of the one I sent yesterday?

Sadly the patch below will not work because the bdi refcount will never
drop to 0. wb_exit() for the default writeback structure is called only
from release_bdi() (i.e., after the bdi refcount is 0). That is why I wrote
above that references to default wb from inodes would hold the ref, not the
default wb structure itself. So we would need to explicitely hack this into
__inode_attach_wb() and inode_detach_wb().

Somewhat cleaner approach might be to modify wb_get(), wb_tryget(),
wb_put() to get reference to bdi instead of doing nothing for the default
wb. And drop a lot of special-casing of the default wb from various
functions. But I guess the special cases are there to avoid the performance
overhead for the common case because getting wb ref is common. Also that's
why wbs use percpu refcount and we would need something similar for bdis.
I guess this needs more thinking and your quick workaround is OK for now.

								Honza

> 
> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> index cd06dca232c3..edfb7ce2cc93 100644
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -283,8 +283,7 @@ static int wb_init(struct bdi_writeback *wb, struct backing_dev_info *bdi,
>  
>  	memset(wb, 0, sizeof(*wb));
>  
> -	if (wb != &bdi->wb)
> -		bdi_get(bdi);
> +	bdi_get(bdi);
>  	wb->bdi = bdi;
>  	wb->last_old_flush = jiffies;
>  	INIT_LIST_HEAD(&wb->b_dirty);
> @@ -362,8 +361,7 @@ static void wb_exit(struct bdi_writeback *wb)
>  		percpu_counter_destroy(&wb->stat[i]);
>  
>  	fprop_local_destroy_percpu(&wb->completions);
> -	if (wb != &wb->bdi->wb)
> -		bdi_put(wb->bdi);
> +	bdi_put(wb->bdi);
>  }
>  
>  #ifdef CONFIG_CGROUP_WRITEBACK
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR