On Mon, Apr 19, 2021 at 02:45:12PM +0800, Qu Wenruo wrote:
> Currently mkfs.btrfs will output a warning message if the sectorsize is
> not the same as page size:
> WARNING: the filesystem may not be mountable, sectorsize 4096 doesn't match
> page size 65536
>
> But since btrfs subpage support for 64K page size is comming, this
> output is populating the golden output of fstests, causing tons of false
> alerts.
>
> This patch will make teach mkfs.btrfs to check
> /sys/fs/btrfs/features/supported_sectorsizes, and compare if the sector
> size is supported.
>
> Then only output above warning message if the sector size is not
> supported.
>
> This patch will also introduce a new helper,
> sysfs_open_global_feature_file() to make it more obvious which global
> feature file we're opening.
>
> Signed-off-by: Qu Wenruo <[email protected]>
> ---
> changelog:
> v2:
> - Introduce new helper to open global feature file
> - Extra the supported sectorsize check into its own function
> - Do proper token check other than strstr()
> - Fix the bug that we're passing @page_size to check
> ---
> common/fsfeatures.c | 49 ++++++++++++++++++++++++++++++++++++++++++++-
> common/utils.c | 15 ++++++++++++++
> common/utils.h | 1 +
> 3 files changed, 64 insertions(+), 1 deletion(-)
>
> diff --git a/common/fsfeatures.c b/common/fsfeatures.c
> index 569208a9e5b1..6641c44dfa45 100644
> --- a/common/fsfeatures.c
> +++ b/common/fsfeatures.c
> @@ -327,8 +327,50 @@ u32 get_running_kernel_version(void)
>
> return version;
> }
> +
> +/*
> + * The buffer size should be strlen("4096 8192 16384 32768 65536"),
> + * which is 28, then we just round it up to 32.
> + */
> +#define SUPPORTED_SECTORSIZE_BUF_SIZE 32
> +
> +/*
> + * Check if the current kernel supports given sectorsize.
> + *
> + * Return true if the sectorsize is supported.
> + * Return false otherwise.
> + */
> +static bool check_supported_sectorsize(u32 sectorsize)
> +{
> + char supported_buf[SUPPORTED_SECTORSIZE_BUF_SIZE] = { 0 };
> + char sectorsize_buf[SUPPORTED_SECTORSIZE_BUF_SIZE] = { 0 };
> + char *this_char;
> + char *save_ptr = NULL;
> + int fd;
> + int ret;
> +
> + fd = sysfs_open_global_feature_file("supported_sectorsizes");
> + if (fd < 0)
> + return false;
> + ret = sysfs_read_file(fd, supported_buf, SUPPORTED_SECTORSIZE_BUF_SIZE);
> + close(fd);
> + if (ret < 0)
> + return false;
> + snprintf(sectorsize_buf, SUPPORTED_SECTORSIZE_BUF_SIZE,
> + "%u", sectorsize);
> +
> + for (this_char = strtok_r(supported_buf, " ", &save_ptr);
> + this_char != NULL;
> + this_char = strtok_r(NULL, ",", &save_ptr)) {
Based on the example file contents in the comment, I would expect " " as
the delimeter for looping through the supported sizes, not ",".
> + if (!strncmp(this_char, sectorsize_buf, strlen(sectorsize_buf)))
> + return true;
> + }
> + return false;
> +}
> +
> int btrfs_check_sectorsize(u32 sectorsize)
> {
> + bool sectorsize_checked = false;
> u32 page_size = (u32)sysconf(_SC_PAGESIZE);
>
> if (!is_power_of_2(sectorsize)) {
> @@ -340,7 +382,12 @@ int btrfs_check_sectorsize(u32 sectorsize)
> sectorsize);
> return -EINVAL;
> }
> - if (page_size != sectorsize)
> + if (page_size == sectorsize)
> + sectorsize_checked = true;
> + else
> + sectorsize_checked = check_supported_sectorsize(sectorsize);
> +
> + if (!sectorsize_checked)
> warning(
> "the filesystem may not be mountable, sectorsize %u doesn't match page size
> %u",
> sectorsize, page_size);
> diff --git a/common/utils.c b/common/utils.c
> index 57e41432c8fb..e8b35879f19f 100644
> --- a/common/utils.c
> +++ b/common/utils.c
> @@ -2205,6 +2205,21 @@ int sysfs_open_fsid_file(int fd, const char *filename)
> return open(sysfs_file, O_RDONLY);
> }
>
> +/*
> + * Open a file in global btrfs features directory and return the file
> + * descriptor or error.
> + */
> +int sysfs_open_global_feature_file(const char *feature_name)
> +{
> + char path[PATH_MAX];
> + int ret;
> +
> + ret = path_cat_out(path, "/sys/fs/btrfs/features", feature_name);
> + if (ret < 0)
> + return ret;
> + return open(path, O_RDONLY);
> +}
> +
> /*
> * Read up to @size bytes to @buf from @fd
> */
> diff --git a/common/utils.h b/common/utils.h
> index c38bdb08077c..d2f6416a9b5a 100644
> --- a/common/utils.h
> +++ b/common/utils.h
> @@ -169,6 +169,7 @@ char *btrfs_test_for_multiple_profiles(int fd);
> int btrfs_warn_multiple_profiles(int fd);
>
> int sysfs_open_fsid_file(int fd, const char *filename);
> +int sysfs_open_global_feature_file(const char *feature_name);
> int sysfs_read_file(int fd, char *buf, size_t size);
>
> #endif
> --
> 2.31.1
>
