On 2021/8/27 上午1:53, David Sterba wrote:

On Tue, Aug 24, 2021 at 01:54:21PM +0800, Anand Jain wrote:

quoted

On 16/08/2021 23:10, David Sterba wrote:

quoted

On Tue, Aug 10, 2021 at 11:23:44PM +0800, Anand Jain wrote:

quoted

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.

quoted

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.

Default max_inline for sectorsize=64K is an interesting topic and
probably long. If time permits, I will look into it.
Furthermore, we need test cases and a repo that can hold it (and
also add  read_policy test cases there).

IMO there is no need to hold this patch in search of optimum default
max_inline for 64K systems.

Yeah, I'm more interested in some reasonable value, now the default is
2048 but probably it should be sectorsize/2 in general.

Half of sectorsize is pretty solid to me.

But I'm afraid this is a little too late, especially considering we're
moving to 4K sectorsize as default for all page sizes.

Thanks,
Qu

quoted

This patch reports and fixes a bug due to which we are limited to test
max_inline only up to 4K on a 64K pagesize system. Not as our document
claimed as below.
-----------------
   man -s 5 btrfs
   ::
          max_inline=bytes
             (default: min(2048, page size) )
   ::
	In practice, this value is limited by the filesystem block size
	(named sectorsize at mkfs time), and memory page size of the
	system.
-----------------


   more below.

quoted

quoted

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.

   I think you are pointing to s/f/F and 80 chars long? Will fix.

Yes, already fixed in the committed version in misc-next, thanks.