Re: [PATCH v4 1/4] btrfs-progs: mkfs: Enhance minimal device size calculation to fix mkfs failure on small file
On Tue, Nov 28, 2017 at 08:40:47AM +0800, Qu Wenruo wrote:
>
>
> On 2017年11月28日 07:21, David Sterba wrote:
> > On Wed, Nov 01, 2017 at 09:30:40AM +0800, Qu Wenruo wrote:
> >> +u64 btrfs_min_dev_size(u32 nodesize, int mixed, u64 meta_profile,
> >> + u64 data_profile)
> >> +{
> >> + u64 reserved = 0;
> >> + u64 meta_size;
> >> + u64 data_size;
> >> +
> >> + if (mixed)
> >> + return 2 * (BTRFS_MKFS_SYSTEM_GROUP_SIZE +
> >> + btrfs_min_global_blk_rsv_size(nodesize));
> >> +
> >> + /*
> >> + * minimal size calculation is complex due to several factors:
> >> + * 1) Temporary chunk resue
> >> + *If specified chunk profile is SINGLE, we can resue
> >> + *temporary chunks, no need to alloc new chunks.
> >> + *
> >> + * 2) Different minimal chunk size for different profile
> >> + *For initial sys chunk, chunk size is fixed to 4M.
> >> + *For single profile, minimal chunk size is 8M for all.
> >> + *For other profiles, minimal chunk and stripe size
> >> + *differs from 8M to 64M.
> >> + *
> >> + * To calculate it a little easier, here we assume we don't
> >> + * reuse any temporary chunk, and calculate the size all
> >> + * by ourselves.
> >> + *
> >> + * Temporary chunks sizes are always fixed:
> >> + * One initial sys chunk, one SINGLE meta, and one SINGLE data.
> >> + * The latter two are all 8M, accroding to @calc_size of
> >> + * btrfs_alloc_chunk().
> >> + */
> >> + reserved += BTRFS_MKFS_SYSTEM_GROUP_SIZE + SZ_8M * 2;
> >> +
> >> + /*
> >> + * For real chunks, we need to refer different sizes:
> >> + * For SINGLE, it's still fixed to 8M (@calc_size).
> >> + * For other profiles, refer to max(@min_stripe_size, @calc_size).
> >> + *
> >> + * And use the stripe size to calculate its physical used space.
> >> + */
> >> + if (meta_profile & BTRFS_BLOCK_GROUP_PROFILE_MASK)
> >> + meta_size = SZ_8M + SZ_32M;
> >> + else
> >> + meta_size = SZ_8M + SZ_8M;
> >> + /* Only metadata put 2 stripes into one disk */
> >
> > 'mkfs.btrfs -d dup' also works, so I think this should be handled here
> > the same way as metadata, right?
>
> Just a few lines under.
> >
> >> + if (meta_profile & BTRFS_BLOCK_GROUP_DUP)
> >> + meta_size *= 2;
> >> + reserved += meta_size;
> >> +
> >> + if (data_profile & BTRFS_BLOCK_GROUP_PROFILE_MASK)
> >> + data_size = SZ_64M;
> >> + else
> >> + data_size = SZ_8M;
> >> + if (data_profile & BTRFS_BLOCK_GROUP_DUP)
> >> + data_size *= 2;
> Here data DUP is also handled.
>
> So the problem is about the word "Only".
Yeah, it's confusing when comment does not match the code because it's
not clear what's the actual intention.
> Do I need to re-submit the whole patchset or just a separate patch to
> update the comment?
No, I'll update it myself as it is fairly trivial change.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/4] btrfs-progs: mkfs: Enhance minimal device size calculation to fix mkfs failure on small file
On 2017年11月28日 07:21, David Sterba wrote:
> On Wed, Nov 01, 2017 at 09:30:40AM +0800, Qu Wenruo wrote:
>> +u64 btrfs_min_dev_size(u32 nodesize, int mixed, u64 meta_profile,
>> + u64 data_profile)
>> +{
>> +u64 reserved = 0;
>> +u64 meta_size;
>> +u64 data_size;
>> +
>> +if (mixed)
>> +return 2 * (BTRFS_MKFS_SYSTEM_GROUP_SIZE +
>> +btrfs_min_global_blk_rsv_size(nodesize));
>> +
>> +/*
>> + * minimal size calculation is complex due to several factors:
>> + * 1) Temporary chunk resue
>> + *If specified chunk profile is SINGLE, we can resue
>> + *temporary chunks, no need to alloc new chunks.
>> + *
>> + * 2) Different minimal chunk size for different profile
>> + *For initial sys chunk, chunk size is fixed to 4M.
>> + *For single profile, minimal chunk size is 8M for all.
>> + *For other profiles, minimal chunk and stripe size
>> + *differs from 8M to 64M.
>> + *
>> + * To calculate it a little easier, here we assume we don't
>> + * reuse any temporary chunk, and calculate the size all
>> + * by ourselves.
>> + *
>> + * Temporary chunks sizes are always fixed:
>> + * One initial sys chunk, one SINGLE meta, and one SINGLE data.
>> + * The latter two are all 8M, accroding to @calc_size of
>> + * btrfs_alloc_chunk().
>> + */
>> +reserved += BTRFS_MKFS_SYSTEM_GROUP_SIZE + SZ_8M * 2;
>> +
>> +/*
>> + * For real chunks, we need to refer different sizes:
>> + * For SINGLE, it's still fixed to 8M (@calc_size).
>> + * For other profiles, refer to max(@min_stripe_size, @calc_size).
>> + *
>> + * And use the stripe size to calculate its physical used space.
>> + */
>> +if (meta_profile & BTRFS_BLOCK_GROUP_PROFILE_MASK)
>> +meta_size = SZ_8M + SZ_32M;
>> +else
>> +meta_size = SZ_8M + SZ_8M;
>> +/* Only metadata put 2 stripes into one disk */
>
> 'mkfs.btrfs -d dup' also works, so I think this should be handled here
> the same way as metadata, right?
Just a few lines under.
>
>> +if (meta_profile & BTRFS_BLOCK_GROUP_DUP)
>> +meta_size *= 2;
>> +reserved += meta_size;
>> +
>> +if (data_profile & BTRFS_BLOCK_GROUP_PROFILE_MASK)
>> +data_size = SZ_64M;
>> +else
>> +data_size = SZ_8M;
>> +if (data_profile & BTRFS_BLOCK_GROUP_DUP)
>> +data_size *= 2;
Here data DUP is also handled.
So the problem is about the word "Only".
Do I need to re-submit the whole patchset or just a separate patch to
update the comment?
Thanks,
Qu
>> +reserved += data_size;
>> +return reserved;
>> +}
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v4 1/4] btrfs-progs: mkfs: Enhance minimal device size calculation to fix mkfs failure on small file
On Wed, Nov 01, 2017 at 09:30:40AM +0800, Qu Wenruo wrote:
> +u64 btrfs_min_dev_size(u32 nodesize, int mixed, u64 meta_profile,
> +u64 data_profile)
> +{
> + u64 reserved = 0;
> + u64 meta_size;
> + u64 data_size;
> +
> + if (mixed)
> + return 2 * (BTRFS_MKFS_SYSTEM_GROUP_SIZE +
> + btrfs_min_global_blk_rsv_size(nodesize));
> +
> + /*
> + * minimal size calculation is complex due to several factors:
> + * 1) Temporary chunk resue
> + *If specified chunk profile is SINGLE, we can resue
> + *temporary chunks, no need to alloc new chunks.
> + *
> + * 2) Different minimal chunk size for different profile
> + *For initial sys chunk, chunk size is fixed to 4M.
> + *For single profile, minimal chunk size is 8M for all.
> + *For other profiles, minimal chunk and stripe size
> + *differs from 8M to 64M.
> + *
> + * To calculate it a little easier, here we assume we don't
> + * reuse any temporary chunk, and calculate the size all
> + * by ourselves.
> + *
> + * Temporary chunks sizes are always fixed:
> + * One initial sys chunk, one SINGLE meta, and one SINGLE data.
> + * The latter two are all 8M, accroding to @calc_size of
> + * btrfs_alloc_chunk().
> + */
> + reserved += BTRFS_MKFS_SYSTEM_GROUP_SIZE + SZ_8M * 2;
> +
> + /*
> + * For real chunks, we need to refer different sizes:
> + * For SINGLE, it's still fixed to 8M (@calc_size).
> + * For other profiles, refer to max(@min_stripe_size, @calc_size).
> + *
> + * And use the stripe size to calculate its physical used space.
> + */
> + if (meta_profile & BTRFS_BLOCK_GROUP_PROFILE_MASK)
> + meta_size = SZ_8M + SZ_32M;
> + else
> + meta_size = SZ_8M + SZ_8M;
> + /* Only metadata put 2 stripes into one disk */
'mkfs.btrfs -d dup' also works, so I think this should be handled here
the same way as metadata, right?
> + if (meta_profile & BTRFS_BLOCK_GROUP_DUP)
> + meta_size *= 2;
> + reserved += meta_size;
> +
> + if (data_profile & BTRFS_BLOCK_GROUP_PROFILE_MASK)
> + data_size = SZ_64M;
> + else
> + data_size = SZ_8M;
> + if (data_profile & BTRFS_BLOCK_GROUP_DUP)
> + data_size *= 2;
> + reserved += data_size;
> + return reserved;
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 1/4] btrfs-progs: mkfs: Enhance minimal device size calculation to fix mkfs failure on small file
Since commit c11e36a29e84 ("Btrfs-progs: Do not force mixed block group
creation unless '-M' option is specified"), mkfs no longer use mixed
block group unless specified manually.
This breaks the minimal device size calculation, which only considered
mixed block group use case.
This patch enhances minimal device size calculation for mkfs, by using
different minimal stripe length (calculated from code) for different
profiles, and use them to calculate minimal device size.
Reported-by: Wesley Aptekar-Cassels
Fixes: c11e36a29e84 ("Btrfs-progs: Do not force mixed block group creation
unless '-M' option is specified")
Signed-off-by: Qu Wenruo
Reviewed-by: Lu Fengqi
---
mkfs/common.c | 68 +--
mkfs/common.h | 4 ++--
mkfs/main.c | 3 ++-
3 files changed, 65 insertions(+), 10 deletions(-)
diff --git a/mkfs/common.c b/mkfs/common.c
index 71318b10fa51..2c400f44dbf3 100644
--- a/mkfs/common.c
+++ b/mkfs/common.c
@@ -438,12 +438,6 @@ out:
return ret;
}
-u64 btrfs_min_dev_size(u32 nodesize)
-{
- return 2 * (BTRFS_MKFS_SYSTEM_GROUP_SIZE +
- btrfs_min_global_blk_rsv_size(nodesize));
-}
-
/*
* Btrfs minimum size calculation is complicated, it should include at least:
* 1. system group size
@@ -454,11 +448,71 @@ u64 btrfs_min_dev_size(u32 nodesize)
* To avoid the overkill calculation, (system group + global block rsv) * 2
* for *EACH* device should be good enough.
*/
-u64 btrfs_min_global_blk_rsv_size(u32 nodesize)
+static u64 btrfs_min_global_blk_rsv_size(u32 nodesize)
{
return (u64)nodesize << 10;
}
+u64 btrfs_min_dev_size(u32 nodesize, int mixed, u64 meta_profile,
+ u64 data_profile)
+{
+ u64 reserved = 0;
+ u64 meta_size;
+ u64 data_size;
+
+ if (mixed)
+ return 2 * (BTRFS_MKFS_SYSTEM_GROUP_SIZE +
+ btrfs_min_global_blk_rsv_size(nodesize));
+
+ /*
+* minimal size calculation is complex due to several factors:
+* 1) Temporary chunk resue
+*If specified chunk profile is SINGLE, we can resue
+*temporary chunks, no need to alloc new chunks.
+*
+* 2) Different minimal chunk size for different profile
+*For initial sys chunk, chunk size is fixed to 4M.
+*For single profile, minimal chunk size is 8M for all.
+*For other profiles, minimal chunk and stripe size
+*differs from 8M to 64M.
+*
+* To calculate it a little easier, here we assume we don't
+* reuse any temporary chunk, and calculate the size all
+* by ourselves.
+*
+* Temporary chunks sizes are always fixed:
+* One initial sys chunk, one SINGLE meta, and one SINGLE data.
+* The latter two are all 8M, accroding to @calc_size of
+* btrfs_alloc_chunk().
+*/
+ reserved += BTRFS_MKFS_SYSTEM_GROUP_SIZE + SZ_8M * 2;
+
+ /*
+* For real chunks, we need to refer different sizes:
+* For SINGLE, it's still fixed to 8M (@calc_size).
+* For other profiles, refer to max(@min_stripe_size, @calc_size).
+*
+* And use the stripe size to calculate its physical used space.
+*/
+ if (meta_profile & BTRFS_BLOCK_GROUP_PROFILE_MASK)
+ meta_size = SZ_8M + SZ_32M;
+ else
+ meta_size = SZ_8M + SZ_8M;
+ /* Only metadata put 2 stripes into one disk */
+ if (meta_profile & BTRFS_BLOCK_GROUP_DUP)
+ meta_size *= 2;
+ reserved += meta_size;
+
+ if (data_profile & BTRFS_BLOCK_GROUP_PROFILE_MASK)
+ data_size = SZ_64M;
+ else
+ data_size = SZ_8M;
+ if (data_profile & BTRFS_BLOCK_GROUP_DUP)
+ data_size *= 2;
+ reserved += data_size;
+ return reserved;
+}
+
#define isoctal(c) (((c) & ~7) == '0')
static inline void translate(char *f, char *t)
diff --git a/mkfs/common.h b/mkfs/common.h
index 3757e9e7fd0a..e9e3b3eb43a6 100644
--- a/mkfs/common.h
+++ b/mkfs/common.h
@@ -66,8 +66,8 @@ struct btrfs_mkfs_config {
};
int make_btrfs(int fd, struct btrfs_mkfs_config *cfg);
-u64 btrfs_min_dev_size(u32 nodesize);
-u64 btrfs_min_global_blk_rsv_size(u32 nodesize);
+u64 btrfs_min_dev_size(u32 nodesize, int mixed, u64 meta_profile,
+ u64 data_profile);
int test_minimum_size(const char *file, u64 min_dev_size);
int is_vol_small(const char *file);
int test_num_disk_vs_raid(u64 metadata_profile, u64 data_profile,
diff --git a/mkfs/main.c b/mkfs/main.c
index 6669a4b901af..982cafc07b84 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -979,7 +979,8 @@ int main(int argc, char **argv)
goto error;
}
- min_dev_size = btrfs_min_dev_size(nodesize);
+ min_dev_size = btrfs_min_dev_size(nodesize, mixed,
