| Message ID | 20240522060232.3569226-6-naohiro.aota@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: zoned: proper "mkfs.btrfs -b" support | expand |
在 2024/5/22 15:32, Naohiro Aota 写道: > While "byte_count" is eventually rounded down to sectorsize at make_btrfs() > or btrfs_add_to_fs_id(), it would be better round it down first and do the > size checks not to confuse the things. > > Also, on a zoned device, creating a btrfs whose size is not aligned to the > zone boundary can be confusing. Round it down further to the zone boundary. > > The size calculation with a source directory is also tweaked to be aligned. > > Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com> > --- > mkfs/main.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/mkfs/main.c b/mkfs/main.c > index a437ecc40c7f..baf889873b41 100644 > --- a/mkfs/main.c > +++ b/mkfs/main.c > @@ -1591,6 +1591,12 @@ int BOX_MAIN(mkfs)(int argc, char **argv) > min_dev_size = btrfs_min_dev_size(nodesize, mixed, > opt_zoned ? zone_size(file) : 0, > metadata_profile, data_profile); > + if (byte_count) { > + byte_count = round_down(byte_count, sectorsize); > + if (opt_zoned) > + byte_count = round_down(byte_count, zone_size(file)); > + } > + > /* > * Enlarge the destination file or create a new one, using the size > * calculated from source dir. > @@ -1624,12 +1630,13 @@ int BOX_MAIN(mkfs)(int argc, char **argv) > * Or we will always use source_dir_size calculated for mkfs. > */ > if (!byte_count) > - byte_count = device_get_partition_size_fd_stat(fd, &statbuf); > + byte_count = round_up(device_get_partition_size_fd_stat(fd, &statbuf), > + sectorsize); > source_dir_size = btrfs_mkfs_size_dir(source_dir, sectorsize, > min_dev_size, metadata_profile, data_profile); > if (byte_count < source_dir_size) { > if (S_ISREG(statbuf.st_mode)) { > - byte_count = source_dir_size; > + byte_count = round_up(source_dir_size, sectorsize); I believe we should round up not round down, if we're using --rootdir option. As smaller size would only be more possible to hit ENOSPC. Otherwise looks good to me. Thanks, Qu > } else { > warning( > "the target device %llu (%s) is smaller than the calculated source directory size %llu (%s), mkfs may fail",
在 2024/5/22 16:13, Qu Wenruo 写道: > > > 在 2024/5/22 15:32, Naohiro Aota 写道: >> While "byte_count" is eventually rounded down to sectorsize at >> make_btrfs() >> or btrfs_add_to_fs_id(), it would be better round it down first and do >> the >> size checks not to confuse the things. >> >> Also, on a zoned device, creating a btrfs whose size is not aligned to >> the >> zone boundary can be confusing. Round it down further to the zone >> boundary. >> >> The size calculation with a source directory is also tweaked to be >> aligned. >> >> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com> >> --- >> mkfs/main.c | 11 +++++++++-- >> 1 file changed, 9 insertions(+), 2 deletions(-) >> >> diff --git a/mkfs/main.c b/mkfs/main.c >> index a437ecc40c7f..baf889873b41 100644 >> --- a/mkfs/main.c >> +++ b/mkfs/main.c >> @@ -1591,6 +1591,12 @@ int BOX_MAIN(mkfs)(int argc, char **argv) >> min_dev_size = btrfs_min_dev_size(nodesize, mixed, >> opt_zoned ? zone_size(file) : 0, >> metadata_profile, data_profile); >> + if (byte_count) { >> + byte_count = round_down(byte_count, sectorsize); >> + if (opt_zoned) >> + byte_count = round_down(byte_count, zone_size(file)); >> + } >> + >> /* >> * Enlarge the destination file or create a new one, using the size >> * calculated from source dir. >> @@ -1624,12 +1630,13 @@ int BOX_MAIN(mkfs)(int argc, char **argv) >> * Or we will always use source_dir_size calculated for mkfs. >> */ >> if (!byte_count) >> - byte_count = device_get_partition_size_fd_stat(fd, >> &statbuf); >> + byte_count = >> round_up(device_get_partition_size_fd_stat(fd, &statbuf), >> + sectorsize); My bad, forgot this one too. We should round_down() here. As if we have a 512 bytes blocked device, and the partition is only aligned to 512 bytes, this can make the last sector go beyond device boundary. Thanks, Qu >> source_dir_size = btrfs_mkfs_size_dir(source_dir, sectorsize, >> min_dev_size, metadata_profile, data_profile); >> if (byte_count < source_dir_size) { >> if (S_ISREG(statbuf.st_mode)) { >> - byte_count = source_dir_size; >> + byte_count = round_up(source_dir_size, sectorsize); > > I believe we should round up not round down, if we're using --rootdir > option. > > As smaller size would only be more possible to hit ENOSPC. > > Otherwise looks good to me. > > Thanks, > Qu >> } else { >> warning( >> "the target device %llu (%s) is smaller than the calculated source >> directory size %llu (%s), mkfs may fail", >
On Wed, May 22, 2024 at 04:13:34PM GMT, Qu Wenruo wrote: > > > 在 2024/5/22 15:32, Naohiro Aota 写道: > > While "byte_count" is eventually rounded down to sectorsize at make_btrfs() > > or btrfs_add_to_fs_id(), it would be better round it down first and do the > > size checks not to confuse the things. > > > > Also, on a zoned device, creating a btrfs whose size is not aligned to the > > zone boundary can be confusing. Round it down further to the zone boundary. > > > > The size calculation with a source directory is also tweaked to be aligned. > > > > Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com> > > --- > > mkfs/main.c | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/mkfs/main.c b/mkfs/main.c > > index a437ecc40c7f..baf889873b41 100644 > > --- a/mkfs/main.c > > +++ b/mkfs/main.c > > @@ -1591,6 +1591,12 @@ int BOX_MAIN(mkfs)(int argc, char **argv) > > min_dev_size = btrfs_min_dev_size(nodesize, mixed, > > opt_zoned ? zone_size(file) : 0, > > metadata_profile, data_profile); > > + if (byte_count) { > > + byte_count = round_down(byte_count, sectorsize); > > + if (opt_zoned) > > + byte_count = round_down(byte_count, zone_size(file)); > > + } > > + > > /* > > * Enlarge the destination file or create a new one, using the size > > * calculated from source dir. > > @@ -1624,12 +1630,13 @@ int BOX_MAIN(mkfs)(int argc, char **argv) > > * Or we will always use source_dir_size calculated for mkfs. > > */ > > if (!byte_count) > > - byte_count = device_get_partition_size_fd_stat(fd, &statbuf); > > + byte_count = round_up(device_get_partition_size_fd_stat(fd, &statbuf), > > + sectorsize); > > source_dir_size = btrfs_mkfs_size_dir(source_dir, sectorsize, > > min_dev_size, metadata_profile, data_profile); > > if (byte_count < source_dir_size) { > > if (S_ISREG(statbuf.st_mode)) { > > - byte_count = source_dir_size; > > + byte_count = round_up(source_dir_size, sectorsize); > > I believe we should round up not round down, if we're using --rootdir > option. > > As smaller size would only be more possible to hit ENOSPC. > > Otherwise looks good to me. The commit log was vague about that, but actually the source dir calculations are rounded up in the code. Sorry for the confusion. Regards, > > Thanks, > Qu > > } else { > > warning( > > "the target device %llu (%s) is smaller than the calculated source directory size %llu (%s), mkfs may fail",
On Wed, May 22, 2024 at 06:53:44AM GMT, Naohiro Aota wrote: > On Wed, May 22, 2024 at 04:13:34PM GMT, Qu Wenruo wrote: > > > > > > 在 2024/5/22 15:32, Naohiro Aota 写道: > > > While "byte_count" is eventually rounded down to sectorsize at make_btrfs() > > > or btrfs_add_to_fs_id(), it would be better round it down first and do the > > > size checks not to confuse the things. > > > > > > Also, on a zoned device, creating a btrfs whose size is not aligned to the > > > zone boundary can be confusing. Round it down further to the zone boundary. > > > > > > The size calculation with a source directory is also tweaked to be aligned. > > > > > > Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com> > > > --- > > > mkfs/main.c | 11 +++++++++-- > > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > > > diff --git a/mkfs/main.c b/mkfs/main.c > > > index a437ecc40c7f..baf889873b41 100644 > > > --- a/mkfs/main.c > > > +++ b/mkfs/main.c > > > @@ -1591,6 +1591,12 @@ int BOX_MAIN(mkfs)(int argc, char **argv) > > > min_dev_size = btrfs_min_dev_size(nodesize, mixed, > > > opt_zoned ? zone_size(file) : 0, > > > metadata_profile, data_profile); > > > + if (byte_count) { > > > + byte_count = round_down(byte_count, sectorsize); > > > + if (opt_zoned) > > > + byte_count = round_down(byte_count, zone_size(file)); > > > + } > > > + > > > /* > > > * Enlarge the destination file or create a new one, using the size > > > * calculated from source dir. > > > @@ -1624,12 +1630,13 @@ int BOX_MAIN(mkfs)(int argc, char **argv) > > > * Or we will always use source_dir_size calculated for mkfs. > > > */ > > > if (!byte_count) > > > - byte_count = device_get_partition_size_fd_stat(fd, &statbuf); > > > + byte_count = round_up(device_get_partition_size_fd_stat(fd, &statbuf), > > > + sectorsize); > > > source_dir_size = btrfs_mkfs_size_dir(source_dir, sectorsize, > > > min_dev_size, metadata_profile, data_profile); > > > if (byte_count < source_dir_size) { > > > if (S_ISREG(statbuf.st_mode)) { > > > - byte_count = source_dir_size; > > > + byte_count = round_up(source_dir_size, sectorsize); > > > > I believe we should round up not round down, if we're using --rootdir > > option. > > > > As smaller size would only be more possible to hit ENOSPC. > > > > Otherwise looks good to me. > > The commit log was vague about that, but actually the source dir > calculations are rounded up in the code. Sorry for the confusion. Checking this line again. I think btrfs_mkfs_size_dir() returns a "sectorsize" aligned size in the first place. So, I think I can just drop this line diff. > > Regards, > > > > > Thanks, > > Qu > > > } else { > > > warning( > > > "the target device %llu (%s) is smaller than the calculated source directory size %llu (%s), mkfs may fail",
diff --git a/mkfs/main.c b/mkfs/main.c index a437ecc40c7f..baf889873b41 100644 --- a/mkfs/main.c +++ b/mkfs/main.c @@ -1591,6 +1591,12 @@ int BOX_MAIN(mkfs)(int argc, char **argv) min_dev_size = btrfs_min_dev_size(nodesize, mixed, opt_zoned ? zone_size(file) : 0, metadata_profile, data_profile); + if (byte_count) { + byte_count = round_down(byte_count, sectorsize); + if (opt_zoned) + byte_count = round_down(byte_count, zone_size(file)); + } + /* * Enlarge the destination file or create a new one, using the size * calculated from source dir. @@ -1624,12 +1630,13 @@ int BOX_MAIN(mkfs)(int argc, char **argv) * Or we will always use source_dir_size calculated for mkfs. */ if (!byte_count) - byte_count = device_get_partition_size_fd_stat(fd, &statbuf); + byte_count = round_up(device_get_partition_size_fd_stat(fd, &statbuf), + sectorsize); source_dir_size = btrfs_mkfs_size_dir(source_dir, sectorsize, min_dev_size, metadata_profile, data_profile); if (byte_count < source_dir_size) { if (S_ISREG(statbuf.st_mode)) { - byte_count = source_dir_size; + byte_count = round_up(source_dir_size, sectorsize); } else { warning( "the target device %llu (%s) is smaller than the calculated source directory size %llu (%s), mkfs may fail",
While "byte_count" is eventually rounded down to sectorsize at make_btrfs() or btrfs_add_to_fs_id(), it would be better round it down first and do the size checks not to confuse the things. Also, on a zoned device, creating a btrfs whose size is not aligned to the zone boundary can be confusing. Round it down further to the zone boundary. The size calculation with a source directory is also tweaked to be aligned. Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com> --- mkfs/main.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)