diff mbox series

block: Optimize disk zone resource cleanup

Message ID 20240607002126.104227-1-dlemoal@kernel.org (mailing list archive)
State New, archived
Headers show
Series block: Optimize disk zone resource cleanup | expand

Commit Message

Damien Le Moal June 7, 2024, 12:21 a.m. UTC
For zoned block devices using zone write plugging, an rcu_barrier() call
is needed in disk_free_zone_resources() to synchronize freeing of zone
write plugs and the destrution of the mempool used to allocate the
plugs. The barrier call does slow down a little teardown of zoned block
devices but should not affect teardown of regular block devices or zoned
block devices that do not use zone write plugging (e.g. zoned DM devices
that do not require zone append emulation).

Modify disk_free_zone_resources() to return early if we do not have a
mempool to start with, that is, if the device does not use zone write
plugging. This avoids the costly rcu_barrier() and speeds up disk
teardown.

Reported-by: Mikulas Patocka <mpatocka@redhat.com>
Fixes: dd291d77cc90 ("block: Introduce zone write plugging")
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 block/blk-zoned.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Christoph Hellwig June 7, 2024, 4:58 a.m. UTC | #1
Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Mikulas Patocka June 7, 2024, 2:02 p.m. UTC | #2
On Fri, 7 Jun 2024, Damien Le Moal wrote:

> For zoned block devices using zone write plugging, an rcu_barrier() call
> is needed in disk_free_zone_resources() to synchronize freeing of zone
> write plugs and the destrution of the mempool used to allocate the
> plugs. The barrier call does slow down a little teardown of zoned block
> devices but should not affect teardown of regular block devices or zoned
> block devices that do not use zone write plugging (e.g. zoned DM devices
> that do not require zone append emulation).
> 
> Modify disk_free_zone_resources() to return early if we do not have a
> mempool to start with, that is, if the device does not use zone write
> plugging. This avoids the costly rcu_barrier() and speeds up disk
> teardown.
> 
> Reported-by: Mikulas Patocka <mpatocka@redhat.com>
> Fixes: dd291d77cc90 ("block: Introduce zone write plugging")
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>

This works.

Tested-by: Mikulas Patocka <mpatocka@redhat.com>


BTW. when we remove large number of DM devices, the process spends a lot 
of time sleeping with this stacktrace:

__schedule+0x242/0x600
schedule+0x21/0xd0
blk_mq_freeze_queue_wait+0x55/0x90
? __wake_up+0x50/0x50
del_gendisk+0x1fc/0x320
cleanup_mapped_device+0x16c/0x180 [dm_mod]
__dm_destroy+0x145/0x1f0 [dm_mod]
dm_hash_remove_all+0x5c/0x180 [dm_mod]
? dm_hash_remove_all+0x180/0x180 [dm_mod]
remove_all+0x1a/0x30 [dm_mod]
ctl_ioctl+0x1bb/0x500 [dm_mod]
dm_compat_ctl_ioctl+0x7/0x10 [dm_mod]
__x64_compat_sys_ioctl+0x133/0x160
do_syscall_64+0x17c/0x1a0
entry_SYSCALL_64_after_hwframe+0x4b/0x53

Is there some way how to remove this wait?

Mikulas

> ---
>  block/blk-zoned.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 8f89705f5e1c..137842dbb59a 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -1552,6 +1552,9 @@ static void disk_destroy_zone_wplugs_hash_table(struct gendisk *disk)
>  
>  void disk_free_zone_resources(struct gendisk *disk)
>  {
> +	if (!disk->zone_wplugs_pool)
> +		return;
> +
>  	cancel_work_sync(&disk->zone_wplugs_work);
>  
>  	if (disk->zone_wplugs_wq) {
> -- 
> 2.45.2
>
Niklas Cassel June 10, 2024, 6:58 a.m. UTC | #3
On Fri, Jun 07, 2024 at 09:21:26AM +0900, Damien Le Moal wrote:
> For zoned block devices using zone write plugging, an rcu_barrier() call
> is needed in disk_free_zone_resources() to synchronize freeing of zone
> write plugs and the destrution of the mempool used to allocate the
> plugs. The barrier call does slow down a little teardown of zoned block
> devices but should not affect teardown of regular block devices or zoned
> block devices that do not use zone write plugging (e.g. zoned DM devices
> that do not require zone append emulation).
> 
> Modify disk_free_zone_resources() to return early if we do not have a
> mempool to start with, that is, if the device does not use zone write
> plugging. This avoids the costly rcu_barrier() and speeds up disk
> teardown.
> 
> Reported-by: Mikulas Patocka <mpatocka@redhat.com>
> Fixes: dd291d77cc90 ("block: Introduce zone write plugging")
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
>  block/blk-zoned.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/block/blk-zoned.c b/block/blk-zoned.c
> index 8f89705f5e1c..137842dbb59a 100644
> --- a/block/blk-zoned.c
> +++ b/block/blk-zoned.c
> @@ -1552,6 +1552,9 @@ static void disk_destroy_zone_wplugs_hash_table(struct gendisk *disk)
>  
>  void disk_free_zone_resources(struct gendisk *disk)
>  {
> +	if (!disk->zone_wplugs_pool)
> +		return;
> +
>  	cancel_work_sync(&disk->zone_wplugs_work);
>  
>  	if (disk->zone_wplugs_wq) {
> -- 
> 2.45.2
> 

Reviewed-by: Niklas Cassel <cassel@kernel.org>
Jens Axboe June 12, 2024, 5 p.m. UTC | #4
On Fri, 07 Jun 2024 09:21:26 +0900, Damien Le Moal wrote:
> For zoned block devices using zone write plugging, an rcu_barrier() call
> is needed in disk_free_zone_resources() to synchronize freeing of zone
> write plugs and the destrution of the mempool used to allocate the
> plugs. The barrier call does slow down a little teardown of zoned block
> devices but should not affect teardown of regular block devices or zoned
> block devices that do not use zone write plugging (e.g. zoned DM devices
> that do not require zone append emulation).
> 
> [...]

Applied, thanks!

[1/1] block: Optimize disk zone resource cleanup
      commit: 1933192a91be0a570663a3c6310c46e4ce3b2baa

Best regards,
diff mbox series

Patch

diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index 8f89705f5e1c..137842dbb59a 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -1552,6 +1552,9 @@  static void disk_destroy_zone_wplugs_hash_table(struct gendisk *disk)
 
 void disk_free_zone_resources(struct gendisk *disk)
 {
+	if (!disk->zone_wplugs_pool)
+		return;
+
 	cancel_work_sync(&disk->zone_wplugs_work);
 
 	if (disk->zone_wplugs_wq) {