| Message ID | 9c00c066e9529f1a6439c1c8895a8f0d010a07e5.1733887702.git.naohiro.aota@wdc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | btrfs: zoned: calculate max_zone_append_size properly on non-zoned setup | expand |
On Wed, Dec 11, 2024 at 12:36:00PM +0900, Naohiro Aota wrote: > Since commit 559218d43ec9 ("block: pre-calculate max_zone_append_sectors"), > queue_limits's max_zone_append_sectors is default to be 0 and it is only > updated when there is a zoned device. So, we have > lim->max_zone_append_sectors = 0 when there is no zoned device in the > filesystem. Which makes sense as zoned append isn't used on conventional devices. > > That leads to fs_info->max_zone_append_size and fs_info->max_extent_size to > be 0, which causes several errors. One example is the divide error as But max_extent_size == 0 is a real problem. Can you try the patch below? diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c index 11ed523e528e..27f4472fb559 100644 --- a/fs/btrfs/zoned.c +++ b/fs/btrfs/zoned.c @@ -748,8 +748,9 @@ int btrfs_check_zoned_mode(struct btrfs_fs_info *fs_info) (u64)lim->max_segments << PAGE_SHIFT), fs_info->sectorsize); fs_info->fs_devices->chunk_alloc_policy = BTRFS_CHUNK_ALLOC_ZONED; - if (fs_info->max_zone_append_size < fs_info->max_extent_size) - fs_info->max_extent_size = fs_info->max_zone_append_size; + + fs_info->max_extent_size = min_not_zero(fs_info->max_extent_size, + fs_info->max_zone_append_size); /* * Check mount options here, because we might change fs_info->zoned
On Dec 11, 2024 / 07:20, Christoph Hellwig wrote: > On Wed, Dec 11, 2024 at 12:36:00PM +0900, Naohiro Aota wrote: > > Since commit 559218d43ec9 ("block: pre-calculate max_zone_append_sectors"), > > queue_limits's max_zone_append_sectors is default to be 0 and it is only > > updated when there is a zoned device. So, we have > > lim->max_zone_append_sectors = 0 when there is no zoned device in the > > filesystem. > > Which makes sense as zoned append isn't used on conventional devices. > > > > > That leads to fs_info->max_zone_append_size and fs_info->max_extent_size to > > be 0, which causes several errors. One example is the divide error as > > But max_extent_size == 0 is a real problem. Can you try the patch below? > > diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c > index 11ed523e528e..27f4472fb559 100644 > --- a/fs/btrfs/zoned.c > +++ b/fs/btrfs/zoned.c > @@ -748,8 +748,9 @@ int btrfs_check_zoned_mode(struct btrfs_fs_info *fs_info) > (u64)lim->max_segments << PAGE_SHIFT), > fs_info->sectorsize); > fs_info->fs_devices->chunk_alloc_policy = BTRFS_CHUNK_ALLOC_ZONED; > - if (fs_info->max_zone_append_size < fs_info->max_extent_size) > - fs_info->max_extent_size = fs_info->max_zone_append_size; > + > + fs_info->max_extent_size = min_not_zero(fs_info->max_extent_size, > + fs_info->max_zone_append_size); > > /* > * Check mount options here, because we might change fs_info->zoned I tried this change instead of Naohiro's fix patch, and observed the btrfs/001 failure goes away. This change also looks good from problem fix point of view.
diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c index abea8f2f497e..12ad6fcc2513 100644 --- a/fs/btrfs/zoned.c +++ b/fs/btrfs/zoned.c @@ -741,12 +741,23 @@ int btrfs_check_zoned_mode(struct btrfs_fs_info *fs_info) * we add the pages one by one to a bio, and cannot increase the * metadata reservation even if it increases the number of extents, it * is safe to stick with the limit. + * + * If there is no zoned device in the filesystem, we have + * max_zone_append_sectors = 0. That will cause + * fs_info->max_zone_append_size and fs_info->max_extent_size to be + * 0 in the following lines. Set the maximum value to avoid that. */ - fs_info->max_zone_append_size = ALIGN_DOWN( - min3((u64)lim->max_zone_append_sectors << SECTOR_SHIFT, - (u64)lim->max_sectors << SECTOR_SHIFT, - (u64)lim->max_segments << PAGE_SHIFT), - fs_info->sectorsize); + if (lim->features & BLK_FEAT_ZONED) + fs_info->max_zone_append_size = ALIGN_DOWN( + min3((u64)lim->max_zone_append_sectors << SECTOR_SHIFT, + (u64)lim->max_sectors << SECTOR_SHIFT, + (u64)lim->max_segments << PAGE_SHIFT), + fs_info->sectorsize); + else + fs_info->max_zone_append_size = ALIGN_DOWN( + min((u64)lim->max_sectors << SECTOR_SHIFT, + (u64)lim->max_segments << PAGE_SHIFT), + fs_info->sectorsize); fs_info->fs_devices->chunk_alloc_policy = BTRFS_CHUNK_ALLOC_ZONED; if (fs_info->max_zone_append_size < fs_info->max_extent_size) fs_info->max_extent_size = fs_info->max_zone_append_size;
Since commit 559218d43ec9 ("block: pre-calculate max_zone_append_sectors"), queue_limits's max_zone_append_sectors is default to be 0 and it is only updated when there is a zoned device. So, we have lim->max_zone_append_sectors = 0 when there is no zoned device in the filesystem. That leads to fs_info->max_zone_append_size and fs_info->max_extent_size to be 0, which causes several errors. One example is the divide error as below. Running simple test as btrfs/001 on a non-zoned device with the zoned mode (zoned emulation) leads to this error because we have fs_info->max_extent_size = 0 in count_max_extents(). Oops: divide error: 0000 [#1] PREEMPT SMP KASAN NOPTI CPU: 21 UID: 0 PID: 2378822 Comm: dd Tainted: G W 6.13.0-rc2-kts #1 Tainted: [W]=WARN Hardware name: Supermicro SYS-520P-WTR/X12SPW-TF, BIOS 1.2 02/14/2022 RIP: 0010:btrfs_delalloc_reserve_metadata+0x161/0x790 [btrfs] The block layer logic, having max_zone_append_sectors = 0 by stacking non-zoned devices, seems reasonable to me. So, let's deal with that in btrfs side by ignoring max_zone_append_sectors if it is non-zoned setup. Reported-by: Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com> Fixes: 559218d43ec9 ("block: pre-calculate max_zone_append_sectors") CC: Christoph Hellwig <hch@lst.de> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com> --- fs/btrfs/zoned.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)