Re: [PATCH 3/8] btrfs: scrub: use flexible array for scrub_page::csums

Josef Bacik
2020-10-26T14:23:56Z

On 10/26/20 3:11 AM, Qu Wenruo wrote:
> There are several factors affecting how many checksum bytes are needed
> for one scrub_page:
>
> - Sector size and page size
> For subpage case, one page can contain several sectors, thus the csum
> size will differ.
>
> - Checksum size
> Since btrfs supports different csum size now, which can vary from 4
> bytes for CRC32 to 32 bytes for SHA256.
>
> So instead of using fixed BTRFS_CSUM_SIZE, now use flexible array for
> scrub_page::csums, and determine the size at scrub_page allocation time.
>
> This does not only provide the basis for later subpage scrub support,
> but also reduce the memory usage for default btrfs on x86_64.
> As the default CRC32 only uses 4 bytes, thus we can save 28 bytes for
> each scrub page.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/scrub.c | 41 ++++++++++++++++++++++++++++++-----------
> 1 file changed, 30 insertions(+), 11 deletions(-)
>
> diff --git a/fs/btrfs/scrub.c b/fs/btrfs/scrub.c
> index 3cb51d1f061f..321d6d457942 100644
> --- a/fs/btrfs/scrub.c
> +++ b/fs/btrfs/scrub.c
> @@ -76,9 +76,14 @@ struct scrub_page {
> unsigned int have_csum:1;
> unsigned int io_error:1;
> };
> - u8 csum[BTRFS_CSUM_SIZE];
> -
> struct scrub_recover *recover;
> +
> + /*
> + * The csums size for the page is deteremined by page size,
> + * sector size and csum size.
> + * Thus the length has to be determined at runtime.
> + */
> + u8 csums[];
> };
>
> struct scrub_bio {
> @@ -207,6 +212,19 @@ struct full_stripe_lock {
> struct mutex mutex;
> };
>
> +static struct scrub_page *alloc_scrub_page(struct scrub_ctx *sctx, gfp_t mask)
> +{
> + u32 sectorsize = sctx->fs_info->sectorsize;
> + size_t size;
> +
> + /* No support for multi-page sector size yet */
> + ASSERT(PAGE_SIZE >= sectorsize && IS_ALIGNED(PAGE_SIZE, sectorsize));

Check patch complains about this

WARNING: Comparisons should place the constant on the right side of the test
#32: FILE: fs/btrfs/scrub.c:221:
+ ASSERT(PAGE_SIZE >= sectorsize && IS_ALIGNED(PAGE_SIZE, sectorsize));

Once you fix that you can add

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef