On 5.07.19 г. 10:26 ч., Qu Wenruo wrote:
> [BUG]
> On aarch64 with 64k page size, mkfs.btrfs -s option doesn't work:
>   $ mkfs.btrfs  -s 4096 ~/10G.img  -f
>   btrfs-progs v5.1.1
>   See http://btrfs.wiki.kernel.org for more information.
> 
>   Label:              (null)
>   UUID:               c2a09334-aaca-4980-aefa-4b3e27390658
>   Node size:          65536
>   Sector size:        65536           << Still 64K, not 4K
>   Filesystem size:    10.00GiB
>   Block group profiles:
>     Data:             single            8.00MiB
>     Metadata:         DUP             256.00MiB
>     System:           DUP               8.00MiB
>   SSD detected:       no
>   Incompat features:  extref, skinny-metadata
>   Number of devices:  1
>   Devices:
>      ID        SIZE  PATH
>       1    10.00GiB  /home/adam/10G.img
> 
> [CAUSE]
> This is because we automatically detect sectorsize based on current
> system page size, then get the maxium number between user specified -s
> parameter and system page size.
> 
> It's fine for x86 as it has fixed page size 4K, also the minimium valid
> sector size.
> 
> But for system like aarch64 or ppc64le, where we can have 64K page size,
> and it makes us unable to create a 4k sector sized btrfs.
> 
> [FIX]
> Only do auto detect when no -s|--sectorsize option is specified.
> 
> Signed-off-by: Qu Wenruo <w...@suse.com>
> ---
>  mkfs/main.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/mkfs/main.c b/mkfs/main.c
> index 8dbec0717b89..26d84e9dafc3 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -817,6 +817,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
>       char *source_dir = NULL;
>       bool source_dir_set = false;
>       bool shrink_rootdir = false;
> +     bool sectorsize_set = false;
>       u64 source_dir_size = 0;
>       u64 min_dev_size;
>       u64 shrink_size;
> @@ -906,6 +907,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
>                               }
>                       case 's':
>                               sectorsize = parse_size(optarg);
> +                             sectorsize_set = true;
>                               break;
>                       case 'b':
>                               block_count = parse_size(optarg);
> @@ -943,7 +945,15 @@ int BOX_MAIN(mkfs)(int argc, char **argv)
>               printf("See %s for more information.\n\n", PACKAGE_URL);
>       }
>  
> -     sectorsize = max(sectorsize, (u32)sysconf(_SC_PAGESIZE));
> +     if (!sectorsize_set)
> +             sectorsize = max(sectorsize, (u32)sysconf(_SC_PAGESIZE));

This means it's possible for the user to create a filesystem that is not
mountable on his current machine, due to the presence of the following
check in validate_super:

if (sectorsize != PAGE_SIZE) {
btrfs_err(..)
}

Perhaps the risk is not that big since if someone creates such a
filesystem they will almost instantly realize it won't work and
re-create it properly.



> +     if (!is_power_of_2(sectorsize) || sectorsize < 4096 ||
> +         sectorsize > SZ_64K) {

nit: Perhaps this check should be modified so that it follows the kernel
style :
if (!is_power_of_2(sectorsize) || sectorsize < 4096 ||
              sectorsize > BTRFS_MAX_METADATA_BLOCKSIZE) {

MAX_METADATA_BLOCKSIZE is defined as 64k but using the same defines
seems more clear to me.


> +             error(
> +             "invalid sectorsize: %u, expect either 4k, 8k, 16k, 32k or 64k",
> +                     sectorsize);
> +             goto error;
> +     }
>       stripesize = sectorsize;
>       saved_optind = optind;
>       dev_cnt = argc - optind;
> 

Reply via email to