| Message ID | da3a097233a87541120dbb2a9624841c7a9e3962.1622552385.git.johannes.thumshirn@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: zoned: limit max extent size to max zone append | expand |
On Tue, Jun 01, 2021 at 10:02:10PM +0900, Johannes Thumshirn wrote: > Damien reported a test failure with btrfs/209. The test itself ran fine, > but the fsck run afterwards reported a corrupted filesystem. > > The filesystem corruption happens because we're splitting an extent and > then writing the extent twice. We have to split the extent though, because > we're creating too large extents for a REQ_OP_ZONE_APPEND operation. > > When dumping the extent tree, we can see two EXTENT_ITEMs at the same > start address but different lengths. > > $ btrfs inspect dump-tree /dev/nullb1 -t extent > ... > item 19 key (269484032 EXTENT_ITEM 126976) itemoff 15470 itemsize 53 > refs 1 gen 7 flags DATA > extent data backref root FS_TREE objectid 257 offset 786432 count 1 > item 20 key (269484032 EXTENT_ITEM 262144) itemoff 15417 itemsize 53 > refs 1 gen 7 flags DATA > extent data backref root FS_TREE objectid 257 offset 786432 count 1 > > On a zoned filesystem, limit the size of an ordered extent to the maximum > size that can be issued as a single REQ_OP_ZONE_APPEND operation. > > Note: This patch breaks fstests btrfs/079, as it increases the number of > on-disk extents from 80 to 83 per 10M write. > > Reported-by: Damien Le Moal <damien.lemoal@wdc.com> > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > --- > fs/btrfs/ctree.h | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h > index 5d0398528a7a..6fbafaaebda0 100644 > --- a/fs/btrfs/ctree.h > +++ b/fs/btrfs/ctree.h > @@ -1373,7 +1373,10 @@ static inline u32 BTRFS_MAX_XATTR_SIZE(const struct btrfs_fs_info *info) > > static inline u64 btrfs_get_max_extent_size(struct btrfs_fs_info *fs_info) > { > - return BTRFS_MAX_EXTENT_SIZE; > + if (!fs_info || !fs_info->max_zone_append_size) > + return BTRFS_MAX_EXTENT_SIZE; > + return min_t(u64, BTRFS_MAX_EXTENT_SIZE, > + ALIGN_DOWN(fs_info->max_zone_append_size, PAGE_SIZE)); Should this be set only once in btrfs_check_zoned_mode ? > } > > /* > -- > 2.31.1 >
On 01/06/2021 18:20, David Sterba wrote: >> --- >> fs/btrfs/ctree.h | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h >> index 5d0398528a7a..6fbafaaebda0 100644 >> --- a/fs/btrfs/ctree.h >> +++ b/fs/btrfs/ctree.h >> @@ -1373,7 +1373,10 @@ static inline u32 BTRFS_MAX_XATTR_SIZE(const struct btrfs_fs_info *info) >> >> static inline u64 btrfs_get_max_extent_size(struct btrfs_fs_info *fs_info) >> { >> - return BTRFS_MAX_EXTENT_SIZE; >> + if (!fs_info || !fs_info->max_zone_append_size) >> + return BTRFS_MAX_EXTENT_SIZE; >> + return min_t(u64, BTRFS_MAX_EXTENT_SIZE, >> + ALIGN_DOWN(fs_info->max_zone_append_size, PAGE_SIZE)); > > Should this be set only once in btrfs_check_zoned_mode ? Do you mean adding a fs_info->max_extent_size member or fs_info->max_zone_append_size = min(BTRFS_MAX_EXTENT_SIZE, queue_max_append_size())? I'd opt for #1
On Wed, Jun 02, 2021 at 06:05:45AM +0000, Johannes Thumshirn wrote: > On 01/06/2021 18:20, David Sterba wrote: > >> --- > >> fs/btrfs/ctree.h | 5 ++++- > >> 1 file changed, 4 insertions(+), 1 deletion(-) > >> > >> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h > >> index 5d0398528a7a..6fbafaaebda0 100644 > >> --- a/fs/btrfs/ctree.h > >> +++ b/fs/btrfs/ctree.h > >> @@ -1373,7 +1373,10 @@ static inline u32 BTRFS_MAX_XATTR_SIZE(const struct btrfs_fs_info *info) > >> > >> static inline u64 btrfs_get_max_extent_size(struct btrfs_fs_info *fs_info) > >> { > >> - return BTRFS_MAX_EXTENT_SIZE; > >> + if (!fs_info || !fs_info->max_zone_append_size) > >> + return BTRFS_MAX_EXTENT_SIZE; > >> + return min_t(u64, BTRFS_MAX_EXTENT_SIZE, > >> + ALIGN_DOWN(fs_info->max_zone_append_size, PAGE_SIZE)); > > > > Should this be set only once in btrfs_check_zoned_mode ? > > Do you mean adding a fs_info->max_extent_size member or > fs_info->max_zone_append_size = min(BTRFS_MAX_EXTENT_SIZE, queue_max_append_size())? > > I'd opt for #1 Yeah, replace BTRFS_MAX_EXTENT_SIZE by a fs_info member and then adjust it for max append zone.
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index 5d0398528a7a..6fbafaaebda0 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -1373,7 +1373,10 @@ static inline u32 BTRFS_MAX_XATTR_SIZE(const struct btrfs_fs_info *info) static inline u64 btrfs_get_max_extent_size(struct btrfs_fs_info *fs_info) { - return BTRFS_MAX_EXTENT_SIZE; + if (!fs_info || !fs_info->max_zone_append_size) + return BTRFS_MAX_EXTENT_SIZE; + return min_t(u64, BTRFS_MAX_EXTENT_SIZE, + ALIGN_DOWN(fs_info->max_zone_append_size, PAGE_SIZE)); } /*
Damien reported a test failure with btrfs/209. The test itself ran fine, but the fsck run afterwards reported a corrupted filesystem. The filesystem corruption happens because we're splitting an extent and then writing the extent twice. We have to split the extent though, because we're creating too large extents for a REQ_OP_ZONE_APPEND operation. When dumping the extent tree, we can see two EXTENT_ITEMs at the same start address but different lengths. $ btrfs inspect dump-tree /dev/nullb1 -t extent ... item 19 key (269484032 EXTENT_ITEM 126976) itemoff 15470 itemsize 53 refs 1 gen 7 flags DATA extent data backref root FS_TREE objectid 257 offset 786432 count 1 item 20 key (269484032 EXTENT_ITEM 262144) itemoff 15417 itemsize 53 refs 1 gen 7 flags DATA extent data backref root FS_TREE objectid 257 offset 786432 count 1 On a zoned filesystem, limit the size of an ordered extent to the maximum size that can be issued as a single REQ_OP_ZONE_APPEND operation. Note: This patch breaks fstests btrfs/079, as it increases the number of on-disk extents from 80 to 83 per 10M write. Reported-by: Damien Le Moal <damien.lemoal@wdc.com> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> --- fs/btrfs/ctree.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)