On 03/30, 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...@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.
So, I just integrated two patches in terms of this issue. Could you take a
look at this?
>From 9e615dd0a350d336a8067c64d9ef2bb7ce43a3e5 Mon Sep 17 00:00:00 2001
From: katao <ka...@xiaomi.com>
Date: Tue, 27 Mar 2018 13:25:46 +0800
Subject: [PATCH] libf2fs,mkfs.f2fs: reset wanted_total_sectors by new
sector_size
Use 512 bytes as the sector size criterion while specifying the
amount of sectors passed to mkfs.
Then, 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.
Signed-off-by: katao <ka...@xiaomi.com>
Signed-off-by: Junling Zheng <zhengjunl...@huawei.com>
Signed-off-by: Jaegeuk Kim <jaeg...@google.com>
---
lib/libf2fs.c | 21 ++++++++++++++++++---
man/mkfs.f2fs.8 | 2 +-
mkfs/f2fs_format_main.c | 2 +-
3 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/lib/libf2fs.c b/lib/libf2fs.c
index ffdbccb..af1ca65 100644
--- a/lib/libf2fs.c
+++ b/lib/libf2fs.c
@@ -812,8 +812,21 @@ 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.
+ */
+ if (c.wanted_total_sectors != -1) {
+ c.wanted_total_sectors =
+ (c.wanted_total_sectors *
+ dev->sector_size) / sector_size;
+ MSG(0, "Warn: change wanted sectors = %"PRIu64""
+ " (in %u bytes)\n",
+ c.wanted_total_sectors, sector_size);
+ }
dev->sector_size = sector_size;
+ }
#endif
#ifdef BLKGETSIZE64
if (ioctl(fd, BLKGETSIZE64, &dev->total_sectors) < 0) {
@@ -1008,9 +1021,11 @@ int f2fs_get_device_info(void)
if (get_device_info(i))
return -1;
- if (c.wanted_total_sectors < c.total_sectors) {
- MSG(0, "Info: total device sectors = %"PRIu64" (in %u bytes)\n",
+ MSG(0, "Info: total device sectors = %"PRIu64" (in %u bytes)\n",
c.total_sectors, c.sector_size);
+ if (c.wanted_total_sectors < c.total_sectors) {
+ MSG(0, "Info: wanted sectors = %"PRIu64" (in 512 bytes)\n",
+ c.wanted_total_sectors);
c.total_sectors = c.wanted_total_sectors;
c.devices[0].total_sectors = c.total_sectors;
}
diff --git a/man/mkfs.f2fs.8 b/man/mkfs.f2fs.8
index c2f9c86..e48ce39 100644
--- a/man/mkfs.f2fs.8
+++ b/man/mkfs.f2fs.8
@@ -63,7 +63,7 @@ mkfs.f2fs \- create an F2FS file system
is used to create a f2fs file system (usually in a disk partition).
\fIdevice\fP is the special file corresponding to the device (e.g.
\fI/dev/sdXX\fP).
-\fIsectors\fP is optionally given for specifing the filesystem size.
+\fIsectors\fP (in 512 bytes) is optionally given for specifing the filesystem
size.
.PP
The exit code returned by
.B mkfs.f2fs
diff --git a/mkfs/f2fs_format_main.c b/mkfs/f2fs_format_main.c
index e522b2f..f8361b2 100644
--- a/mkfs/f2fs_format_main.c
+++ b/mkfs/f2fs_format_main.c
@@ -55,7 +55,7 @@ static void mkfs_usage()
MSG(0, " -S sparse mode\n");
MSG(0, " -t 0: nodiscard, 1: discard [default:1]\n");
MSG(0, " -z # of sections per zone [default:1]\n");
- MSG(0, "sectors: number of sectors. [default: determined by device
size]\n");
+ MSG(0, "sectors: number of sectors (in 512 bytes). [default: determined
by device size]\n");
exit(1);
}
--
2.15.0.531.g2ccb3012c9-goog
------------------------------------------------------------------------------
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
