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
