public inbox for linux-block@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] btrfs: zoned: skip reporting zone for new block group
@ 2025-03-18  0:59 Naohiro Aota
  2025-03-18  0:59 ` [PATCH v2 1/2] block: introduce zone capacity helper Naohiro Aota
  2025-03-18  0:59 ` [PATCH v2 2/2] btrfs: zoned: skip reporting zone for new block group Naohiro Aota
  0 siblings, 2 replies; 9+ messages in thread
From: Naohiro Aota @ 2025-03-18  0:59 UTC (permalink / raw)
  To: linux-btrfs; +Cc: dlemoal, axboe, linux-block, hch, Naohiro Aota

Newly created block group should reside on empty zones, whose write pointer
should always be 0. Also, we can load the zone capacity from the block
layer. So, we don't need to REPORT_ZONE to load the info.

The reporting zone on a new block group is not only unnecessary, but also
can cause a deadlock. When one process do a report zones and another
process freezes the block device, the report zones side cannot allocate
a tag because the freeze is already started. This can thus result in new
block group creation to hang forever, blocking the write path.

v1: https://patch.msgid.link/cover.1741596325.git.naohiro.aota@wdc.com

Naohiro Aota (2):
  block: introduce zone capacity helper
  btrfs: zoned: skip reporting zone for new block group

 fs/btrfs/zoned.c       | 18 ++++++++++--
 include/linux/blkdev.h | 67 ++++++++++++++++++++++++++++--------------
 2 files changed, 61 insertions(+), 24 deletions(-)

-- 
2.49.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/2] block: introduce zone capacity helper
  2025-03-18  0:59 [PATCH v2 0/2] btrfs: zoned: skip reporting zone for new block group Naohiro Aota
@ 2025-03-18  0:59 ` Naohiro Aota
  2025-03-18  5:14   ` Damien Le Moal
  2025-03-18  7:27   ` Johannes Thumshirn
  2025-03-18  0:59 ` [PATCH v2 2/2] btrfs: zoned: skip reporting zone for new block group Naohiro Aota
  1 sibling, 2 replies; 9+ messages in thread
From: Naohiro Aota @ 2025-03-18  0:59 UTC (permalink / raw)
  To: linux-btrfs; +Cc: dlemoal, axboe, linux-block, hch, Naohiro Aota

{bdev,disk}_zone_capacity() takes block_device or gendisk and sector position
and returns the zone capacity of the corresponding zone.

With that, move disk_nr_zones() and blk_zone_plug_bio() to consolidate them in
the same #ifdef block.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 include/linux/blkdev.h | 67 ++++++++++++++++++++++++++++--------------
 1 file changed, 45 insertions(+), 22 deletions(-)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index d37751789bf5..6f1bdec27ac7 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -691,23 +691,6 @@ static inline bool blk_queue_is_zoned(struct request_queue *q)
 		(q->limits.features & BLK_FEAT_ZONED);
 }
 
-#ifdef CONFIG_BLK_DEV_ZONED
-static inline unsigned int disk_nr_zones(struct gendisk *disk)
-{
-	return disk->nr_zones;
-}
-bool blk_zone_plug_bio(struct bio *bio, unsigned int nr_segs);
-#else /* CONFIG_BLK_DEV_ZONED */
-static inline unsigned int disk_nr_zones(struct gendisk *disk)
-{
-	return 0;
-}
-static inline bool blk_zone_plug_bio(struct bio *bio, unsigned int nr_segs)
-{
-	return false;
-}
-#endif /* CONFIG_BLK_DEV_ZONED */
-
 static inline unsigned int disk_zone_no(struct gendisk *disk, sector_t sector)
 {
 	if (!blk_queue_is_zoned(disk->queue))
@@ -715,11 +698,6 @@ static inline unsigned int disk_zone_no(struct gendisk *disk, sector_t sector)
 	return sector >> ilog2(disk->queue->limits.chunk_sectors);
 }
 
-static inline unsigned int bdev_nr_zones(struct block_device *bdev)
-{
-	return disk_nr_zones(bdev->bd_disk);
-}
-
 static inline unsigned int bdev_max_open_zones(struct block_device *bdev)
 {
 	return bdev->bd_disk->queue->limits.max_open_zones;
@@ -826,6 +804,51 @@ static inline u64 sb_bdev_nr_blocks(struct super_block *sb)
 		(sb->s_blocksize_bits - SECTOR_SHIFT);
 }
 
+#ifdef CONFIG_BLK_DEV_ZONED
+static inline unsigned int disk_nr_zones(struct gendisk *disk)
+{
+	return disk->nr_zones;
+}
+bool blk_zone_plug_bio(struct bio *bio, unsigned int nr_segs);
+
+/**
+ * disk_zone_capacity - returns the zone capacity of zone at @pos
+ * @disk:	disk to work with
+ * @pos:	sector number within the querying zone
+ *
+ * Returns the zone capacity of a zone at @pos. @pos can be any sector contained
+ * in the zone.
+ */
+static inline unsigned int disk_zone_capacity(struct gendisk *disk,
+					      sector_t pos)
+{
+	sector_t zone_sectors = disk->queue->limits.chunk_sectors;
+
+	if (pos + zone_sectors >= get_capacity(disk))
+		return disk->last_zone_capacity;
+	return disk->zone_capacity;
+}
+static inline unsigned int bdev_zone_capacity(struct block_device *bdev,
+					      sector_t pos)
+{
+	return disk_zone_capacity(bdev->bd_disk, pos);
+}
+#else /* CONFIG_BLK_DEV_ZONED */
+static inline unsigned int disk_nr_zones(struct gendisk *disk)
+{
+	return 0;
+}
+static inline bool blk_zone_plug_bio(struct bio *bio, unsigned int nr_segs)
+{
+	return false;
+}
+#endif /* CONFIG_BLK_DEV_ZONED */
+
+static inline unsigned int bdev_nr_zones(struct block_device *bdev)
+{
+	return disk_nr_zones(bdev->bd_disk);
+}
+
 int bdev_disk_changed(struct gendisk *disk, bool invalidate);
 
 void put_disk(struct gendisk *disk);
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/2] btrfs: zoned: skip reporting zone for new block group
  2025-03-18  0:59 [PATCH v2 0/2] btrfs: zoned: skip reporting zone for new block group Naohiro Aota
  2025-03-18  0:59 ` [PATCH v2 1/2] block: introduce zone capacity helper Naohiro Aota
@ 2025-03-18  0:59 ` Naohiro Aota
  2025-03-18  7:25   ` Johannes Thumshirn
  1 sibling, 1 reply; 9+ messages in thread
From: Naohiro Aota @ 2025-03-18  0:59 UTC (permalink / raw)
  To: linux-btrfs
  Cc: dlemoal, axboe, linux-block, hch, Naohiro Aota,
	Shin'ichiro Kawasaki

There is a potential deadlock if we do report zones in an IO context, detailed
in below lockdep report. When one process do a report zones and another process
freezes the block device, the report zones side cannot allocate a tag because
the freeze is already started. This can thus result in new block group creation
to hang forever, blocking the write path.

Thankfully, a new block group should be created on empty zones. So, reporting
the zones is not necessary and we can set the write pointer = 0 and load the
zone capacity from the block layer using bdev_zone_capacity() helper.

 ======================================================
 WARNING: possible circular locking dependency detected
 6.14.0-rc1 #252 Not tainted
 ------------------------------------------------------
 modprobe/1110 is trying to acquire lock:
 ffff888100ac83e0 ((work_completion)(&(&wb->dwork)->work)){+.+.}-{0:0}, at: __flush_work+0x38f/0xb60

 but task is already holding lock:
 ffff8881205b6f20 (&q->q_usage_counter(queue)#16){++++}-{0:0}, at: sd_remove+0x85/0x130

 which lock already depends on the new lock.

 the existing dependency chain (in reverse order) is:

 -> #3 (&q->q_usage_counter(queue)#16){++++}-{0:0}:
        blk_queue_enter+0x3d9/0x500
        blk_mq_alloc_request+0x47d/0x8e0
        scsi_execute_cmd+0x14f/0xb80
        sd_zbc_do_report_zones+0x1c1/0x470
        sd_zbc_report_zones+0x362/0xd60
        blkdev_report_zones+0x1b1/0x2e0
        btrfs_get_dev_zones+0x215/0x7e0 [btrfs]
        btrfs_load_block_group_zone_info+0x6d2/0x2c10 [btrfs]
        btrfs_make_block_group+0x36b/0x870 [btrfs]
        btrfs_create_chunk+0x147d/0x2320 [btrfs]
        btrfs_chunk_alloc+0x2ce/0xcf0 [btrfs]
        start_transaction+0xce6/0x1620 [btrfs]
        btrfs_uuid_scan_kthread+0x4ee/0x5b0 [btrfs]
        kthread+0x39d/0x750
        ret_from_fork+0x30/0x70
        ret_from_fork_asm+0x1a/0x30

 -> #2 (&fs_info->dev_replace.rwsem){++++}-{4:4}:
        down_read+0x9b/0x470
        btrfs_map_block+0x2ce/0x2ce0 [btrfs]
        btrfs_submit_chunk+0x2d4/0x16c0 [btrfs]
        btrfs_submit_bbio+0x16/0x30 [btrfs]
        btree_write_cache_pages+0xb5a/0xf90 [btrfs]
        do_writepages+0x17f/0x7b0
        __writeback_single_inode+0x114/0xb00
        writeback_sb_inodes+0x52b/0xe00
        wb_writeback+0x1a7/0x800
        wb_workfn+0x12a/0xbd0
        process_one_work+0x85a/0x1460
        worker_thread+0x5e2/0xfc0
        kthread+0x39d/0x750
        ret_from_fork+0x30/0x70
        ret_from_fork_asm+0x1a/0x30

 -> #1 (&fs_info->zoned_meta_io_lock){+.+.}-{4:4}:
        __mutex_lock+0x1aa/0x1360
        btree_write_cache_pages+0x252/0xf90 [btrfs]
        do_writepages+0x17f/0x7b0
        __writeback_single_inode+0x114/0xb00
        writeback_sb_inodes+0x52b/0xe00
        wb_writeback+0x1a7/0x800
        wb_workfn+0x12a/0xbd0
        process_one_work+0x85a/0x1460
        worker_thread+0x5e2/0xfc0
        kthread+0x39d/0x750
        ret_from_fork+0x30/0x70
        ret_from_fork_asm+0x1a/0x30

 -> #0 ((work_completion)(&(&wb->dwork)->work)){+.+.}-{0:0}:
        __lock_acquire+0x2f52/0x5ea0
        lock_acquire+0x1b1/0x540
        __flush_work+0x3ac/0xb60
        wb_shutdown+0x15b/0x1f0
        bdi_unregister+0x172/0x5b0
        del_gendisk+0x841/0xa20
        sd_remove+0x85/0x130
        device_release_driver_internal+0x368/0x520
        bus_remove_device+0x1f1/0x3f0
        device_del+0x3bd/0x9c0
        __scsi_remove_device+0x272/0x340
        scsi_forget_host+0xf7/0x170
        scsi_remove_host+0xd2/0x2a0
        sdebug_driver_remove+0x52/0x2f0 [scsi_debug]
        device_release_driver_internal+0x368/0x520
        bus_remove_device+0x1f1/0x3f0
        device_del+0x3bd/0x9c0
        device_unregister+0x13/0xa0
        sdebug_do_remove_host+0x1fb/0x290 [scsi_debug]
        scsi_debug_exit+0x17/0x70 [scsi_debug]
        __do_sys_delete_module.isra.0+0x321/0x520
        do_syscall_64+0x93/0x180
        entry_SYSCALL_64_after_hwframe+0x76/0x7e

 other info that might help us debug this:

 Chain exists of:
   (work_completion)(&(&wb->dwork)->work) --> &fs_info->dev_replace.rwsem --> &q->q_usage_counter(queue)#16

  Possible unsafe locking scenario:

        CPU0                    CPU1
        ----                    ----
   lock(&q->q_usage_counter(queue)#16);
                                lock(&fs_info->dev_replace.rwsem);
                                lock(&q->q_usage_counter(queue)#16);
   lock((work_completion)(&(&wb->dwork)->work));

  *** DEADLOCK ***

 5 locks held by modprobe/1110:
  #0: ffff88811f7bc108 (&dev->mutex){....}-{4:4}, at: device_release_driver_internal+0x8f/0x520
  #1: ffff8881022ee0e0 (&shost->scan_mutex){+.+.}-{4:4}, at: scsi_remove_host+0x20/0x2a0
  #2: ffff88811b4c4378 (&dev->mutex){....}-{4:4}, at: device_release_driver_internal+0x8f/0x520
  #3: ffff8881205b6f20 (&q->q_usage_counter(queue)#16){++++}-{0:0}, at: sd_remove+0x85/0x130
  #4: ffffffffa3284360 (rcu_read_lock){....}-{1:3}, at: __flush_work+0xda/0xb60

 stack backtrace:
 CPU: 0 UID: 0 PID: 1110 Comm: modprobe Not tainted 6.14.0-rc1 #252
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-3.fc41 04/01/2014
 Call Trace:
  <TASK>
  dump_stack_lvl+0x6a/0x90
  print_circular_bug.cold+0x1e0/0x274
  check_noncircular+0x306/0x3f0
  ? __pfx_check_noncircular+0x10/0x10
  ? mark_lock+0xf5/0x1650
  ? __pfx_check_irq_usage+0x10/0x10
  ? lockdep_lock+0xca/0x1c0
  ? __pfx_lockdep_lock+0x10/0x10
  __lock_acquire+0x2f52/0x5ea0
  ? __pfx___lock_acquire+0x10/0x10
  ? __pfx_mark_lock+0x10/0x10
  lock_acquire+0x1b1/0x540
  ? __flush_work+0x38f/0xb60
  ? __pfx_lock_acquire+0x10/0x10
  ? __pfx_lock_release+0x10/0x10
  ? mark_held_locks+0x94/0xe0
  ? __flush_work+0x38f/0xb60
  __flush_work+0x3ac/0xb60
  ? __flush_work+0x38f/0xb60
  ? __pfx_mark_lock+0x10/0x10
  ? __pfx___flush_work+0x10/0x10
  ? __pfx_wq_barrier_func+0x10/0x10
  ? __pfx___might_resched+0x10/0x10
  ? mark_held_locks+0x94/0xe0
  wb_shutdown+0x15b/0x1f0
  bdi_unregister+0x172/0x5b0
  ? __pfx_bdi_unregister+0x10/0x10
  ? up_write+0x1ba/0x510
  del_gendisk+0x841/0xa20
  ? __pfx_del_gendisk+0x10/0x10
  ? _raw_spin_unlock_irqrestore+0x35/0x60
  ? __pm_runtime_resume+0x79/0x110
  sd_remove+0x85/0x130
  device_release_driver_internal+0x368/0x520
  ? kobject_put+0x5d/0x4a0
  bus_remove_device+0x1f1/0x3f0
  device_del+0x3bd/0x9c0
  ? __pfx_device_del+0x10/0x10
  __scsi_remove_device+0x272/0x340
  scsi_forget_host+0xf7/0x170
  scsi_remove_host+0xd2/0x2a0
  sdebug_driver_remove+0x52/0x2f0 [scsi_debug]
  ? kernfs_remove_by_name_ns+0xc0/0xf0
  device_release_driver_internal+0x368/0x520
  ? kobject_put+0x5d/0x4a0
  bus_remove_device+0x1f1/0x3f0
  device_del+0x3bd/0x9c0
  ? __pfx_device_del+0x10/0x10
  ? __pfx___mutex_unlock_slowpath+0x10/0x10
  device_unregister+0x13/0xa0
  sdebug_do_remove_host+0x1fb/0x290 [scsi_debug]
  scsi_debug_exit+0x17/0x70 [scsi_debug]
  __do_sys_delete_module.isra.0+0x321/0x520
  ? __pfx___do_sys_delete_module.isra.0+0x10/0x10
  ? __pfx_slab_free_after_rcu_debug+0x10/0x10
  ? kasan_save_stack+0x2c/0x50
  ? kasan_record_aux_stack+0xa3/0xb0
  ? __call_rcu_common.constprop.0+0xc4/0xfb0
  ? kmem_cache_free+0x3a0/0x590
  ? __x64_sys_close+0x78/0xd0
  do_syscall_64+0x93/0x180
  ? lock_is_held_type+0xd5/0x130
  ? __call_rcu_common.constprop.0+0x3c0/0xfb0
  ? lockdep_hardirqs_on+0x78/0x100
  ? __call_rcu_common.constprop.0+0x3c0/0xfb0
  ? __pfx___call_rcu_common.constprop.0+0x10/0x10
  ? kmem_cache_free+0x3a0/0x590
  ? lockdep_hardirqs_on_prepare+0x16d/0x400
  ? do_syscall_64+0x9f/0x180
  ? lockdep_hardirqs_on+0x78/0x100
  ? do_syscall_64+0x9f/0x180
  ? __pfx___x64_sys_openat+0x10/0x10
  ? lockdep_hardirqs_on_prepare+0x16d/0x400
  ? do_syscall_64+0x9f/0x180
  ? lockdep_hardirqs_on+0x78/0x100
  ? do_syscall_64+0x9f/0x180
  entry_SYSCALL_64_after_hwframe+0x76/0x7e
 RIP: 0033:0x7f436712b68b
 Code: 73 01 c3 48 8b 0d 8d a7 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 b0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 5d a7 0c 00 f7 d8 64 89 01 48
 RSP: 002b:00007ffe9f1a8658 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
 RAX: ffffffffffffffda RBX: 00005559b367fd80 RCX: 00007f436712b68b
 RDX: 0000000000000000 RSI: 0000000000000800 RDI: 00005559b367fde8
 RBP: 00007ffe9f1a8680 R08: 1999999999999999 R09: 0000000000000000
 R10: 00007f43671a5fe0 R11: 0000000000000206 R12: 0000000000000000
 R13: 00007ffe9f1a86b0 R14: 0000000000000000 R15: 0000000000000000
  </TASK>

Reported-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
Tested-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
CC: stable@vger.kernel.org # 6.13+
---
 fs/btrfs/zoned.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index 4956baf8220a..6c730f6bce10 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -1277,7 +1277,7 @@ struct zone_info {
 
 static int btrfs_load_zone_info(struct btrfs_fs_info *fs_info, int zone_idx,
 				struct zone_info *info, unsigned long *active,
-				struct btrfs_chunk_map *map)
+				struct btrfs_chunk_map *map, bool new)
 {
 	struct btrfs_dev_replace *dev_replace = &fs_info->dev_replace;
 	struct btrfs_device *device;
@@ -1307,6 +1307,8 @@ static int btrfs_load_zone_info(struct btrfs_fs_info *fs_info, int zone_idx,
 		return 0;
 	}
 
+	ASSERT(!new || btrfs_dev_is_empty_zone(device, info->physical));
+
 	/* This zone will be used for allocation, so mark this zone non-empty. */
 	btrfs_dev_clear_zone_empty(device, info->physical);
 
@@ -1319,6 +1321,18 @@ static int btrfs_load_zone_info(struct btrfs_fs_info *fs_info, int zone_idx,
 	 * to determine the allocation offset within the zone.
 	 */
 	WARN_ON(!IS_ALIGNED(info->physical, fs_info->zone_size));
+
+	if (new) {
+		sector_t capacity;
+
+		capacity = bdev_zone_capacity(device->bdev, info->physical >> SECTOR_SHIFT);
+		up_read(&dev_replace->rwsem);
+		info->alloc_offset = 0;
+		info->capacity = capacity << SECTOR_SHIFT;
+
+		return 0;
+	}
+
 	nofs_flag = memalloc_nofs_save();
 	ret = btrfs_get_dev_zone(device, info->physical, &zone);
 	memalloc_nofs_restore(nofs_flag);
@@ -1588,7 +1602,7 @@ int btrfs_load_block_group_zone_info(struct btrfs_block_group *cache, bool new)
 	}
 
 	for (i = 0; i < map->num_stripes; i++) {
-		ret = btrfs_load_zone_info(fs_info, i, &zone_info[i], active, map);
+		ret = btrfs_load_zone_info(fs_info, i, &zone_info[i], active, map, new);
 		if (ret)
 			goto out;
 
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/2] block: introduce zone capacity helper
  2025-03-18  0:59 ` [PATCH v2 1/2] block: introduce zone capacity helper Naohiro Aota
@ 2025-03-18  5:14   ` Damien Le Moal
  2025-03-18  5:54     ` Christoph Hellwig
  2025-03-18  7:27   ` Johannes Thumshirn
  1 sibling, 1 reply; 9+ messages in thread
From: Damien Le Moal @ 2025-03-18  5:14 UTC (permalink / raw)
  To: Naohiro Aota, linux-btrfs; +Cc: axboe, linux-block, hch

On 3/18/25 9:59 AM, Naohiro Aota wrote:
> {bdev,disk}_zone_capacity() takes block_device or gendisk and sector position
> and returns the zone capacity of the corresponding zone.
> 
> With that, move disk_nr_zones() and blk_zone_plug_bio() to consolidate them in
> the same #ifdef block.
> 
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> ---
>  include/linux/blkdev.h | 67 ++++++++++++++++++++++++++++--------------
>  1 file changed, 45 insertions(+), 22 deletions(-)
> 
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index d37751789bf5..6f1bdec27ac7 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -691,23 +691,6 @@ static inline bool blk_queue_is_zoned(struct request_queue *q)
>  		(q->limits.features & BLK_FEAT_ZONED);
>  }
>  
> -#ifdef CONFIG_BLK_DEV_ZONED
> -static inline unsigned int disk_nr_zones(struct gendisk *disk)
> -{
> -	return disk->nr_zones;
> -}
> -bool blk_zone_plug_bio(struct bio *bio, unsigned int nr_segs);
> -#else /* CONFIG_BLK_DEV_ZONED */
> -static inline unsigned int disk_nr_zones(struct gendisk *disk)
> -{
> -	return 0;
> -}
> -static inline bool blk_zone_plug_bio(struct bio *bio, unsigned int nr_segs)
> -{
> -	return false;
> -}
> -#endif /* CONFIG_BLK_DEV_ZONED */
> -
>  static inline unsigned int disk_zone_no(struct gendisk *disk, sector_t sector)
>  {
>  	if (!blk_queue_is_zoned(disk->queue))
> @@ -715,11 +698,6 @@ static inline unsigned int disk_zone_no(struct gendisk *disk, sector_t sector)
>  	return sector >> ilog2(disk->queue->limits.chunk_sectors);
>  }
>  
> -static inline unsigned int bdev_nr_zones(struct block_device *bdev)
> -{
> -	return disk_nr_zones(bdev->bd_disk);
> -}
> -
>  static inline unsigned int bdev_max_open_zones(struct block_device *bdev)
>  {
>  	return bdev->bd_disk->queue->limits.max_open_zones;
> @@ -826,6 +804,51 @@ static inline u64 sb_bdev_nr_blocks(struct super_block *sb)
>  		(sb->s_blocksize_bits - SECTOR_SHIFT);
>  }
>  
> +#ifdef CONFIG_BLK_DEV_ZONED
> +static inline unsigned int disk_nr_zones(struct gendisk *disk)
> +{
> +	return disk->nr_zones;
> +}
> +bool blk_zone_plug_bio(struct bio *bio, unsigned int nr_segs);
> +
> +/**
> + * disk_zone_capacity - returns the zone capacity of zone at @pos

@pos is not the start of the zone, so this should be:

 * disk_zone_capacity - returns the capacity of the zone containing @sect

And rename pos to sect.

With that, this looks good to me, so feel free to add:

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/2] block: introduce zone capacity helper
  2025-03-18  5:14   ` Damien Le Moal
@ 2025-03-18  5:54     ` Christoph Hellwig
  2025-03-18  7:57       ` Damien Le Moal
  0 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2025-03-18  5:54 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: Naohiro Aota, linux-btrfs, axboe, linux-block, hch

On Tue, Mar 18, 2025 at 02:14:29PM +0900, Damien Le Moal wrote:
> @pos is not the start of the zone, so this should be:
> 
>  * disk_zone_capacity - returns the capacity of the zone containing @sect
> 
> And rename pos to sect.

While we're nitpicking:  sect is a bit weird, the block layer usually
spells it out as sectors (sect sounds too close to the german word for
sparkling wine anyway :))


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] btrfs: zoned: skip reporting zone for new block group
  2025-03-18  0:59 ` [PATCH v2 2/2] btrfs: zoned: skip reporting zone for new block group Naohiro Aota
@ 2025-03-18  7:25   ` Johannes Thumshirn
  0 siblings, 0 replies; 9+ messages in thread
From: Johannes Thumshirn @ 2025-03-18  7:25 UTC (permalink / raw)
  To: Naohiro Aota, linux-btrfs@vger.kernel.org
  Cc: dlemoal@kernel.org, axboe@kernel.dk, linux-block@vger.kernel.org,
	hch@infradead.org, Shinichiro Kawasaki

On 18.03.25 02:00, Naohiro Aota wrote:
> There is a potential deadlock if we do report zones in an IO context, detailed
> in below lockdep report. When one process do a report zones and another process
                                         does ^
> freezes the block device, the report zones side cannot allocate a tag because
> the freeze is already started. This can thus result in new block group creation
> to hang forever, blocking the write path.
> 



>    entry_SYSCALL_64_after_hwframe+0x76/0x7e
>   RIP: 0033:0x7f436712b68b
>   Code: 73 01 c3 48 8b 0d 8d a7 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 b0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 5d a7 0c 00 f7 d8 64 89 01 48

The code line isn't really necessary for the bug report and overly long, 
so I think you can just delete it.
Other than that,

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/2] block: introduce zone capacity helper
  2025-03-18  0:59 ` [PATCH v2 1/2] block: introduce zone capacity helper Naohiro Aota
  2025-03-18  5:14   ` Damien Le Moal
@ 2025-03-18  7:27   ` Johannes Thumshirn
  1 sibling, 0 replies; 9+ messages in thread
From: Johannes Thumshirn @ 2025-03-18  7:27 UTC (permalink / raw)
  To: Naohiro Aota, linux-btrfs@vger.kernel.org
  Cc: dlemoal@kernel.org, axboe@kernel.dk, linux-block@vger.kernel.org,
	hch@infradead.org

Minus the nitpicks from Damien and Christoph,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/2] block: introduce zone capacity helper
  2025-03-18  5:54     ` Christoph Hellwig
@ 2025-03-18  7:57       ` Damien Le Moal
  2025-03-18  8:04         ` Christoph Hellwig
  0 siblings, 1 reply; 9+ messages in thread
From: Damien Le Moal @ 2025-03-18  7:57 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Naohiro Aota, linux-btrfs, axboe, linux-block

On 3/18/25 2:54 PM, Christoph Hellwig wrote:
> On Tue, Mar 18, 2025 at 02:14:29PM +0900, Damien Le Moal wrote:
>> @pos is not the start of the zone, so this should be:
>>
>>  * disk_zone_capacity - returns the capacity of the zone containing @sect
>>
>> And rename pos to sect.
> 
> While we're nitpicking:  sect is a bit weird, the block layer usually
> spells it out as sectors (sect sounds too close to the german word for
> sparkling wine anyway :))

Given that this is not a number of sectors, "sector" would be the right name
then :)


-- 
Damien Le Moal
Western Digital Research

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/2] block: introduce zone capacity helper
  2025-03-18  7:57       ` Damien Le Moal
@ 2025-03-18  8:04         ` Christoph Hellwig
  0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2025-03-18  8:04 UTC (permalink / raw)
  To: Damien Le Moal
  Cc: Christoph Hellwig, Naohiro Aota, linux-btrfs, axboe, linux-block

On Tue, Mar 18, 2025 at 04:57:45PM +0900, Damien Le Moal wrote:
> Given that this is not a number of sectors, "sector" would be the right name
> then :)

Yes.  That's what I meant, but I obviously can't type before the
first three coffees of the day.


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-03-18  8:04 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-18  0:59 [PATCH v2 0/2] btrfs: zoned: skip reporting zone for new block group Naohiro Aota
2025-03-18  0:59 ` [PATCH v2 1/2] block: introduce zone capacity helper Naohiro Aota
2025-03-18  5:14   ` Damien Le Moal
2025-03-18  5:54     ` Christoph Hellwig
2025-03-18  7:57       ` Damien Le Moal
2025-03-18  8:04         ` Christoph Hellwig
2025-03-18  7:27   ` Johannes Thumshirn
2025-03-18  0:59 ` [PATCH v2 2/2] btrfs: zoned: skip reporting zone for new block group Naohiro Aota
2025-03-18  7:25   ` Johannes Thumshirn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox