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...@xiaomi.com> > >>>> > >>>> 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. 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 xfstests, we're able to add another testcase in /f2fs to check this is applied or not by simply testing some wanted_total_sectors numbers on different sector sizes. Thanks, > > > Thanks, > > > >> > >> Thanks, > >> Junling > >> > >>> Thanks, > >>> > >>>> > >>>> Signed-off-by: katao <ka...@xiaomi.com> > >>>> Signed-off-by: Jaegeuk Kim <jaeg...@google.com> > >>>> --- > >>>> lib/libf2fs.c | 9 ++++++++- > >>>> 1 file changed, 8 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/lib/libf2fs.c b/lib/libf2fs.c > >>>> index 0c684d5..5f11796 100644 > >>>> --- a/lib/libf2fs.c > >>>> +++ b/lib/libf2fs.c > >>>> @@ -799,8 +799,15 @@ int get_device_info(int i) > >>>> #ifdef BLKSSZGET > >>>> if (ioctl(fd, BLKSSZGET, §or_size) < 0) > >>>> MSG(0, "\tError: Using the default sector > >>>> size\n"); > >>>> - else if (dev->sector_size < sector_size) > >>>> + else if (dev->sector_size < sector_size){ > >>>> + /* > >>>> + * wanted_total_sectors need to be reset by new > >>>> + * sector_size. > >>>> + */ > >>>> + c.wanted_total_sectors = > >>>> (c.wanted_total_sectors * > >>>> + dev->sector_size) / > >>>> sector_size; > >>>> dev->sector_size = sector_size; > >>>> + } > >>>> #endif > >>>> #ifdef BLKGETSIZE64 > >>>> if (ioctl(fd, BLKGETSIZE64, &dev->total_sectors) < 0) { > >>>> > >>> > >>> > >>> . > >>> > >> > >> > >> > >> . > >> > > > > > > . > > > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel