Re: [f2fs-dev] [PATCH v2] mkfs.f2fs: check zeros in first 16MB for Android
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 님이 작성:
>>
>>> We actually don't need to issue trim on entire disk by checking first
>>> blocks having zeros.
>>>
>>> Signed-off-by: Jaegeuk Kim
>>> ---
>>> 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
>>> #include
>>> #include
>>> +#include
>>> #ifndef ANDROID_WINDOWS_HOST
>>> #include
>>> #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
Re: [f2fs-dev] [PATCH] f2fs_io: add get/setflags
On 2019/8/9 23:23, Jaegeuk Kim wrote:
> From: Jaegeuk Kim
>
> Signed-off-by: Jaegeuk Kim
> ---
> tools/f2fs_io/f2fs_io.c | 91 +
> tools/f2fs_io/f2fs_io.h | 14 +++
> 2 files changed, 105 insertions(+)
>
> diff --git a/tools/f2fs_io/f2fs_io.c b/tools/f2fs_io/f2fs_io.c
> index 6b43778..b57c458 100644
> --- a/tools/f2fs_io/f2fs_io.c
> +++ b/tools/f2fs_io/f2fs_io.c
> @@ -45,6 +45,95 @@ struct cmd_desc {
> int cmd_flags;
> };
>
> +#define getflags_desc "getflags ioctl"
> +#define getflags_help\
> +"f2fs_io getflags [file]\n\n"\
> +"get a flag given the file\n"\
> +"flag can show \n" \
> +" casefold\n" \
> +" compression\n"\
> +" nocompression\n"
Could we support +/- flags?
> +
> +static void do_getflags(int argc, char **argv, const struct cmd_desc *cmd)
> +{
> + long flag;
> + int ret, fd;
> + int exist = 0;
> +
> + if (argc != 2) {
> + fputs("Excess arguments\n\n", stderr);
> + fputs(cmd->cmd_help, stderr);
> + exit(1);
> + }
> +
> + fd = open(argv[1], O_RDONLY);
> + if (fd == -1) {
> + fputs("Open failed\n\n", stderr);
> + fputs(cmd->cmd_help, stderr);
> + exit(1);
> + }
> + ret = ioctl(fd, F2FS_IOC_GETFLAGS, );
> + printf("get a flag on %s ret=%d, flags=", argv[1], ret);
> + if (flag & FS_CASEFOLD_FL) {
> + printf("casefold");
> + exist = 1;
> + }
> + if (flag & FS_COMPR_FL) {
> + if (exist)
> + printf(",");
> + printf("compression");
> + exist = 1;
> + }
> + if (flag & FS_NOCOMP_FL) {
> + if (exist)
> + printf(",");
> + printf("nocompression");
> + exist = 1;
> + }
> + if (!exist)
> + printf("none");
> + printf("\n");
> + exit(0);
> +}
> +
> +#define setflags_desc "setflags ioctl"
> +#define setflags_help\
> +"f2fs_io setflags [flag] [file]\n\n" \
> +"set a flag given the file\n"\
> +"flag can be\n" \
> +" casefold\n" \
> +" compression\n"\
> +" nocompression\n"
> +
> +static void do_setflags(int argc, char **argv, const struct cmd_desc *cmd)
> +{
> + long flag;
> + int ret, fd;
> +
> + if (argc != 3) {
> + fputs("Excess arguments\n\n", stderr);
> + fputs(cmd->cmd_help, stderr);
> + exit(1);
> + }
> +
> + fd = open(argv[2], O_RDONLY);
> + if (fd == -1) {
> + fputs("Open failed\n\n", stderr);
> + fputs(cmd->cmd_help, stderr);
> + exit(1);
> + }
> + if (!strcmp(argv[1], "casefold"))
> + flag = FS_CASEFOLD_FL;
> + else if (!strcmp(argv[1], "compression"))
> + flag = FS_COMPR_FL;
> + else if (!strcmp(argv[1], "nocompression"))
> + flag = FS_NOCOMP_FL;
> +
> + ret = ioctl(fd, F2FS_IOC_SETFLAGS, );
It will drop other existed flags, should be:
F2FS_IOC_GETFLAGS flag
flag +/- FS_*_FL
F2FS_IOC_SETFLAGS flag
Thanks,
> + printf("set a flag: %s ret=%d\n", argv[2], ret);
> + exit(0);
> +}
> +
> #define shutdown_desc "shutdown filesystem"
> #define shutdown_help\
> "f2fs_io shutdown [level] [dir]\n\n" \
> @@ -488,6 +577,8 @@ static void do_defrag_file(int argc, char **argv, const
> struct cmd_desc *cmd)
> static void do_help(int argc, char **argv, const struct cmd_desc *cmd);
> const struct cmd_desc cmd_list[] = {
> _CMD(help),
> + CMD(getflags),
> + CMD(setflags),
> CMD(shutdown),
> CMD(pinfile),
> CMD(fallocate),
> diff --git a/tools/f2fs_io/f2fs_io.h b/tools/f2fs_io/f2fs_io.h
> index 95fd5be..5768c1b 100644
> --- a/tools/f2fs_io/f2fs_io.h
> +++ b/tools/f2fs_io/f2fs_io.h
> @@ -41,9 +41,13 @@ typedef u32__be32;
> #ifndef FS_IOC_GETFLAGS
> #define FS_IOC_GETFLAGS _IOR('f', 1, long)
> #endif
> +#ifndef FS_IOC_SETFLAGS
> +#define FS_IOC_SETFLAGS _IOW('f', 2, long)
> +#endif
>
> #define F2FS_IOCTL_MAGIC 0xf5
> #define F2FS_IOC_GETFLAGSFS_IOC_GETFLAGS
> +#define F2FS_IOC_SETFLAGSFS_IOC_SETFLAGS
>
> #define F2FS_IOC_START_ATOMIC_WRITE _IO(F2FS_IOCTL_MAGIC, 1)
> #define F2FS_IOC_COMMIT_ATOMIC_WRITE _IO(F2FS_IOCTL_MAGIC, 2)
> @@ -98,6 +102,16 @@ typedef u32 __be32;
> #define F2FS_IOC_FSGETXATTR
Re: [f2fs-dev] [PATCH v2] mkfs.f2fs: check zeros in first 16MB for Android
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
> ---
> 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
> #include
> #include
> +#include
> #ifndef ANDROID_WINDOWS_HOST
> #include
> #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
[f2fs-dev] [PATCH 1/6] fs-verity: fix crash on read error in build_merkle_tree_level()
From: Eric Biggers
In build_merkle_tree_level(), use a separate fsverity_err() statement
when failing to read a data page, rather than incorrectly reusing the
one for failure to read a Merkle tree page and accessing level_start[]
out of bounds due to 'params->level_start[level - 1]' when level == 0.
Fixes: 248676649d53 ("fs-verity: implement FS_IOC_ENABLE_VERITY ioctl")
Signed-off-by: Eric Biggers
---
fs/verity/enable.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/fs/verity/enable.c b/fs/verity/enable.c
index 3371d51563962..eabc6ac199064 100644
--- a/fs/verity/enable.c
+++ b/fs/verity/enable.c
@@ -43,19 +43,27 @@ static int build_merkle_tree_level(struct inode *inode,
unsigned int level,
pr_debug("Hashing block %llu of %llu for level %u\n",
i + 1, num_blocks_to_hash, level);
- if (level == 0)
+ if (level == 0) {
/* Leaf: hashing a data block */
src_page = read_mapping_page(inode->i_mapping, i, NULL);
- else
+ if (IS_ERR(src_page)) {
+ err = PTR_ERR(src_page);
+ fsverity_err(inode,
+"Error %d reading data page %llu",
+err, i);
+ return err;
+ }
+ } else {
/* Non-leaf: hashing hash block from level below */
src_page = vops->read_merkle_tree_page(inode,
params->level_start[level - 1] + i);
- if (IS_ERR(src_page)) {
- err = PTR_ERR(src_page);
- fsverity_err(inode,
-"Error %d reading Merkle tree page %llu",
-err, params->level_start[level - 1] + i);
- return err;
+ if (IS_ERR(src_page)) {
+ err = PTR_ERR(src_page);
+ fsverity_err(inode,
+"Error %d reading Merkle tree page
%llu",
+err, params->level_start[level -
1] + i);
+ return err;
+ }
}
err = fsverity_hash_page(params, inode, req, src_page,
--
2.22.0
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 3/6] f2fs: skip truncate when verity in progress in ->write_begin()
From: Eric Biggers
When an error (e.g. ENOSPC) occurs during f2fs_write_begin() when called
from f2fs_write_merkle_tree_block(), skip truncating the file. i_size
is not meaningful in this case, and the truncation is handled by
f2fs_end_enable_verity() instead.
Fixes: 60d7bf0f790f ("f2fs: add fs-verity support")
Signed-off-by: Eric Biggers
---
fs/f2fs/data.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 3f525f8a3a5fa..00b03fb87bd9b 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2476,7 +2476,7 @@ static void f2fs_write_failed(struct address_space
*mapping, loff_t to)
struct inode *inode = mapping->host;
loff_t i_size = i_size_read(inode);
- if (to > i_size) {
+ if (to > i_size && !f2fs_verity_in_progress(inode)) {
down_write(_I(inode)->i_gc_rwsem[WRITE]);
down_write(_I(inode)->i_mmap_sem);
--
2.22.0
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 5/6] ext4: fix comment in ext4_end_enable_verity()
From: Eric Biggers
Signed-off-by: Eric Biggers
---
fs/ext4/verity.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/verity.c b/fs/ext4/verity.c
index bb0a3b8e6ea71..d0d8a9795dd62 100644
--- a/fs/ext4/verity.c
+++ b/fs/ext4/verity.c
@@ -196,7 +196,7 @@ static int ext4_end_enable_verity(struct file *filp, const
void *desc,
size_t desc_size, u64 merkle_tree_size)
{
struct inode *inode = file_inode(filp);
- const int credits = 2; /* superblock and inode for ext4_orphan_add() */
+ const int credits = 2; /* superblock and inode for ext4_orphan_del() */
handle_t *handle;
int err = 0;
int err2;
--
2.22.0
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 2/6] ext4: skip truncate when verity in progress in ->write_begin()
From: Eric Biggers
When an error (e.g. ENOSPC) occurs during ext4_write_begin() when called
from ext4_write_merkle_tree_block(), skip truncating the file. i_size
is not meaningful in this case, and the truncation is handled by
ext4_end_enable_verity() instead. Also, this was triggering the
WARN_ON(!inode_is_locked(inode)) in ext4_truncate().
Fixes: ea54d7e4c0f8 ("ext4: add basic fs-verity support")
Signed-off-by: Eric Biggers
---
fs/ext4/inode.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index b2c8d09acf652..cf0fce1173a4c 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1340,6 +1340,9 @@ static int ext4_write_begin(struct file *file, struct
address_space *mapping,
}
if (ret) {
+ bool extended = (pos + len > inode->i_size) &&
+ !ext4_verity_in_progress(inode);
+
unlock_page(page);
/*
* __block_write_begin may have instantiated a few blocks
@@ -1349,11 +1352,11 @@ static int ext4_write_begin(struct file *file, struct
address_space *mapping,
* Add inode to orphan list in case we crash before
* truncate finishes
*/
- if (pos + len > inode->i_size && ext4_can_truncate(inode))
+ if (extended && ext4_can_truncate(inode))
ext4_orphan_add(handle, inode);
ext4_journal_stop(handle);
- if (pos + len > inode->i_size) {
+ if (extended) {
ext4_truncate_failed_write(inode);
/*
* If truncate failed early the inode might
--
2.22.0
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 4/6] ext4: remove ext4_bio_encrypted()
From: Eric Biggers
ext4_bio_encrypted() is unused following commit 4e47a0d40dac
("ext4: add fs-verity read support"), so remove it.
Signed-off-by: Eric Biggers
---
fs/ext4/readpage.c | 9 -
1 file changed, 9 deletions(-)
diff --git a/fs/ext4/readpage.c b/fs/ext4/readpage.c
index ec8aeab3af65a..a30b203fa461c 100644
--- a/fs/ext4/readpage.c
+++ b/fs/ext4/readpage.c
@@ -52,15 +52,6 @@
static struct kmem_cache *bio_post_read_ctx_cache;
static mempool_t *bio_post_read_ctx_pool;
-static inline bool ext4_bio_encrypted(struct bio *bio)
-{
-#ifdef CONFIG_FS_ENCRYPTION
- return unlikely(bio->bi_private != NULL);
-#else
- return false;
-#endif
-}
-
/* postprocessing steps for read bios */
enum bio_post_read_step {
STEP_INITIAL = 0,
--
2.22.0
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 6/6] f2fs: use EFSCORRUPTED in f2fs_get_verity_descriptor()
From: Eric Biggers
EFSCORRUPTED is now defined in f2fs.h, so use it instead of EUCLEAN.
Signed-off-by: Eric Biggers
---
fs/f2fs/verity.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/f2fs/verity.c b/fs/f2fs/verity.c
index 6bc3470d99d00..a401ef72bc821 100644
--- a/fs/f2fs/verity.c
+++ b/fs/f2fs/verity.c
@@ -210,7 +210,7 @@ static int f2fs_get_verity_descriptor(struct inode *inode,
void *buf,
if (pos + size < pos || pos + size > inode->i_sb->s_maxbytes ||
pos < f2fs_verity_metadata_pos(inode) || size > INT_MAX) {
f2fs_warn(F2FS_I_SB(inode), "invalid verity xattr");
- return -EUCLEAN; /* EFSCORRUPTED */
+ return -EFSCORRUPTED;
}
if (buf_size) {
if (size > buf_size)
--
2.22.0
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 0/6] fs-verity fixups
A few fixes and cleanups for fs-verity. If there are no objections, I'll fold these into the original patches. Eric Biggers (6): fs-verity: fix crash on read error in build_merkle_tree_level() ext4: skip truncate when verity in progress in ->write_begin() f2fs: skip truncate when verity in progress in ->write_begin() ext4: remove ext4_bio_encrypted() ext4: fix comment in ext4_end_enable_verity() f2fs: use EFSCORRUPTED in f2fs_get_verity_descriptor() fs/ext4/inode.c| 7 +-- fs/ext4/readpage.c | 9 - fs/ext4/verity.c | 2 +- fs/f2fs/data.c | 2 +- fs/f2fs/verity.c | 2 +- fs/verity/enable.c | 24 6 files changed, 24 insertions(+), 22 deletions(-) -- 2.22.0 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
