Re: [PATCH] btrfs: fix max max_inline for pagesize=64K
From: David Sterba <hidden>
Date: 2021-08-16 15:13:27
On Tue, Aug 10, 2021 at 11:23:44PM +0800, Anand Jain wrote:
The mount option max_inline ranges from 0 to the sectorsize (which is equal to pagesize). But we parse the mount options too early and before the sectorsize is a cache from the superblock. So the upper limit of max_inline is unaware of the actual sectorsize. And is limited by the temporary sectorsize 4096 (as below), even on a system where the default sectorsize is 64K.
So the question is what's a sensible value for >4K sectors, which is 64K in this case. Generally we allow setting values that may make sense only for some limited usecase and leave it up to the user to decide. The inline files reduce the slack space and on 64K sectors it could be more noticeable than on 4K sectors. It's a trade off as the inline data are stored in metadata blocks that are considered more precious. Do you have any analysis of file size distribution on 64K systems for some normal use case like roo partition? I think this is worth fixing so to be in line with constraints we have for 4K sectors but some numbers would be good too.
quoted hunk ↗ jump to hunk
disk-io.c :: 2980 /* Usable values until the real ones are cached from the superblock */ 2981 fs_info->nodesize = 4096; 2982 fs_info->sectorsize = 4096; Fix this by reading the superblock sectorsize before the mount option parse. Reported-by: Alexander Tsvetkov <redacted> Signed-off-by: Anand Jain <redacted> --- fs/btrfs/disk-io.c | 49 +++++++++++++++++++++++----------------------- 1 file changed, 25 insertions(+), 24 deletions(-)diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 2dd56ee23b35..d9505b35c7f5 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c@@ -3317,6 +3317,31 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device */ fs_info->compress_type = BTRFS_COMPRESS_ZLIB; + /* + * flag our filesystem as having big metadata blocks if + * they are bigger than the page size
Please fix/reformat/improve any comments that are in moved code.
+ */
+ if (btrfs_super_nodesize(disk_super) > PAGE_SIZE) {
+ if (!(features & BTRFS_FEATURE_INCOMPAT_BIG_METADATA))
+ btrfs_info(fs_info,
+ "flagging fs with big metadata feature");
+ features |= BTRFS_FEATURE_INCOMPAT_BIG_METADATA;
+ }
+
+ /* btrfs_parse_options uses fs_info::sectorsize, so read it here */
+ nodesize = btrfs_super_nodesize(disk_super);
+ sectorsize = btrfs_super_sectorsize(disk_super);
+ stripesize = sectorsize;
+ fs_info->dirty_metadata_batch = nodesize * (1 + ilog2(nr_cpu_ids));
+ fs_info->delalloc_batch = sectorsize * 512 * (1 + ilog2(nr_cpu_ids));
+
+ /* Cache block sizes */
+ fs_info->nodesize = nodesize;
+ fs_info->sectorsize = sectorsize;
+ fs_info->sectorsize_bits = ilog2(sectorsize);
+ fs_info->csums_per_leaf = BTRFS_MAX_ITEM_SIZE(fs_info) / fs_info->csum_size;
+ fs_info->stripesize = stripesize;It looks that there are no invariants broken by moving that code, so ok.
quoted hunk ↗ jump to hunk
+ ret = btrfs_parse_options(fs_info, options, sb->s_flags); if (ret) { err = ret;@@ -3343,30 +3368,6 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device if (features & BTRFS_FEATURE_INCOMPAT_SKINNY_METADATA) btrfs_info(fs_info, "has skinny extents"); - /* - * flag our filesystem as having big metadata blocks if - * they are bigger than the page size - */ - if (btrfs_super_nodesize(disk_super) > PAGE_SIZE) { - if (!(features & BTRFS_FEATURE_INCOMPAT_BIG_METADATA)) - btrfs_info(fs_info, - "flagging fs with big metadata feature"); - features |= BTRFS_FEATURE_INCOMPAT_BIG_METADATA; - } - - nodesize = btrfs_super_nodesize(disk_super); - sectorsize = btrfs_super_sectorsize(disk_super); - stripesize = sectorsize; - fs_info->dirty_metadata_batch = nodesize * (1 + ilog2(nr_cpu_ids)); - fs_info->delalloc_batch = sectorsize * 512 * (1 + ilog2(nr_cpu_ids)); - - /* Cache block sizes */ - fs_info->nodesize = nodesize; - fs_info->sectorsize = sectorsize; - fs_info->sectorsize_bits = ilog2(sectorsize); - fs_info->csums_per_leaf = BTRFS_MAX_ITEM_SIZE(fs_info) / fs_info->csum_size; - fs_info->stripesize = stripesize; - /* * mixed block groups end up with duplicate but slightly offset * extent buffers for the same range. It leads to corruptions-- 2.27.0