Linux-f2fs-devel Archive on lore.kernel.org
 help / color / Atom feed
* [f2fs-dev] [PATCH] mkfs.f2fs: check zeros in first 16MB for Android
@ 2019-08-08 23:11 Jaegeuk Kim
  2019-08-09 15:12 ` [f2fs-dev] [PATCH v2] " Jaegeuk Kim
  0 siblings, 1 reply; 9+ messages in thread
From: Jaegeuk Kim @ 2019-08-08 23:11 UTC (permalink / raw)
  To: linux-f2fs-devel; +Cc: Jaegeuk Kim

We actually don't need to issue trim on entire disk by checking first
blocks having zeros.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 mkfs/f2fs_format_utils.c | 47 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/mkfs/f2fs_format_utils.c b/mkfs/f2fs_format_utils.c
index 8bf128c..52fedc7 100644
--- a/mkfs/f2fs_format_utils.c
+++ b/mkfs/f2fs_format_utils.c
@@ -110,13 +110,56 @@ static int trim_device(int i)
 	return 0;
 }
 
+static int f2fs_zero_blocks(int i)
+{
+#ifdef WITH_ANDROID
+	struct device_info *dev = c.devices + i;
+	int fd = dev->fd;
+	char buf[F2FS_BLKSIZE];
+	char *zero_buf;
+	int j, ret;
+	int ret2 = 0;
+
+	zero_buf = calloc(1, F2FS_BLKSIZE);
+	if (zero_buf == NULL) {
+		MSG(1, "\tError: Malloc Failed for zero buf!!!\n");
+		return -1;
+	}
+
+	/* check first 16MB blocks */
+	for (j = 0; j < 4096; j++) {
+		ret = lseek(fd, j * F2FS_BLKSIZE, SEEK_SET);
+		if (ret < 0) {
+			ret2 = -1;
+			break;
+		}
+		ret = read(fd, buf, F2FS_BLKSIZE);
+		if (ret != F2FS_BLKSIZE) {
+			ret2 = -1;
+			break;
+		}
+		if (memcmp(buf, zero_buf, F2FS_BLKSIZE)) {
+			ret2 = -1;
+			break;
+		}
+	}
+	free(zero_buf);
+	if (!ret2)
+		MSG(0, "Info: Skip discarding blocks (found all zeros\n");
+	return ret2;
+#else
+	return -1;
+#endif
+}
+
 int f2fs_trim_devices(void)
 {
 	int i;
 
-	for (i = 0; i < c.ndevs; i++)
-		if (trim_device(i))
+	for (i = 0; i < c.ndevs; i++) {
+		if (f2fs_zero_blocks(i) && trim_device(i))
 			return -1;
+	}
 	c.trimmed = 1;
 	return 0;
 }
-- 
2.19.0.605.g01d371f741-goog



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [f2fs-dev] [PATCH v2] mkfs.f2fs: check zeros in first 16MB for Android
  2019-08-08 23:11 [f2fs-dev] [PATCH] mkfs.f2fs: check zeros in first 16MB for Android Jaegeuk Kim
@ 2019-08-09 15:12 ` " Jaegeuk Kim
       [not found]   ` <CAD14+f2V=j8o=0sUGMgmJHmwKgm80WyzJC5yW7qmyffL=CBJhw@mail.gmail.com>
  2019-08-12  2:19   ` Chao Yu
  0 siblings, 2 replies; 9+ messages in thread
From: Jaegeuk Kim @ 2019-08-09 15:12 UTC (permalink / raw)
  To: linux-f2fs-devel

We actually don't need to issue trim on entire disk by checking first
blocks having zeros.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
v2 from v1:
 - clean up

 mkfs/f2fs_format_utils.c | 53 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/mkfs/f2fs_format_utils.c b/mkfs/f2fs_format_utils.c
index 8bf128c..f2d55ad 100644
--- a/mkfs/f2fs_format_utils.c
+++ b/mkfs/f2fs_format_utils.c
@@ -25,6 +25,7 @@
 #include <stdio.h>
 #include <unistd.h>
 #include <stdlib.h>
+#include <stdbool.h>
 #ifndef ANDROID_WINDOWS_HOST
 #include <sys/ioctl.h>
 #endif
@@ -110,13 +111,61 @@ static int trim_device(int i)
 	return 0;
 }
 
+static bool is_wiped_device(int i)
+{
+#ifdef WITH_ANDROID
+	struct device_info *dev = c.devices + i;
+	int fd = dev->fd;
+	char *buf, *zero_buf;
+	bool wiped = true;
+	int nblocks = 4096;	/* 16MB size */
+	int j;
+
+	buf = malloc(F2FS_BLKSIZE);
+	if (buf == NULL) {
+		MSG(1, "\tError: Malloc Failed for buf!!!\n");
+		return false;
+	}
+	zero_buf = calloc(1, F2FS_BLKSIZE);
+	if (zero_buf == NULL) {
+		MSG(1, "\tError: Calloc Failed for zero buf!!!\n");
+		free(buf);
+		return false;
+	}
+
+	if (lseek(fd, 0, SEEK_SET) < 0) {
+		free(zero_buf);
+		free(buf);
+		return false;
+	}
+
+	/* check first n blocks */
+	for (j = 0; j < nblocks; j++) {
+		if (read(fd, buf, F2FS_BLKSIZE) != F2FS_BLKSIZE ||
+				memcmp(buf, zero_buf, F2FS_BLKSIZE)) {
+			wiped = false;
+			break;
+		}
+	}
+	free(zero_buf);
+	free(buf);
+
+	if (wiped)
+		MSG(0, "Info: Found all zeros in first %d blocks\n", nblocks);
+	return wiped;
+#else
+	return false;
+#endif
+}
+
 int f2fs_trim_devices(void)
 {
 	int i;
 
-	for (i = 0; i < c.ndevs; i++)
-		if (trim_device(i))
+	for (i = 0; i < c.ndevs; i++) {
+		if (!is_wiped_device(i) && trim_device(i))
 			return -1;
+	}
 	c.trimmed = 1;
 	return 0;
 }
-- 
2.19.0.605.g01d371f741-goog



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [f2fs-dev] [PATCH v2] mkfs.f2fs: check zeros in first 16MB for Android
       [not found]   ` <CAD14+f2V=j8o=0sUGMgmJHmwKgm80WyzJC5yW7qmyffL=CBJhw@mail.gmail.com>
@ 2019-08-09 15:39     ` Jaegeuk Kim
  2019-08-12  3:26       ` Chao Yu
  0 siblings, 1 reply; 9+ messages in thread
From: Jaegeuk Kim @ 2019-08-09 15:39 UTC (permalink / raw)
  To: Ju Hyung Park; +Cc: linux-f2fs-devel

On 08/10, Ju Hyung Park wrote:
> Hi Jaegeuk,
> 
> Just out of curiosity, what's the point of this?
> 
> I thought flash chips skip erasing blocks if it's already erased to
> preserve P/E cycles as much as possible.
> All Android devices I had(various versions of eMMC and UFS) ran full range
> block-level discards pretty fast too.

Unfortunately, some of them are giving long delays on a bunch of unmap commands
resulting in user janky issue.

> 
> Thanks.
> 
> 2019년 8월 10일 (토) 오전 12:13, Jaegeuk Kim <jaegeuk@kernel.org>님이 작성:
> 
> > We actually don't need to issue trim on entire disk by checking first
> > blocks having zeros.
> >
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> > v2 from v1:
> >  - clean up
> >
> >  mkfs/f2fs_format_utils.c | 53 ++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 51 insertions(+), 2 deletions(-)
> >
> > diff --git a/mkfs/f2fs_format_utils.c b/mkfs/f2fs_format_utils.c
> > index 8bf128c..f2d55ad 100644
> > --- a/mkfs/f2fs_format_utils.c
> > +++ b/mkfs/f2fs_format_utils.c
> > @@ -25,6 +25,7 @@
> >  #include <stdio.h>
> >  #include <unistd.h>
> >  #include <stdlib.h>
> > +#include <stdbool.h>
> >  #ifndef ANDROID_WINDOWS_HOST
> >  #include <sys/ioctl.h>
> >  #endif
> > @@ -110,13 +111,61 @@ static int trim_device(int i)
> >         return 0;
> >  }
> >
> > +static bool is_wiped_device(int i)
> > +{
> > +#ifdef WITH_ANDROID
> > +       struct device_info *dev = c.devices + i;
> > +       int fd = dev->fd;
> > +       char *buf, *zero_buf;
> > +       bool wiped = true;
> > +       int nblocks = 4096;     /* 16MB size */
> > +       int j;
> > +
> > +       buf = malloc(F2FS_BLKSIZE);
> > +       if (buf == NULL) {
> > +               MSG(1, "\tError: Malloc Failed for buf!!!\n");
> > +               return false;
> > +       }
> > +       zero_buf = calloc(1, F2FS_BLKSIZE);
> > +       if (zero_buf == NULL) {
> > +               MSG(1, "\tError: Calloc Failed for zero buf!!!\n");
> > +               free(buf);
> > +               return false;
> > +       }
> > +
> > +       if (lseek(fd, 0, SEEK_SET) < 0) {
> > +               free(zero_buf);
> > +               free(buf);
> > +               return false;
> > +       }
> > +
> > +       /* check first n blocks */
> > +       for (j = 0; j < nblocks; j++) {
> > +               if (read(fd, buf, F2FS_BLKSIZE) != F2FS_BLKSIZE ||
> > +                               memcmp(buf, zero_buf, F2FS_BLKSIZE)) {
> > +                       wiped = false;
> > +                       break;
> > +               }
> > +       }
> > +       free(zero_buf);
> > +       free(buf);
> > +
> > +       if (wiped)
> > +               MSG(0, "Info: Found all zeros in first %d blocks\n",
> > nblocks);
> > +       return wiped;
> > +#else
> > +       return false;
> > +#endif
> > +}
> > +
> >  int f2fs_trim_devices(void)
> >  {
> >         int i;
> >
> > -       for (i = 0; i < c.ndevs; i++)
> > -               if (trim_device(i))
> > +       for (i = 0; i < c.ndevs; i++) {
> > +               if (!is_wiped_device(i) && trim_device(i))
> >                         return -1;
> > +       }
> >         c.trimmed = 1;
> >         return 0;
> >  }
> > --
> > 2.19.0.605.g01d371f741-goog
> >
> >
> >
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> >


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [f2fs-dev] [PATCH v2] mkfs.f2fs: check zeros in first 16MB for Android
  2019-08-09 15:12 ` [f2fs-dev] [PATCH v2] " Jaegeuk Kim
       [not found]   ` <CAD14+f2V=j8o=0sUGMgmJHmwKgm80WyzJC5yW7qmyffL=CBJhw@mail.gmail.com>
@ 2019-08-12  2:19   ` Chao Yu
  2019-08-15 22:21     ` Jaegeuk Kim
  1 sibling, 1 reply; 9+ messages in thread
From: Chao Yu @ 2019-08-12  2:19 UTC (permalink / raw)
  To: Jaegeuk Kim, linux-f2fs-devel

On 2019/8/9 23:12, Jaegeuk Kim wrote:
> We actually don't need to issue trim on entire disk by checking first
> blocks having zeros.

In heap mode, we locate node log header to tail end of device, should we
consider to check block contain according to heap option?

BTW, if we changed cp_ver whenever mkfs, why should we still issue trim to
obsolete old data in node remained in image?

Thanks,

> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
> v2 from v1:
>  - clean up
> 
>  mkfs/f2fs_format_utils.c | 53 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 51 insertions(+), 2 deletions(-)
> 
> diff --git a/mkfs/f2fs_format_utils.c b/mkfs/f2fs_format_utils.c
> index 8bf128c..f2d55ad 100644
> --- a/mkfs/f2fs_format_utils.c
> +++ b/mkfs/f2fs_format_utils.c
> @@ -25,6 +25,7 @@
>  #include <stdio.h>
>  #include <unistd.h>
>  #include <stdlib.h>
> +#include <stdbool.h>
>  #ifndef ANDROID_WINDOWS_HOST
>  #include <sys/ioctl.h>
>  #endif
> @@ -110,13 +111,61 @@ static int trim_device(int i)
>  	return 0;
>  }
>  
> +static bool is_wiped_device(int i)
> +{
> +#ifdef WITH_ANDROID
> +	struct device_info *dev = c.devices + i;
> +	int fd = dev->fd;
> +	char *buf, *zero_buf;
> +	bool wiped = true;
> +	int nblocks = 4096;	/* 16MB size */
> +	int j;
> +
> +	buf = malloc(F2FS_BLKSIZE);
> +	if (buf == NULL) {
> +		MSG(1, "\tError: Malloc Failed for buf!!!\n");
> +		return false;
> +	}
> +	zero_buf = calloc(1, F2FS_BLKSIZE);
> +	if (zero_buf == NULL) {
> +		MSG(1, "\tError: Calloc Failed for zero buf!!!\n");
> +		free(buf);
> +		return false;
> +	}
> +
> +	if (lseek(fd, 0, SEEK_SET) < 0) {
> +		free(zero_buf);
> +		free(buf);
> +		return false;
> +	}
> +
> +	/* check first n blocks */
> +	for (j = 0; j < nblocks; j++) {
> +		if (read(fd, buf, F2FS_BLKSIZE) != F2FS_BLKSIZE ||
> +				memcmp(buf, zero_buf, F2FS_BLKSIZE)) {
> +			wiped = false;
> +			break;
> +		}
> +	}
> +	free(zero_buf);
> +	free(buf);
> +
> +	if (wiped)
> +		MSG(0, "Info: Found all zeros in first %d blocks\n", nblocks);
> +	return wiped;
> +#else
> +	return false;
> +#endif
> +}
> +
>  int f2fs_trim_devices(void)
>  {
>  	int i;
>  
> -	for (i = 0; i < c.ndevs; i++)
> -		if (trim_device(i))
> +	for (i = 0; i < c.ndevs; i++) {
> +		if (!is_wiped_device(i) && trim_device(i))
>  			return -1;
> +	}
>  	c.trimmed = 1;
>  	return 0;
>  }
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [f2fs-dev] [PATCH v2] mkfs.f2fs: check zeros in first 16MB for Android
  2019-08-09 15:39     ` Jaegeuk Kim
@ 2019-08-12  3:26       ` Chao Yu
  0 siblings, 0 replies; 9+ messages in thread
From: Chao Yu @ 2019-08-12  3:26 UTC (permalink / raw)
  To: Jaegeuk Kim, Ju Hyung Park; +Cc: linux-f2fs-devel

On 2019/8/9 23:39, Jaegeuk Kim wrote:
> On 08/10, Ju Hyung Park wrote:
>> Hi Jaegeuk,
>>
>> Just out of curiosity, what's the point of this?
>>
>> I thought flash chips skip erasing blocks if it's already erased to
>> preserve P/E cycles as much as possible.
>> All Android devices I had(various versions of eMMC and UFS) ran full range
>> block-level discards pretty fast too.
> 
> Unfortunately, some of them are giving long delays on a bunch of unmap commands
> resulting in user janky issue.

Agreed, the performance completely depends on FTL implementation, some of them
perform very bad...

Thanks,

> 
>>
>> Thanks.
>>
>> 2019년 8월 10일 (토) 오전 12:13, Jaegeuk Kim <jaegeuk@kernel.org>님이 작성:
>>
>>> We actually don't need to issue trim on entire disk by checking first
>>> blocks having zeros.
>>>
>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>> ---
>>> v2 from v1:
>>>  - clean up
>>>
>>>  mkfs/f2fs_format_utils.c | 53 ++++++++++++++++++++++++++++++++++++++--
>>>  1 file changed, 51 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/mkfs/f2fs_format_utils.c b/mkfs/f2fs_format_utils.c
>>> index 8bf128c..f2d55ad 100644
>>> --- a/mkfs/f2fs_format_utils.c
>>> +++ b/mkfs/f2fs_format_utils.c
>>> @@ -25,6 +25,7 @@
>>>  #include <stdio.h>
>>>  #include <unistd.h>
>>>  #include <stdlib.h>
>>> +#include <stdbool.h>
>>>  #ifndef ANDROID_WINDOWS_HOST
>>>  #include <sys/ioctl.h>
>>>  #endif
>>> @@ -110,13 +111,61 @@ static int trim_device(int i)
>>>         return 0;
>>>  }
>>>
>>> +static bool is_wiped_device(int i)
>>> +{
>>> +#ifdef WITH_ANDROID
>>> +       struct device_info *dev = c.devices + i;
>>> +       int fd = dev->fd;
>>> +       char *buf, *zero_buf;
>>> +       bool wiped = true;
>>> +       int nblocks = 4096;     /* 16MB size */
>>> +       int j;
>>> +
>>> +       buf = malloc(F2FS_BLKSIZE);
>>> +       if (buf == NULL) {
>>> +               MSG(1, "\tError: Malloc Failed for buf!!!\n");
>>> +               return false;
>>> +       }
>>> +       zero_buf = calloc(1, F2FS_BLKSIZE);
>>> +       if (zero_buf == NULL) {
>>> +               MSG(1, "\tError: Calloc Failed for zero buf!!!\n");
>>> +               free(buf);
>>> +               return false;
>>> +       }
>>> +
>>> +       if (lseek(fd, 0, SEEK_SET) < 0) {
>>> +               free(zero_buf);
>>> +               free(buf);
>>> +               return false;
>>> +       }
>>> +
>>> +       /* check first n blocks */
>>> +       for (j = 0; j < nblocks; j++) {
>>> +               if (read(fd, buf, F2FS_BLKSIZE) != F2FS_BLKSIZE ||
>>> +                               memcmp(buf, zero_buf, F2FS_BLKSIZE)) {
>>> +                       wiped = false;
>>> +                       break;
>>> +               }
>>> +       }
>>> +       free(zero_buf);
>>> +       free(buf);
>>> +
>>> +       if (wiped)
>>> +               MSG(0, "Info: Found all zeros in first %d blocks\n",
>>> nblocks);
>>> +       return wiped;
>>> +#else
>>> +       return false;
>>> +#endif
>>> +}
>>> +
>>>  int f2fs_trim_devices(void)
>>>  {
>>>         int i;
>>>
>>> -       for (i = 0; i < c.ndevs; i++)
>>> -               if (trim_device(i))
>>> +       for (i = 0; i < c.ndevs; i++) {
>>> +               if (!is_wiped_device(i) && trim_device(i))
>>>                         return -1;
>>> +       }
>>>         c.trimmed = 1;
>>>         return 0;
>>>  }
>>> --
>>> 2.19.0.605.g01d371f741-goog
>>>
>>>
>>>
>>> _______________________________________________
>>> Linux-f2fs-devel mailing list
>>> Linux-f2fs-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>>
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [f2fs-dev] [PATCH v2] mkfs.f2fs: check zeros in first 16MB for Android
  2019-08-12  2:19   ` Chao Yu
@ 2019-08-15 22:21     ` Jaegeuk Kim
  2019-08-16  1:00       ` Chao Yu
  0 siblings, 1 reply; 9+ messages in thread
From: Jaegeuk Kim @ 2019-08-15 22:21 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel

On 08/12, Chao Yu wrote:
> On 2019/8/9 23:12, Jaegeuk Kim wrote:
> > We actually don't need to issue trim on entire disk by checking first
> > blocks having zeros.
> 
> In heap mode, we locate node log header to tail end of device, should we
> consider to check block contain according to heap option?

I wanted to check F2FS metadata mainly.

> 
> BTW, if we changed cp_ver whenever mkfs, why should we still issue trim to
> obsolete old data in node remained in image?

For simplicity. :P

> 
> Thanks,
> 
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> > v2 from v1:
> >  - clean up
> > 
> >  mkfs/f2fs_format_utils.c | 53 ++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 51 insertions(+), 2 deletions(-)
> > 
> > diff --git a/mkfs/f2fs_format_utils.c b/mkfs/f2fs_format_utils.c
> > index 8bf128c..f2d55ad 100644
> > --- a/mkfs/f2fs_format_utils.c
> > +++ b/mkfs/f2fs_format_utils.c
> > @@ -25,6 +25,7 @@
> >  #include <stdio.h>
> >  #include <unistd.h>
> >  #include <stdlib.h>
> > +#include <stdbool.h>
> >  #ifndef ANDROID_WINDOWS_HOST
> >  #include <sys/ioctl.h>
> >  #endif
> > @@ -110,13 +111,61 @@ static int trim_device(int i)
> >  	return 0;
> >  }
> >  
> > +static bool is_wiped_device(int i)
> > +{
> > +#ifdef WITH_ANDROID
> > +	struct device_info *dev = c.devices + i;
> > +	int fd = dev->fd;
> > +	char *buf, *zero_buf;
> > +	bool wiped = true;
> > +	int nblocks = 4096;	/* 16MB size */
> > +	int j;
> > +
> > +	buf = malloc(F2FS_BLKSIZE);
> > +	if (buf == NULL) {
> > +		MSG(1, "\tError: Malloc Failed for buf!!!\n");
> > +		return false;
> > +	}
> > +	zero_buf = calloc(1, F2FS_BLKSIZE);
> > +	if (zero_buf == NULL) {
> > +		MSG(1, "\tError: Calloc Failed for zero buf!!!\n");
> > +		free(buf);
> > +		return false;
> > +	}
> > +
> > +	if (lseek(fd, 0, SEEK_SET) < 0) {
> > +		free(zero_buf);
> > +		free(buf);
> > +		return false;
> > +	}
> > +
> > +	/* check first n blocks */
> > +	for (j = 0; j < nblocks; j++) {
> > +		if (read(fd, buf, F2FS_BLKSIZE) != F2FS_BLKSIZE ||
> > +				memcmp(buf, zero_buf, F2FS_BLKSIZE)) {
> > +			wiped = false;
> > +			break;
> > +		}
> > +	}
> > +	free(zero_buf);
> > +	free(buf);
> > +
> > +	if (wiped)
> > +		MSG(0, "Info: Found all zeros in first %d blocks\n", nblocks);
> > +	return wiped;
> > +#else
> > +	return false;
> > +#endif
> > +}
> > +
> >  int f2fs_trim_devices(void)
> >  {
> >  	int i;
> >  
> > -	for (i = 0; i < c.ndevs; i++)
> > -		if (trim_device(i))
> > +	for (i = 0; i < c.ndevs; i++) {
> > +		if (!is_wiped_device(i) && trim_device(i))
> >  			return -1;
> > +	}
> >  	c.trimmed = 1;
> >  	return 0;
> >  }
> > 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [f2fs-dev] [PATCH v2] mkfs.f2fs: check zeros in first 16MB for Android
  2019-08-15 22:21     ` Jaegeuk Kim
@ 2019-08-16  1:00       ` Chao Yu
  2019-08-16  1:02         ` Jaegeuk Kim
  0 siblings, 1 reply; 9+ messages in thread
From: Chao Yu @ 2019-08-16  1:00 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel

On 2019/8/16 6:21, Jaegeuk Kim wrote:
> On 08/12, Chao Yu wrote:
>> On 2019/8/9 23:12, Jaegeuk Kim wrote:
>>> We actually don't need to issue trim on entire disk by checking first
>>> blocks having zeros.
>>
>> In heap mode, we locate node log header to tail end of device, should we
>> consider to check block contain according to heap option?
> 
> I wanted to check F2FS metadata mainly.

Oh, I thought you mean main area. :P

> 
>>
>> BTW, if we changed cp_ver whenever mkfs, why should we still issue trim to
>> obsolete old data in node remained in image?
> 
> For simplicity. :P

I didn't get why we can assume all metadata are zeroed if first 16MB are all zero...

BTW, if first 16MB are non-zero, why not just trim F2FS metadata rather than
whole area?

Thanks,

> 
>>
>> Thanks,
>>
>>>
>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>> ---
>>> v2 from v1:
>>>  - clean up
>>>
>>>  mkfs/f2fs_format_utils.c | 53 ++++++++++++++++++++++++++++++++++++++--
>>>  1 file changed, 51 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/mkfs/f2fs_format_utils.c b/mkfs/f2fs_format_utils.c
>>> index 8bf128c..f2d55ad 100644
>>> --- a/mkfs/f2fs_format_utils.c
>>> +++ b/mkfs/f2fs_format_utils.c
>>> @@ -25,6 +25,7 @@
>>>  #include <stdio.h>
>>>  #include <unistd.h>
>>>  #include <stdlib.h>
>>> +#include <stdbool.h>
>>>  #ifndef ANDROID_WINDOWS_HOST
>>>  #include <sys/ioctl.h>
>>>  #endif
>>> @@ -110,13 +111,61 @@ static int trim_device(int i)
>>>  	return 0;
>>>  }
>>>  
>>> +static bool is_wiped_device(int i)
>>> +{
>>> +#ifdef WITH_ANDROID
>>> +	struct device_info *dev = c.devices + i;
>>> +	int fd = dev->fd;
>>> +	char *buf, *zero_buf;
>>> +	bool wiped = true;
>>> +	int nblocks = 4096;	/* 16MB size */
>>> +	int j;
>>> +
>>> +	buf = malloc(F2FS_BLKSIZE);
>>> +	if (buf == NULL) {
>>> +		MSG(1, "\tError: Malloc Failed for buf!!!\n");
>>> +		return false;
>>> +	}
>>> +	zero_buf = calloc(1, F2FS_BLKSIZE);
>>> +	if (zero_buf == NULL) {
>>> +		MSG(1, "\tError: Calloc Failed for zero buf!!!\n");
>>> +		free(buf);
>>> +		return false;
>>> +	}
>>> +
>>> +	if (lseek(fd, 0, SEEK_SET) < 0) {
>>> +		free(zero_buf);
>>> +		free(buf);
>>> +		return false;
>>> +	}
>>> +
>>> +	/* check first n blocks */
>>> +	for (j = 0; j < nblocks; j++) {
>>> +		if (read(fd, buf, F2FS_BLKSIZE) != F2FS_BLKSIZE ||
>>> +				memcmp(buf, zero_buf, F2FS_BLKSIZE)) {
>>> +			wiped = false;
>>> +			break;
>>> +		}
>>> +	}
>>> +	free(zero_buf);
>>> +	free(buf);
>>> +
>>> +	if (wiped)
>>> +		MSG(0, "Info: Found all zeros in first %d blocks\n", nblocks);
>>> +	return wiped;
>>> +#else
>>> +	return false;
>>> +#endif
>>> +}
>>> +
>>>  int f2fs_trim_devices(void)
>>>  {
>>>  	int i;
>>>  
>>> -	for (i = 0; i < c.ndevs; i++)
>>> -		if (trim_device(i))
>>> +	for (i = 0; i < c.ndevs; i++) {
>>> +		if (!is_wiped_device(i) && trim_device(i))
>>>  			return -1;
>>> +	}
>>>  	c.trimmed = 1;
>>>  	return 0;
>>>  }
>>>
> .
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [f2fs-dev] [PATCH v2] mkfs.f2fs: check zeros in first 16MB for Android
  2019-08-16  1:00       ` Chao Yu
@ 2019-08-16  1:02         ` Jaegeuk Kim
  2019-08-16  1:23           ` Chao Yu
  0 siblings, 1 reply; 9+ messages in thread
From: Jaegeuk Kim @ 2019-08-16  1:02 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel

On 08/16, Chao Yu wrote:
> On 2019/8/16 6:21, Jaegeuk Kim wrote:
> > On 08/12, Chao Yu wrote:
> >> On 2019/8/9 23:12, Jaegeuk Kim wrote:
> >>> We actually don't need to issue trim on entire disk by checking first
> >>> blocks having zeros.
> >>
> >> In heap mode, we locate node log header to tail end of device, should we
> >> consider to check block contain according to heap option?
> > 
> > I wanted to check F2FS metadata mainly.
> 
> Oh, I thought you mean main area. :P
> 
> > 
> >>
> >> BTW, if we changed cp_ver whenever mkfs, why should we still issue trim to
> >> obsolete old data in node remained in image?
> > 
> > For simplicity. :P
> 
> I didn't get why we can assume all metadata are zeroed if first 16MB are all zero...
> 
> BTW, if first 16MB are non-zero, why not just trim F2FS metadata rather than
> whole area?

Trim the entire space so that we can skip discard in runtime by the flag, right?

> 
> Thanks,
> 
> > 
> >>
> >> Thanks,
> >>
> >>>
> >>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> >>> ---
> >>> v2 from v1:
> >>>  - clean up
> >>>
> >>>  mkfs/f2fs_format_utils.c | 53 ++++++++++++++++++++++++++++++++++++++--
> >>>  1 file changed, 51 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/mkfs/f2fs_format_utils.c b/mkfs/f2fs_format_utils.c
> >>> index 8bf128c..f2d55ad 100644
> >>> --- a/mkfs/f2fs_format_utils.c
> >>> +++ b/mkfs/f2fs_format_utils.c
> >>> @@ -25,6 +25,7 @@
> >>>  #include <stdio.h>
> >>>  #include <unistd.h>
> >>>  #include <stdlib.h>
> >>> +#include <stdbool.h>
> >>>  #ifndef ANDROID_WINDOWS_HOST
> >>>  #include <sys/ioctl.h>
> >>>  #endif
> >>> @@ -110,13 +111,61 @@ static int trim_device(int i)
> >>>  	return 0;
> >>>  }
> >>>  
> >>> +static bool is_wiped_device(int i)
> >>> +{
> >>> +#ifdef WITH_ANDROID
> >>> +	struct device_info *dev = c.devices + i;
> >>> +	int fd = dev->fd;
> >>> +	char *buf, *zero_buf;
> >>> +	bool wiped = true;
> >>> +	int nblocks = 4096;	/* 16MB size */
> >>> +	int j;
> >>> +
> >>> +	buf = malloc(F2FS_BLKSIZE);
> >>> +	if (buf == NULL) {
> >>> +		MSG(1, "\tError: Malloc Failed for buf!!!\n");
> >>> +		return false;
> >>> +	}
> >>> +	zero_buf = calloc(1, F2FS_BLKSIZE);
> >>> +	if (zero_buf == NULL) {
> >>> +		MSG(1, "\tError: Calloc Failed for zero buf!!!\n");
> >>> +		free(buf);
> >>> +		return false;
> >>> +	}
> >>> +
> >>> +	if (lseek(fd, 0, SEEK_SET) < 0) {
> >>> +		free(zero_buf);
> >>> +		free(buf);
> >>> +		return false;
> >>> +	}
> >>> +
> >>> +	/* check first n blocks */
> >>> +	for (j = 0; j < nblocks; j++) {
> >>> +		if (read(fd, buf, F2FS_BLKSIZE) != F2FS_BLKSIZE ||
> >>> +				memcmp(buf, zero_buf, F2FS_BLKSIZE)) {
> >>> +			wiped = false;
> >>> +			break;
> >>> +		}
> >>> +	}
> >>> +	free(zero_buf);
> >>> +	free(buf);
> >>> +
> >>> +	if (wiped)
> >>> +		MSG(0, "Info: Found all zeros in first %d blocks\n", nblocks);
> >>> +	return wiped;
> >>> +#else
> >>> +	return false;
> >>> +#endif
> >>> +}
> >>> +
> >>>  int f2fs_trim_devices(void)
> >>>  {
> >>>  	int i;
> >>>  
> >>> -	for (i = 0; i < c.ndevs; i++)
> >>> -		if (trim_device(i))
> >>> +	for (i = 0; i < c.ndevs; i++) {
> >>> +		if (!is_wiped_device(i) && trim_device(i))
> >>>  			return -1;
> >>> +	}
> >>>  	c.trimmed = 1;
> >>>  	return 0;
> >>>  }
> >>>
> > .
> > 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [f2fs-dev] [PATCH v2] mkfs.f2fs: check zeros in first 16MB for Android
  2019-08-16  1:02         ` Jaegeuk Kim
@ 2019-08-16  1:23           ` Chao Yu
  0 siblings, 0 replies; 9+ messages in thread
From: Chao Yu @ 2019-08-16  1:23 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel

On 2019/8/16 9:02, Jaegeuk Kim wrote:
> On 08/16, Chao Yu wrote:
>> On 2019/8/16 6:21, Jaegeuk Kim wrote:
>>> On 08/12, Chao Yu wrote:
>>>> On 2019/8/9 23:12, Jaegeuk Kim wrote:
>>>>> We actually don't need to issue trim on entire disk by checking first
>>>>> blocks having zeros.
>>>>
>>>> In heap mode, we locate node log header to tail end of device, should we
>>>> consider to check block contain according to heap option?
>>>
>>> I wanted to check F2FS metadata mainly.
>>
>> Oh, I thought you mean main area. :P
>>
>>>
>>>>
>>>> BTW, if we changed cp_ver whenever mkfs, why should we still issue trim to
>>>> obsolete old data in node remained in image?
>>>
>>> For simplicity. :P
>>
>> I didn't get why we can assume all metadata are zeroed if first 16MB are all zero...
>>
>> BTW, if first 16MB are non-zero, why not just trim F2FS metadata rather than
>> whole area?
> 
> Trim the entire space so that we can skip discard in runtime by the flag, right?

You're right, thanks helping recall. ;)

Thanks,

> 
>>
>> Thanks,
>>
>>>
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>>>> ---
>>>>> v2 from v1:
>>>>>  - clean up
>>>>>
>>>>>  mkfs/f2fs_format_utils.c | 53 ++++++++++++++++++++++++++++++++++++++--
>>>>>  1 file changed, 51 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/mkfs/f2fs_format_utils.c b/mkfs/f2fs_format_utils.c
>>>>> index 8bf128c..f2d55ad 100644
>>>>> --- a/mkfs/f2fs_format_utils.c
>>>>> +++ b/mkfs/f2fs_format_utils.c
>>>>> @@ -25,6 +25,7 @@
>>>>>  #include <stdio.h>
>>>>>  #include <unistd.h>
>>>>>  #include <stdlib.h>
>>>>> +#include <stdbool.h>
>>>>>  #ifndef ANDROID_WINDOWS_HOST
>>>>>  #include <sys/ioctl.h>
>>>>>  #endif
>>>>> @@ -110,13 +111,61 @@ static int trim_device(int i)
>>>>>  	return 0;
>>>>>  }
>>>>>  
>>>>> +static bool is_wiped_device(int i)
>>>>> +{
>>>>> +#ifdef WITH_ANDROID
>>>>> +	struct device_info *dev = c.devices + i;
>>>>> +	int fd = dev->fd;
>>>>> +	char *buf, *zero_buf;
>>>>> +	bool wiped = true;
>>>>> +	int nblocks = 4096;	/* 16MB size */
>>>>> +	int j;
>>>>> +
>>>>> +	buf = malloc(F2FS_BLKSIZE);
>>>>> +	if (buf == NULL) {
>>>>> +		MSG(1, "\tError: Malloc Failed for buf!!!\n");
>>>>> +		return false;
>>>>> +	}
>>>>> +	zero_buf = calloc(1, F2FS_BLKSIZE);
>>>>> +	if (zero_buf == NULL) {
>>>>> +		MSG(1, "\tError: Calloc Failed for zero buf!!!\n");
>>>>> +		free(buf);
>>>>> +		return false;
>>>>> +	}
>>>>> +
>>>>> +	if (lseek(fd, 0, SEEK_SET) < 0) {
>>>>> +		free(zero_buf);
>>>>> +		free(buf);
>>>>> +		return false;
>>>>> +	}
>>>>> +
>>>>> +	/* check first n blocks */
>>>>> +	for (j = 0; j < nblocks; j++) {
>>>>> +		if (read(fd, buf, F2FS_BLKSIZE) != F2FS_BLKSIZE ||
>>>>> +				memcmp(buf, zero_buf, F2FS_BLKSIZE)) {
>>>>> +			wiped = false;
>>>>> +			break;
>>>>> +		}
>>>>> +	}
>>>>> +	free(zero_buf);
>>>>> +	free(buf);
>>>>> +
>>>>> +	if (wiped)
>>>>> +		MSG(0, "Info: Found all zeros in first %d blocks\n", nblocks);
>>>>> +	return wiped;
>>>>> +#else
>>>>> +	return false;
>>>>> +#endif
>>>>> +}
>>>>> +
>>>>>  int f2fs_trim_devices(void)
>>>>>  {
>>>>>  	int i;
>>>>>  
>>>>> -	for (i = 0; i < c.ndevs; i++)
>>>>> -		if (trim_device(i))
>>>>> +	for (i = 0; i < c.ndevs; i++) {
>>>>> +		if (!is_wiped_device(i) && trim_device(i))
>>>>>  			return -1;
>>>>> +	}
>>>>>  	c.trimmed = 1;
>>>>>  	return 0;
>>>>>  }
>>>>>
>>> .
>>>
> .
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, back to index

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-08 23:11 [f2fs-dev] [PATCH] mkfs.f2fs: check zeros in first 16MB for Android Jaegeuk Kim
2019-08-09 15:12 ` [f2fs-dev] [PATCH v2] " Jaegeuk Kim
     [not found]   ` <CAD14+f2V=j8o=0sUGMgmJHmwKgm80WyzJC5yW7qmyffL=CBJhw@mail.gmail.com>
2019-08-09 15:39     ` Jaegeuk Kim
2019-08-12  3:26       ` Chao Yu
2019-08-12  2:19   ` Chao Yu
2019-08-15 22:21     ` Jaegeuk Kim
2019-08-16  1:00       ` Chao Yu
2019-08-16  1:02         ` Jaegeuk Kim
2019-08-16  1:23           ` Chao Yu

Linux-f2fs-devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-f2fs-devel/0 linux-f2fs-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-f2fs-devel linux-f2fs-devel/ https://lore.kernel.org/linux-f2fs-devel \
		linux-f2fs-devel@lists.sourceforge.net linux-f2fs-devel@archiver.kernel.org
	public-inbox-index linux-f2fs-devel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/net.sourceforge.lists.linux-f2fs-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox