Re: [f2fs-dev] [PATCH] libf2fs: reset wanted_total_sectors by new sector_size

2018-04-02 Thread Chao Yu 
On 2018/4/3 4:03, Jaegeuk Kim wrote:
> 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 

 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.

That's better.

> 
> From: katao 
> 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 
> Signed-off-by: Junling Zheng 
> Signed-off-by: Jaegeuk Kim 

Reviewed-by: Chao Yu 

Thanks,


--
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

Re: [f2fs-dev] [PATCH] libf2fs: reset wanted_total_sectors by new sector_size

2018-04-02 Thread Jaegeuk Kim 
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 
> >>
> >> 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 
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 
Signed-off-by: Junling Zheng 
Signed-off-by: Jaegeuk Kim 
---
 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;

Re: [f2fs-dev] [PATCH] libf2fs: reset wanted_total_sectors by new sector_size

2018-04-02 Thread Chao Yu 
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 
>>
>> 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.

> 
> 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.

> 
> 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.

But, for people who upgrade mkfs but not the newly introduced testcase will
encounter this issue.

I just make xfstests as an example, I doubt there will be more user/application
on phone/server/pc scenario may be impacted by this.

Thanks,

> 
> Thanks,
> 
>>
>>> Thanks,
>>>

 Thanks,
 Junling

> Thanks,
>
>>
>> Signed-off-by: katao 
>> Signed-off-by: Jaegeuk Kim 
>> ---
>>  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, _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 *

Re: [f2fs-dev] [PATCH] libf2fs: reset wanted_total_sectors by new sector_size

2018-03-30 Thread Jaegeuk Kim 
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 
> > 
> >  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 
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 
Signed-off-by: Junling Zheng 
Signed-off-by: Jaegeuk Kim 
---
 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, _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);
+

Re: [f2fs-dev] [PATCH] libf2fs: reset wanted_total_sectors by new sector_size

2018-03-30 Thread Jaegeuk Kim 
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 
> 
>  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 
>  Signed-off-by: Jaegeuk Kim 
>  ---
>   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, _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, >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

Re: [f2fs-dev] [PATCH] libf2fs: reset wanted_total_sectors by new sector_size

2018-03-30 Thread Junling Zheng 
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 

 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.

> Thanks,
> 
>>
>> Thanks,
>> Junling
>>
>>> Thanks,
>>>

 Signed-off-by: katao 
 Signed-off-by: Jaegeuk Kim 
 ---
  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, _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, >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

Re: [f2fs-dev] [PATCH] libf2fs: reset wanted_total_sectors by new sector_size

2018-03-30 Thread Chao Yu 
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 
>>>
>>> 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.

Thanks,

> 
> Thanks,
> Junling
> 
>> Thanks,
>>
>>>
>>> Signed-off-by: katao 
>>> Signed-off-by: Jaegeuk Kim 
>>> ---
>>>  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, _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, >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

Re: [f2fs-dev] [PATCH] libf2fs: reset wanted_total_sectors by new sector_size

2018-03-30 Thread Junling Zheng 
Hi,

On 2018/3/30 17:28, Chao Yu wrote:
> Hi All,
> 
> On 2018/3/28 1:19, Jaegeuk Kim wrote:
>> From: katao 
>>
>> 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.

Thanks,
Junling

> Thanks,
> 
>>
>> Signed-off-by: katao 
>> Signed-off-by: Jaegeuk Kim 
>> ---
>>  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, _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, >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

Re: [f2fs-dev] [PATCH] libf2fs: reset wanted_total_sectors by new sector_size

2018-03-30 Thread Chao Yu 
Hi All,

On 2018/3/28 1:19, Jaegeuk Kim wrote:
> From: katao 
> 
> 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?

Thanks,

> 
> Signed-off-by: katao 
> Signed-off-by: Jaegeuk Kim 
> ---
>  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, _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, >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

Re: [f2fs-dev] [PATCH] libf2fs: reset wanted_total_sectors by new sector_size

2018-03-27 Thread Junling Zheng 
Hi, Jaegeuk:

On 2018/3/28 1:19, Jaegeuk Kim wrote:
> From: katao 
> 
> 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
> 

This fix is puzzling...

IMO, we should not recalculate wanted_total_sectors here.
c.wanted_total_sectors is got from user in f2fs_parse_options.
Users must know the sector size of device they use, and they need to
pass a correct wanted_total_sectors to mkfs.f2fs.

Thanks,
Junling

> Signed-off-by: katao 
> Signed-off-by: Jaegeuk Kim 
> ---
>  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, _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, >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

[f2fs-dev] [PATCH] libf2fs: reset wanted_total_sectors by new sector_size

2018-03-27 Thread Jaegeuk Kim 
From: katao 

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

Signed-off-by: katao 
Signed-off-by: Jaegeuk Kim 
---
 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, _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, >total_sectors) < 0) {
-- 
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