Re: [f2fs-dev] [PATCH] libf2fs: reset wanted_total_sectors by new sector_size
Brought to you by:
kjgkr
Menu
▾
▴
Re: [f2fs-dev] [PATCH] libf2fs: reset wanted_total_sectors by new sector_size
|
From: Jaegeuk K. <ja...@ke...> - 2018-04-02 20:03:45
|
On 04/02, Chao Yu wrote: > On 2018/3/30 23:39, Jaegeuk Kim wrote: > > On 03/30, Junling Zheng wrote: > >> On 2018/3/30 19:26, Chao Yu wrote: > >>> On 2018/3/30 18:51, Junling Zheng wrote: > >>>> Hi, > >>>> > >>>> On 2018/3/30 17:28, Chao Yu wrote: > >>>>> Hi All, > >>>>> > >>>>> On 2018/3/28 1:19, Jaegeuk Kim wrote: > >>>>>> From: katao <ka...@xi...> > >>>>>> > >>>>>> The args of wanted_total_sectors is calculated based > >>>>>> on the DEFAULT_SECTOR_SIZE(512Bytes).get_device_info(i) > >>>>>> may be reset dev_sector_size, we should reset the number > >>>>>> of wanted_total_sectors. > >>>>>> > >>>>>> This bug was reported to Google Issue Tracker. > >>>>>> Link: https://issuetracker.google.com/issues/76407663 > >>>>> > >>>>> I don't think this is the right way, since now we have changed previous > >>>>> sector_counter's meaning, some applications, for example, like xfstests will get > >>>>> device's real sector size via blockdev --getsize64, then calculate total wanted > >>>>> sector count by total_wanted_size / real_sector_size, if we changed default > >>>>> sector size to 512bytes, xfstests will pass a wrong sector number, result in > >>>>> getting wrong partition size. > >>>>> > >>>>> For something worse, in order to get the correct sector number, we have to > >>>>> change the way of calculation method of xfstests for new mkfs, but how can > >>>>> xfstests know the current version of mkfs is new or old... > >>>>> > >>>>> I think the change didn't consider backward compatibility of mkfs, so, in order > >>>>> to keep that, we'd better to let user pass the right sector number based on > >>>>> their device, or we can introduce a new parameter to indicate user wanted total > >>>>> size. > >>>>> > >>>>> How do you think? > >>>>> > >>>> > >>>> Agree. It's not backward-compatible. Most users can pass the correct sector number > >>>> calculated by the real sector size. For those very few users using 512B despite of > >>>> the actual sector size, all we need to do is informing them the real sector size. > >>> > >>> The problem is via passed sector number, we can't know user has already knew the > >>> real sector size or not, so we don't have any chance to info them. > >>> > >> > >> Yeah, we can't guess users' behaviors. And only when wanted size is over device size, > >> we can inform users the real sector size. > > > > So, current problem would be that wanted_total_sectors is given by: > > - device sector size in xfstests > > - 4KB in fs_mgr in Android > > - 512B in recovery in Android > > > > And, my concern is why user always needs to think about sector_size to give > > target image size, and I thought 512B would be good to set as a default smallest > > value. > > Of course, IMO, the most direct way is let user pass image size to mkfs.f2fs > instead of caculated sector size, I think that will make all happy. No, it can become too large number. > > > > > Setting it 512B by default can give some confusion to users tho, it won't hurt > > the existing partitions or images. So, I bet users will get noticed quickly, > > when formatting new partition under this rule. > > For those f2fs users, who makes image based on non-512B sector device, once they > upgrade mkfs.f2fs, they will encounter this issue, that may make bad reputation. > > For interface in f2fs private ioctl, sysfs entry, or syscall, we keep the rule > that once we want to do some change on it, we will not change the old interface > directly, instead, introduce a new one to keep backward compatibility of old > one. Still, I hope we can keep that rule in mkfs.f2fs parameter. Okay, so in order to give more flexible ways, it'd be fine to add wanted_sector_size by "-w" so that we could blow out all the existing concern like this. From: katao <ka...@xi...> Date: Tue, 27 Mar 2018 13:25:46 +0800 Subject: [PATCH] libf2fs,mkfs.f2fs: add wanted_sector_size for wanted_total_sectors The wanted_total_sectors was determined by device sector size, but sometimes we don't know precise sector_size by default. So, let's give wanted_sector_size in such the ambiguous situation. Signed-off-by: katao <ka...@xi...> Signed-off-by: Junling Zheng <zhe...@hu...> Signed-off-by: Jaegeuk Kim <ja...@go...> --- include/f2fs_fs.h | 1 + lib/libf2fs.c | 13 +++++++++++++ man/mkfs.f2fs.8 | 8 ++++++++ mkfs/f2fs_format_main.c | 6 +++++- 4 files changed, 27 insertions(+), 1 deletion(-) diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h index 053ccbe..2d75d39 100644 --- a/include/f2fs_fs.h +++ b/include/f2fs_fs.h @@ -335,6 +335,7 @@ struct f2fs_configuration { u_int64_t device_size; u_int64_t total_sectors; u_int64_t wanted_total_sectors; + u_int64_t wanted_sector_size; u_int64_t target_sectors; u_int32_t sectors_per_blk; u_int32_t blks_per_seg; diff --git a/lib/libf2fs.c b/lib/libf2fs.c index ffdbccb..bb7fe2e 100644 --- a/lib/libf2fs.c +++ b/lib/libf2fs.c @@ -593,6 +593,7 @@ void f2fs_init_configuration(void) c.blks_per_seg = DEFAULT_BLOCKS_PER_SEGMENT; c.rootdev_name = get_rootdev(); c.wanted_total_sectors = -1; + c.wanted_sector_size = -1; c.zoned_mode = 0; c.zoned_model = 0; c.zone_blocks = 0; @@ -900,6 +901,18 @@ int get_device_info(int i) dev->zone_blocks); } #endif + /* adjust wanted_total_sectors */ + if (c.wanted_total_sectors != -1) { + MSG(0, "Info: wanted sectors = %"PRIu64" (in %"PRIu64" bytes)\n", + c.wanted_total_sectors, c.wanted_sector_size); + if (c.wanted_sector_size == -1) { + c.wanted_sector_size = dev->sector_size; + } else if (dev->sector_size != c.wanted_sector_size) { + c.wanted_total_sectors *= c.wanted_sector_size; + c.wanted_total_sectors /= dev->sector_size; + } + } + c.total_sectors += dev->total_sectors; return 0; } diff --git a/man/mkfs.f2fs.8 b/man/mkfs.f2fs.8 index c2f9c86..442c0ea 100644 --- a/man/mkfs.f2fs.8 +++ b/man/mkfs.f2fs.8 @@ -53,6 +53,10 @@ mkfs.f2fs \- create an F2FS file system .I nodiscard/discard ] [ +.B \-w +.I specific sector_size for target sectors +] +[ .B \-z .I #-of-sections-per-zone ] @@ -125,6 +129,10 @@ Specify 1 or 0 to enable/disable discard policy. If the value is equal to 1, discard policy is enabled, otherwise is disable. The default value is 1. .TP +.BI \-w "sector-size" +Specify the sector size in bytes along with given target sectors. +Without it, the sectors will be calculated by device sector size. +.TP .BI \-z " #-of-sections-per-zone" Specify the number of sections per zone. A zone consists of multiple sections. F2FS allocates segments for active logs with separated zones as much as possible. diff --git a/mkfs/f2fs_format_main.c b/mkfs/f2fs_format_main.c index e522b2f..19f0854 100644 --- a/mkfs/f2fs_format_main.c +++ b/mkfs/f2fs_format_main.c @@ -54,6 +54,7 @@ static void mkfs_usage() MSG(0, " -s # of segments per section [default:1]\n"); MSG(0, " -S sparse mode\n"); MSG(0, " -t 0: nodiscard, 1: discard [default:1]\n"); + MSG(0, " -w wanted sector size\n"); MSG(0, " -z # of sections per zone [default:1]\n"); MSG(0, "sectors: number of sectors. [default: determined by device size]\n"); exit(1); @@ -102,7 +103,7 @@ static void parse_feature(const char *features) static void f2fs_parse_options(int argc, char *argv[]) { - static const char *option_string = "qa:c:d:e:l:mo:O:s:S:z:t:f"; + static const char *option_string = "qa:c:d:e:l:mo:O:s:S:z:t:fw:"; int32_t option=0; while ((option = getopt(argc,argv,option_string)) != EOF) { @@ -166,6 +167,9 @@ static void f2fs_parse_options(int argc, char *argv[]) case 'f': force_overwrite = 1; break; + case 'w': + c.wanted_sector_size = atoi(optarg); + break; default: MSG(0, "\tError: Unknown option %c\n",option); mkfs_usage(); -- 2.15.0.531.g2ccb3012c9-goog |
