[f2fs-dev] Multiple fsck runs required to fix quota

[Ju Hyung Park]

Hi.

I'm reporting a possible issue with fsck requiring multiple runs to fix quota.

The fsck version is the latest master branch, statically compiled
under arm64 flavor of Ubuntu and was executed under recovery mode.

First run:
OnePlus7Pro:/ # /fsck.f2fs -a -f /dev/block/sda19
Info: Fix the reported corruption.
Info: Force to fix corruption
Info: [/dev/block/sda19] Disk Model: KLUEG8UHDB-C2D1
Info: Segments per section = 1
Info: Sections per zone = 1
Info: sector size = 4096
Info: total sectors = 59463283 (232278 MB)
Info: MKFS version
  "Linux version 4.14.117-perf+ (OnePlus@rd-build-81) (clang version
8.0.8 for Android NDK) #1 SMP PREEMPT Tue Sep 17 21:09:27 CST 2019"
Info: FSCK version
  from "Linux version 4.14.117-perf+ (OnePlus@rd-build-81) (clang
version 8.0.8 for Android NDK) #1 SMP PREEMPT Tue Sep 17 21:09:27 CST
2019"
to "Linux version 4.14.117-perf+ (OnePlus@rd-build-81) (clang
version 8.0.8 for Android NDK) #1 SMP PREEMPT Tue Sep 17 21:09:27 CST
2019"
Info: superblock features = 80 :  quota_ino
Info: superblock encrypt level = 0, salt = 
Info: total FS sectors = 59463283 (232278 MB)
Info: CKPT version = 5a6b3a3
Info: checkpoint state = 45 :  crc compacted_summary unmount
[ERROR] quotaio_v2.c:201:v2_init_io:: Quota inode 4 corrupted: file
size 88064 does not match page offset 28
[fsck_chk_quota_files:1880] Fixing Quota file ([  0] ino [0x4])
[ERROR] quotaio_v2.c:201:v2_init_io:: Quota inode 5 corrupted: file
size 160768 does not match page offset 53
[fsck_chk_quota_files:1880] Fixing Quota file ([  1] ino [0x5])

[FSCK] Unreachable nat entries[Ok..] [0x0]
[FSCK] SIT valid block bitmap checking[Ok..]
[FSCK] Hard link checking for regular file[Ok..] [0x153c]
[FSCK] valid_block_count matching with CP [Ok..] [0x1e13d74]
[FSCK] valid_node_count matching with CP (de lookup)  [Ok..] [0x9d260]
[FSCK] valid_node_count matching with CP (nat lookup) [Ok..] [0x9d260]
[FSCK] valid_inode_count matched with CP  [Ok..] [0x98b99]
[FSCK] free segment_count matched with CP [Ok..] [0xd395]
[FSCK] next block offset is free  [Ok..]
[FSCK] fixing SIT types
[FSCK] other corrupted bugs   [Fail]
Info: Duplicate valid checkpoint to mirror position 1024 -> 512
Info: Write valid nat_bits in checkpoint
Info: Write valid nat_bits in checkpoint

Done: 75.117901 secs
1|OnePlus7Pro:/ #

As you can see, the first run returns 1.

Second run:
1|OnePlus7Pro:/ # /fsck.f2fs -a -f /dev/block/sda19
Info: Fix the reported corruption.
Info: Force to fix corruption
Info: [/dev/block/sda19] Disk Model: KLUEG8UHDB-C2D1
Info: Segments per section = 1
Info: Sections per zone = 1
Info: sector size = 4096
Info: total sectors = 59463283 (232278 MB)
Info: MKFS version
  "Linux version 4.14.117-perf+ (OnePlus@rd-build-81) (clang version
8.0.8 for Android NDK) #1 SMP PREEMPT Tue Sep 17 21:09:27 CST 2019"
Info: FSCK version
  from "Linux version 4.14.117-perf+ (OnePlus@rd-build-81) (clang
version 8.0.8 for Android NDK) #1 SMP PREEMPT Tue Sep 17 21:09:27 CST
2019"
to "Linux version 4.14.117-perf+ (OnePlus@rd-build-81) (clang
version 8.0.8 for Android NDK) #1 SMP PREEMPT Tue Sep 17 21:09:27 CST
2019"
Info: superblock features = 80 :  quota_ino
Info: superblock encrypt level = 0, salt = 
Info: total FS sectors = 59463283 (232278 MB)
Info: CKPT version = 5a6b3a3
Info: Checked valid nat_bits in checkpoint
Info: checkpoint state = 281 :  allow_nocrc nat_bits unmount
[ERROR] quotaio_v2.c:201:v2_init_io:: Quota inode 4 corrupted: file
size 88064 does not match page offset 28
[ERROR] quotaio_v2.c:201:v2_init_io:: Quota inode 5 corrupted: file
size 159744 does not match page offset 53

[FSCK] Unreachable nat entries[Ok..] [0x0]
[FSCK] SIT valid block bitmap checking[Ok..]
[FSCK] Hard link checking for regular file[Ok..] [0x153c]
[FSCK] valid_block_count matching with CP [Ok..] [0x1e13d74]
[FSCK] valid_node_count matching with CP (de lookup)  [Ok..] [0x9d260]
[FSCK] valid_node_count matching with CP (nat lookup) [Ok..] [0x9d260]
[FSCK] valid_inode_count matched with CP  [Ok..] [0x98b99]
[FSCK] free segment_count matched with CP [Ok..] [0xd395]
[FSCK] next block offset is free  [Ok..]
[FSCK] fixing SIT types
[FSCK] other corrupted bugs   [Ok..]

Done: 73.506501 secs
OnePlus7Pro:/ #

We can see that it returns 0, but still prints "Quota inode # corrupted".

Third run:
OnePlus7Pro:/ # /fsck.f2fs -a -f /dev/block/sda19
Info: Fix the reported corruption.
Info: Force to fix corruption
Info: [/dev/block/sda19] Disk Model: KLUEG8UHDB-C2D1
Info: Segments per section = 1
Info: Sections per zone = 1
Info: sector size = 4096
Info: total sectors = 59463283 (232278 MB)
Info: MKFS version
  "Linux version 4.14.117-perf+ (OnePlus@rd-build-81) 

Re: [f2fs-dev] [PATCH] f2fs: compress: support lz4hc compression

[Ju Hyung Park]

Hi Chao.

On Mon, Nov 30, 2020 at 8:28 PM Chao Yu  wrote:
>
> Add a new mount option "compress_lz4hc_clevel=%u" to enable lz4hc compress
> algorithm and specify the compress level of lz4hc.

Wouldn't it be better to introduce a generic mount option like
"compress_level=%u" to allow other algorithms to use it as well?

Or retire compress_algorithm altogether and follow btrfs format, which
I quite like:
compress=lzo:1, compress=zstd:4, etc.

Just my two cents.

>
> Signed-off-by: Chao Yu 
> ---
>  Documentation/filesystems/f2fs.rst |  2 ++
>  fs/f2fs/compress.c | 22 +-
>  fs/f2fs/f2fs.h |  8 
>  fs/f2fs/inode.c|  4 
>  fs/f2fs/super.c| 15 +++
>  5 files changed, 46 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/filesystems/f2fs.rst 
> b/Documentation/filesystems/f2fs.rst
> index 8830a11a11be..cda30ea124ee 100644
> --- a/Documentation/filesystems/f2fs.rst
> +++ b/Documentation/filesystems/f2fs.rst
> @@ -264,6 +264,8 @@ compress_chksum  Support verifying chksum of 
> raw data in compressed cluster.
>  compress_cache  Support to use address space of inner inode to cache
>  compressed block, in order to improve cache hit 
> ratio of
>  random read.
> +compress_lz4hc_clevel   Support to enable LZ4 high compression algorithm, 
> compress
> +level range is [3, 16].
>  inlinecrypt When possible, encrypt/decrypt the contents of 
> encrypted
>  files using the blk-crypto framework rather than
>  filesystem-layer encryption. This allows the use of
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 2ec34168adbb..233be7f71e48 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -253,17 +253,24 @@ static const struct f2fs_compress_ops f2fs_lzo_ops = {
>  #ifdef CONFIG_F2FS_FS_LZ4
>  static int lz4_init_compress_ctx(struct compress_ctx *cc)
>  {
> -   cc->private = f2fs_kvmalloc(F2FS_I_SB(cc->inode),
> -   LZ4_MEM_COMPRESS, GFP_NOFS);
> +   unsigned int size;
> +
> +   size = F2FS_I(cc->inode)->i_lz4hc_clevel ?
> +   LZ4HC_MEM_COMPRESS : LZ4_MEM_COMPRESS;
> +
> +   cc->private = f2fs_kvmalloc(F2FS_I_SB(cc->inode), size, GFP_NOFS);
> if (!cc->private)
> return -ENOMEM;
>
> /*
>  * we do not change cc->clen to LZ4_compressBound(inputsize) to
>  * adapt worst compress case, because lz4 compressor can handle
> -* output budget properly.
> +* output budget properly; for lz4hc case, keep it as it is.
>  */
> -   cc->clen = cc->rlen - PAGE_SIZE - COMPRESS_HEADER_SIZE;
> +   if (F2FS_I(cc->inode)->i_lz4hc_clevel)
> +   cc->clen = LZ4_compressBound(cc->rlen);
> +   else
> +   cc->clen = cc->rlen - PAGE_SIZE - COMPRESS_HEADER_SIZE;
> return 0;
>  }
>
> @@ -275,9 +282,14 @@ static void lz4_destroy_compress_ctx(struct compress_ctx 
> *cc)
>
>  static int lz4_compress_pages(struct compress_ctx *cc)
>  {
> +   unsigned char clevel = F2FS_I(cc->inode)->i_lz4hc_clevel;
> int len;
>
> -   len = LZ4_compress_default(cc->rbuf, cc->cbuf->cdata, cc->rlen,
> +   if (clevel)
> +   len = LZ4_compress_HC(cc->rbuf, cc->cbuf->cdata, cc->rlen,
> +   cc->clen, clevel, 
> cc->private);
> +   else
> +   len = LZ4_compress_default(cc->rbuf, cc->cbuf->cdata, 
> cc->rlen,
> cc->clen, cc->private);
> if (!len)
> return -EAGAIN;
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index d32065417616..d3d5583ea9e5 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -149,6 +149,7 @@ struct f2fs_mount_info {
> /* For compression */
> unsigned char compress_algorithm;   /* algorithm type */
> unsigned char compress_log_size;/* cluster log size */
> +   unsigned char lz4hc_clevel; /* lz4hc compress level */
> bool compress_chksum;   /* compressed data chksum */
> unsigned char compress_ext_cnt; /* extension count */
> unsigned char extensions[COMPRESS_EXT_NUM][F2FS_EXTENSION_LEN]; /* 
> extensions */
> @@ -736,6 +737,10 @@ struct f2fs_inode_info {
> unsigned char i_log_cluster_size;   /* log of cluster size */
> unsigned short i_compress_flag; /* compress flag */
> unsigned int i_cluster_size;/* cluster size */
> +   unsigned char i_lz4hc_clevel;   /*
> +* lz4hc compress level,
> +* range: 3-16, disable: 0
> +*/
>  };
>

Re: [f2fs-dev] [PATCH v2] f2fs: use kmem_cache pool during inline xattr lookups

[Ju Hyung Park]

Hi Chao.

I got the time around to test this patch.
The v2 patch seems to work just fine, and the code looks good.

On Tue, Feb 25, 2020 at 7:17 PM Chao Yu  wrote:
> diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
> index a3360a97e624..e46a10eb0e42 100644
> --- a/fs/f2fs/xattr.c
> +++ b/fs/f2fs/xattr.c
> @@ -23,6 +23,25 @@
>  #include "xattr.h"
>  #include "segment.h"
>
> +static void *xattr_alloc(struct f2fs_sb_info *sbi, int size, bool *is_inline)
> +{
> +   *is_inline = (size == sbi->inline_xattr_slab_size);

Would it be meaningless to change this to the following code?
if (likely(size == sbi->inline_xattr_slab_size))
*is_inline = true;
else
*is_inline = false;

The above statement seems to be only false during the initial mount
and the rest(millions) seems to be always true.

Thanks.


___
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: use kmem_cache pool during inline xattr lookups

[Ju Hyung Park]

Hi Chao.

Sorry for the late reply, I've been busy with my uni stuffs.

It's nice to see this issue getting some attentions back, but
unfortunately, I can't test it at the moment.

Here's the patch we used to see the alloc statistics:
https://pastebin.com/bhBh2dgs by Sultan Alsawaf.

Hope it helps.

On Tue, Feb 25, 2020 at 10:43 AM Chao Yu  wrote:
>
> It's been observed that kzalloc() on lookup_all_xattrs() are called millions
> of times on Android, quickly becoming the top abuser of slub memory allocator.
>
> Use a dedicated kmem cache pool for xattr lookups to mitigate this.
>
> Signed-off-by: Park Ju Hyung 
> Signed-off-by: Chao Yu 
> ---
>
> Hi, Ju Hyung,
>
> Let me know if you have any objection on this patch. :)
>
>  fs/f2fs/f2fs.h  |  3 +++
>  fs/f2fs/super.c | 10 -
>  fs/f2fs/xattr.c | 54 -
>  fs/f2fs/xattr.h |  4 
>  4 files changed, 65 insertions(+), 6 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 12a197e89a3e..23b93a116c73 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1486,6 +1486,9 @@ struct f2fs_sb_info {
> __u32 s_chksum_seed;
>
> struct workqueue_struct *post_read_wq;  /* post read workqueue */
> +
> +   struct kmem_cache *inline_xattr_slab;   /* inline xattr entry */
> +   unsigned int inline_xattr_slab_size;/* default inline xattr slab 
> size */
>  };
>
>  struct f2fs_private_dio {
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index d241e07c0bfa..0b16204d3b7d 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1201,6 +1201,7 @@ static void f2fs_put_super(struct super_block *sb)
> kvfree(sbi->raw_super);
>
> destroy_device_list(sbi);
> +   f2fs_destroy_xattr_caches(sbi);
> mempool_destroy(sbi->write_io_dummy);
>  #ifdef CONFIG_QUOTA
> for (i = 0; i < MAXQUOTAS; i++)
> @@ -3457,12 +3458,17 @@ static int f2fs_fill_super(struct super_block *sb, 
> void *data, int silent)
> }
> }
>
> +   /* init per sbi slab cache */
> +   err = f2fs_init_xattr_caches(sbi);
> +   if (err)
> +   goto free_io_dummy;
> +
> /* get an inode for meta space */
> sbi->meta_inode = f2fs_iget(sb, F2FS_META_INO(sbi));
> if (IS_ERR(sbi->meta_inode)) {
> f2fs_err(sbi, "Failed to read F2FS meta data inode");
> err = PTR_ERR(sbi->meta_inode);
> -   goto free_io_dummy;
> +   goto free_xattr_cache;
> }
>
> err = f2fs_get_valid_checkpoint(sbi);
> @@ -3735,6 +3741,8 @@ static int f2fs_fill_super(struct super_block *sb, void 
> *data, int silent)
> make_bad_inode(sbi->meta_inode);
> iput(sbi->meta_inode);
> sbi->meta_inode = NULL;
> +free_xattr_cache:
> +   f2fs_destroy_xattr_caches(sbi);
>  free_io_dummy:
> mempool_destroy(sbi->write_io_dummy);
>  free_percpu:
> diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
> index a3360a97e624..e46a10eb0e42 100644
> --- a/fs/f2fs/xattr.c
> +++ b/fs/f2fs/xattr.c
> @@ -23,6 +23,25 @@
>  #include "xattr.h"
>  #include "segment.h"
>
> +static void *xattr_alloc(struct f2fs_sb_info *sbi, int size, bool *is_inline)
> +{
> +   *is_inline = (size == sbi->inline_xattr_slab_size);
> +
> +   if (*is_inline)
> +   return kmem_cache_zalloc(sbi->inline_xattr_slab, GFP_NOFS);
> +
> +   return f2fs_kzalloc(sbi, size, GFP_NOFS);
> +}
> +
> +static void xattr_free(struct f2fs_sb_info *sbi, void *xattr_addr,
> +   bool is_inline)
> +{
> +   if (is_inline)
> +   kmem_cache_free(sbi->inline_xattr_slab, xattr_addr);
> +   else
> +   kvfree(xattr_addr);
> +}
> +
>  static int f2fs_xattr_generic_get(const struct xattr_handler *handler,
> struct dentry *unused, struct inode *inode,
> const char *name, void *buffer, size_t size)
> @@ -301,7 +320,8 @@ static int read_xattr_block(struct inode *inode, void 
> *txattr_addr)
>  static int lookup_all_xattrs(struct inode *inode, struct page *ipage,
> unsigned int index, unsigned int len,
> const char *name, struct f2fs_xattr_entry 
> **xe,
> -   void **base_addr, int *base_size)
> +   void **base_addr, int *base_size,
> +   bool *is_inline)
>  {
> void *cur_addr, *txattr_addr, *last_txattr_addr;
> void *last_addr = NULL;
> @@ -313,7 +333,7 @@ static int lookup_all_xattrs(struct inode *inode, struct 
> page *ipage,
> return -ENODATA;
>
> *base_size = XATTR_SIZE(inode) + XATTR_PADDING_SIZE;
> -   txattr_addr = f2fs_kzalloc(F2FS_I_SB(inode), *base_size, GFP_NOFS);
> +   txattr_addr = xattr_alloc(F2FS_I_SB(inode), *base_size, is_inline);
> if (!txattr_addr)
> return -ENOMEM;
>
> 

Re: [f2fs-dev] [PATCH] f2fs: fix to add swap extent correctly

[Ju Hyung Park]

Hi Chao,

I think this should be sent to linux-stable.
Thoughts?

Thanks.

On Fri, Dec 27, 2019 at 7:45 PM Chao Yu  wrote:
>
> As Youling reported in mailing list:
>
> https://www.linuxquestions.org/questions/linux-newbie-8/the-file-system-f2fs-is-broken-4175666043/
>
> https://www.linux.org/threads/the-file-system-f2fs-is-broken.26490/
>
> There is a test case can corrupt f2fs image:
> - dd if=/dev/zero of=/swapfile bs=1M count=4096
> - chmod 600 /swapfile
> - mkswap /swapfile
> - swapon --discard /swapfile
>
> The root cause is f2fs_swap_activate() intends to return zero value
> to setup_swap_extents() to enable SWP_FS mode (swap file goes through
> fs), in this flow, setup_swap_extents() setups swap extent with wrong
> block address range, result in discard_swap() erasing incorrect address.
>
> Because f2fs_swap_activate() has pinned swapfile, its data block
> address will not change, it's safe to let swap to handle IO through
> raw device, so we can get rid of SWAP_FS mode and initial swap extents
> inside f2fs_swap_activate(), by this way, later discard_swap() can trim
> in right address range.
>
> Fixes: 4969c06a0d83 ("f2fs: support swap file w/ DIO")
> Signed-off-by: Chao Yu 
> ---
>  fs/f2fs/data.c | 32 +---
>  1 file changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 19cd03450066..ee4d3d284379 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -3608,7 +3608,8 @@ int f2fs_migrate_page(struct address_space *mapping,
>
>  #ifdef CONFIG_SWAP
>  /* Copied from generic_swapfile_activate() to check any holes */
> -static int check_swap_activate(struct file *swap_file, unsigned int max)
> +static int check_swap_activate(struct swap_info_struct *sis,
> +   struct file *swap_file, sector_t *span)
>  {
> struct address_space *mapping = swap_file->f_mapping;
> struct inode *inode = mapping->host;
> @@ -3619,6 +3620,8 @@ static int check_swap_activate(struct file *swap_file, 
> unsigned int max)
> sector_t last_block;
> sector_t lowest_block = -1;
> sector_t highest_block = 0;
> +   int nr_extents = 0;
> +   int ret;
>
> blkbits = inode->i_blkbits;
> blocks_per_page = PAGE_SIZE >> blkbits;
> @@ -3630,7 +3633,8 @@ static int check_swap_activate(struct file *swap_file, 
> unsigned int max)
> probe_block = 0;
> page_no = 0;
> last_block = i_size_read(inode) >> blkbits;
> -   while ((probe_block + blocks_per_page) <= last_block && page_no < 
> max) {
> +   while ((probe_block + blocks_per_page) <= last_block &&
> +   page_no < sis->max) {
> unsigned block_in_page;
> sector_t first_block;
>
> @@ -3670,13 +3674,27 @@ static int check_swap_activate(struct file 
> *swap_file, unsigned int max)
> highest_block = first_block;
> }
>
> +   /*
> +* We found a PAGE_SIZE-length, PAGE_SIZE-aligned run of 
> blocks
> +*/
> +   ret = add_swap_extent(sis, page_no, 1, first_block);
> +   if (ret < 0)
> +   goto out;
> +   nr_extents += ret;
> page_no++;
> probe_block += blocks_per_page;
>  reprobe:
> continue;
> }
> -   return 0;
> -
> +   ret = nr_extents;
> +   *span = 1 + highest_block - lowest_block;
> +   if (page_no == 0)
> +   page_no = 1;/* force Empty message */
> +   sis->max = page_no;
> +   sis->pages = page_no - 1;
> +   sis->highest_bit = page_no - 1;
> +out:
> +   return ret;
>  bad_bmap:
> pr_err("swapon: swapfile has holes\n");
> return -EINVAL;
> @@ -3701,14 +3719,14 @@ static int f2fs_swap_activate(struct swap_info_struct 
> *sis, struct file *file,
> if (f2fs_disable_compressed_file(inode))
> return -EINVAL;
>
> -   ret = check_swap_activate(file, sis->max);
> -   if (ret)
> +   ret = check_swap_activate(sis, file, span);
> +   if (ret < 0)
> return ret;
>
> set_inode_flag(inode, FI_PIN_FILE);
> f2fs_precache_extents(inode);
> f2fs_update_time(F2FS_I_SB(inode), REQ_TIME);
> -   return 0;
> +   return ret;
>  }
>
>  static void f2fs_swap_deactivate(struct file *file)
> --
> 2.18.0.rc1
>
>
>
> ___
> 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

[f2fs-dev] f2fs: How should I use the defragmentation tools?

[Ju Hyung Park]

Hi everyone.

I've had interests in defragging f2fs for a long time now.

https://www.usenix.org/system/files/conference/atc17/atc17-hahn.pdf
This paper proves that defragmentation caused from the filesystem
takes a big portion of performance degradation.
(It'd be interesting to see whether f2fs would behave differently from
ext4 in terms of fragmentation.)

While doing defragmentation on a production device currently poses a
lot of practicality issues, I'd still like to try them out for
educational purposes.

However, I'm still not sure how to do defragmentation while f2fs
having 2 tools for that purpose.

1. Is either of `defrag.f2fs` or `f2fs_io defrag_file` meant to be
feature-parity with e4defrag?

2. What's the expected output from `defrag.f2fs` and `f2fs_io
defrag_file`? I'd like to get example outputs from you guys for me to
compare.

3. Is `defrag.f2fs` and `f2fs_io defrag_file` tested thoroughly?
Should I expect any potential corruptions from running these?

4. `defrag.f2fs` seems to only work on an unmounted block device.
What's the intended use-case scenario for this? I'm assuming running
`defrag.f2fs` and feeding the entire block length to -l is not how
it's meant to be used.

5. Why did you make it mandatory for the users to supply the lengths
to `f2fs_io defrag_file`? Is it any practical to defrag a part of a
file?

6. How exactly should I run `f2fs_io defrag_file`? My following
attempts failed with -EINVAL:
# ls -al session-02.pdf
-rw-rw-r--. 1 1023 1023 30517375 Nov  7 21:10 session-02.pdf
# f2fs_io defrag_file 0 30517375 session-02.pdf
F2FS_IOC_DEFRAGMENT failed: Invalid argument
# f2fs_io defrag_file 0 1048576 session-02.pdf
F2FS_IOC_DEFRAGMENT failed: Invalid argument

Thanks.


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

Re: [f2fs-dev] [PATCH 2/2] f2fs: support data compression

[Ju Hyung Park]

Hi Jaegeuk and Chao,

Nice to see this finally getting into shape :) Great work
I'm excited to see possible use-cases for this in the future.

Would f2fs compress files automatically like how btrfs' "compress" option works?
Or is it per-extension basis for now?

On Wed, Oct 23, 2019 at 2:16 AM Jaegeuk Kim  wrote:
> +compress_algorithm=%s  Control compress algorithm, currently f2fs supports 
> "lzo"
> +   and "lz4" algorithm.

I see absolutely no reason to support regular lzo variant at this time.
Everyone should use lz4 instead of lzo. If one wants zlib-level
compression, they should use zstd.

However, there's recent conversation on new lzo-rle and how it could
be a better candidate than lz4.

Since the mainline now have lz4, zstd and lzo-rle, I don't think
supporting lzo is a good idea.

> diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig
> index 652fd2e2b23d..c12854c3b1a1 100644
> --- a/fs/f2fs/Kconfig
> +++ b/fs/f2fs/Kconfig
> @@ -6,6 +6,10 @@ config F2FS_FS
> select CRYPTO
> select CRYPTO_CRC32
> select F2FS_FS_XATTR if FS_ENCRYPTION
> +   select LZO_COMPRESS
> +   select LZO_DECOMPRESS
> +   select LZ4_COMPRESS
> +   select LZ4_DECOMPRESS

This is a bad idea.
This unnecessarily increases kernel binary image when no the user
intends to change the defaults.

For example, my Android kernel doesn't use lzo anywhere and this
wouldn't be welcome.

> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> new file mode 100644
> index ..f276d82a67aa
> --- /dev/null
> +++ b/fs/f2fs/compress.c
> @@ -0,0 +1,1066 @@
> +static unsigned int offset_in_cluster(struct compress_ctx *cc, pgoff_t index)
> +static unsigned int cluster_idx(struct compress_ctx *cc, pgoff_t index)
> +static unsigned int start_idx_of_cluster(struct compress_ctx *cc)

Looks like these would be better if they were explicitly marked as inline.

> +static void f2fs_init_compress_ops(struct f2fs_sb_info *sbi)
> +{
> +   sbi->cops[COMPRESS_LZO] = _lzo_ops;
> +   sbi->cops[COMPRESS_LZ4] = _lz4_ops;
> +}

Would it be possible for f2fs to use generic crypto compression APIs?
Hardcoding for lzo/lz4 would make it harder to venture future different options.

Have a look at mm/zswap.c:__zswap_pool_create_fallback().

> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index c681f51e351b..775c96291490 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -155,6 +163,7 @@ struct f2fs_mount_info {
>  #define F2FS_FEATURE_VERITY0x0400
>  #define F2FS_FEATURE_SB_CHKSUM 0x0800
>  #define F2FS_FEATURE_CASEFOLD  0x1000
> +#define F2FS_FEATURE_COMPRESSION   0x2000

How would older versions of f2fs behave if an image was used by the
latest f2fs and have compressed pages?
I hope fail-safes are in place.

Thanks.

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

Re: [f2fs-dev] f2fs: dirty memory increasing during gc_urgent

[Ju Hyung Park]

Hi Chao,

On Sat, Aug 24, 2019 at 12:52 AM Chao Yu  wrote:
> It's not intentional, I failed to reproduce this issue, could you add some 
> logs
> to track why we stop urgent GC even there are still dirty segments?

I'm pretty sure you can reproduce this issue quite easily.

I can see this happening on multiple devices including my workstation,
laptop and my Android phone.

Here's a simple reproduction step:
1. Do `rm -rf * && git reset --hard` a few times under a Linux kernel Git
2. Do a sync
3. echo 1 > /sys/fs/f2fs/dev/gc_urgent_sleep_time
4. echo 1 > /sys/fs/f2fs/dev/gc_urgent
5. Once the number on "GC calls" doesn't change, look at "Dirty" under
/sys/kernel/debug/f2fs/status. It's close to 0.
6. After doing a 'sync', "Dirty" increases a lot.
7. Remember the number on "GC calls" and run 3 and 4 again.
8. The number of "GC calls" increases by a few hundreds.

Thanks.


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

Re: [f2fs-dev] f2fs: dirty memory increasing during gc_urgent

[Ju Hyung Park]

Hi Chao,

On Thu, Aug 15, 2019 at 3:49 PM Chao Yu  wrote:
> I doubt that before triggering urgent GC, system has dirty datas in memory, 
> then
> when you trigger `sync`, GCed data and dirty data were flushed to devices
> together, if we write dirty data with out-place-update model, it may make 
> fragment.
>
> So we can try
> - sync
> - trigger urgent GC
> - sync
> - cat /sys/kernel/debug/f2fs/status to check 'Dirty' field, the value should
> close to zero

It's actually not zero.

Before triggering gc_urgent: 601
After gc_urgent ends and doing a `sync`: 400

And after another 2nd gc_urgent run, it finally becomes 0.

So I'm guessing this wasn't intentional? :P

Thanks,


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

[f2fs-dev] f2fs: dirty memory increasing during gc_urgent

[Ju Hyung Park]

Hi.

I'm reporting some strangeness with gc_urgent.

When running gc_urgent, I can see that dirty memory written in
/proc/meminfo continuously getting increased until GC cannot find any
more segments to clean.

I thought FG_GC are flushed.

And after GC ends, if I do `sync` and run gc_urgent again, it easily
runs thousands of times more.

Is this an expected behavior?

I would much prefer gc_urgent cleaning everything up at first run,
without having to sync at the end and running gc_urgent again.

Thanks.


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

Re: [f2fs-dev] [PATCH 1/2] mkfs.f2fs: add "zip" to cold data types

[Ju Hyung Park]

Hi Chao,

/data/log isn't even here.
I think it must be specific to EMUI.

Thanks.

On Wed, Aug 14, 2019 at 6:48 PM Chao Yu  wrote:
>
> Hi Ju Hyung,
>
> On 2019/8/14 17:20, Ju Hyung Park wrote:
> > Hi Chao,
> >
> > On Wed, Aug 14, 2019 at 10:47 AM Chao Yu  wrote:
> >> In android, as I see, most zip file is small-sized log type, and will be 
> >> removed
> >> after a roll-back, such as:
> >>
> >> time1: create log1.zip
> >> time2: create log2.zip
> >> time3: create log3.zip
> >> time4: remove log1.zip, rename log2.zip -> log1.zip; rename log3.zip ->
> >> log2.zip; create log3.zip
> >>
> >> I suggest we can keep zip type in android as warm type with IPU mode to 
> >> avoid
> >> fragmentation caused by small holes in cold area. In linux distro, I 
> >> agreed to
> >> treat zip as cold type.
> >
> > I actually thought your original suggestion of adding "zip" was to
> > handle big zip files under /sdcard(/data/media).
> >
> > The one case you've mentioned will be entirely dependent on which apps
> > user's using.
>
> Yeah, actually, now I didn't see much large zip file in my external storage,
> most of them are xxxKB or xMB, some of them looks very old tho. :)
>
> > In case of mine, I don't have any zip files under /data that's
> > seemingly used for logs.
>
> Huawei cell phone has log file with .zip/.gz type locating in /data/log/... 
> can
> you search that directory?
>
> If we relocate them into cold area, they will make holes in cold area crazily,
> as they have small size and will be created/unlinked frequently.
>
> Thanks,
>
> >
> > Thanks,
> > .
> >


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

Re: [f2fs-dev] [PATCH 2/2] mkfs.f2fs: add VM disk files to hot data types

[Ju Hyung Park]

Hi Chao,

You're right and Android will never use those types.

But then again, what's the point of separating the list?
I haven't encountered an Android user or an OEM that wants to
customize this list by passing an argument to mkfs.f2fs.

If an OEM want to customize this list, directly modifying the code for
mkfs.f2fs sounds better anyways.

Thanks.

On Wed, Aug 14, 2019 at 10:51 AM Chao Yu  wrote:
>
> On 2019/8/13 6:52, Park Ju Hyung wrote:
> > Similar to .db files, these are randomly updated extremely frequently.
>
> It looks android doesn't need this, how about adding them under "#ifndef
> WITH_ANDROID"?
>
> Thanks,
>
> >
> > Signed-off-by: Park Ju Hyung 
> > ---
> >  mkfs/f2fs_format.c | 5 +
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
> > index 37d82c3..1c08e3e 100644
> > --- a/mkfs/f2fs_format.c
> > +++ b/mkfs/f2fs_format.c
> > @@ -94,6 +94,11 @@ const char *media_ext_lists[] = {
> >
> >  const char *hot_ext_lists[] = {
> >   "db",
> > +
> > + /* Virtual machines */
> > + "vmdk", // VMware or VirtualBox
> > + "vdi", // VirtualBox
> > + "qcow2", // QEMU
> >   NULL
> >  };
> >
> >


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

Re: [f2fs-dev] [PATCH 1/2] mkfs.f2fs: add "zip" to cold data types

[Ju Hyung Park]

Hi Chao,

On Wed, Aug 14, 2019 at 10:47 AM Chao Yu  wrote:
> In android, as I see, most zip file is small-sized log type, and will be 
> removed
> after a roll-back, such as:
>
> time1: create log1.zip
> time2: create log2.zip
> time3: create log3.zip
> time4: remove log1.zip, rename log2.zip -> log1.zip; rename log3.zip ->
> log2.zip; create log3.zip
>
> I suggest we can keep zip type in android as warm type with IPU mode to avoid
> fragmentation caused by small holes in cold area. In linux distro, I agreed to
> treat zip as cold type.

I actually thought your original suggestion of adding "zip" was to
handle big zip files under /sdcard(/data/media).

The one case you've mentioned will be entirely dependent on which apps
user's using.
In case of mine, I don't have any zip files under /data that's
seemingly used for logs.

Thanks,


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

Re: [PATCH v2] f2fs: improve print log in f2fs_sanity_check_ckpt()

[Ju Hyung Park]

Hi Jaegeuk,

I think we'll all appreciate if v1.13.0 gets tagged :)
I believe the current dev branch has been in good shape for quite some time now.

Thanks.


2019년 7월 11일 (목) 오전 10:31, Chao Yu 님이 작성:
>
> As Park Ju Hyung suggested:
>
> "I'd like to suggest to write down an actual version of f2fs-tools
> here as we've seen older versions of fsck doing even more damage
> and the users might not have the latest f2fs-tools installed."
>
> This patch give a more detailed info of how we fix such corruption
> to user to avoid damageable repair with low version fsck.
>
> Signed-off-by: Park Ju Hyung 
> Signed-off-by: Chao Yu 
> ---
> v2:
> - add fixing patch's title in log suggested by Jaegeuk.
>  fs/f2fs/super.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 1a6d82d558e4..47dae7c3ae4f 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -2696,7 +2696,9 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
>
> if (__is_set_ckpt_flags(ckpt, CP_LARGE_NAT_BITMAP_FLAG) &&
> le32_to_cpu(ckpt->checksum_offset) != CP_MIN_CHKSUM_OFFSET) {
> -   f2fs_warn(sbi, "layout of large_nat_bitmap is deprecated, run 
> fsck to repair, chksum_offset: %u",
> +   f2fs_warn(sbi, "using deprecated layout of large_nat_bitmap, "
> + "please run fsck v1.13.0 or higher to repair, 
> chksum_offset: %u, "
> + "fixed with patch: \"f2fs-tools: relocate 
> chksum_offset for large_nat_bitmap feature\"",
>   le32_to_cpu(ckpt->checksum_offset));
> return 1;
> }
> --
> 2.18.0.rc1
>

Re: [f2fs-dev] [PATCH] f2fs: use kmem_cache pool during xattr lookups

[Ju Hyung Park]

Hi Chao and Jaegeuk,

I have no idea how that patch got merged.

We(me and Yaro) were supposed to work on doing some finishing touches
to the patch before sending it to upstream.

I'll personally check with Yaro.

Jaegeuk, please remove the patch.
That patch has numerous issues, biggest one being hardcoded size(SZ_256).

Also, I need to figure out how to allocate kmem cache per mounts.

Thanks.

On Mon, Jul 29, 2019 at 4:28 PM Chao Yu  wrote:
>
> Hi Jaegeuk, Ju Hyung, Yaroslav,
>
> I can see "f2fs: xattr: reserve cache for xattr allocations" has been merged 
> in
> dev-test branch, however, it doesn't exist in f2fs mailing list, so I can not
> comment on it Can anyone send it to the list?
>
> Thanks,


___
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: use kmem_cache pool during xattr lookups

[Ju Hyung Park]

Hi everyone.

This is a RFC patch.

This patch introduces an even bigger problem, which is forcing all
xattr lookup memory allocations to be made in 4076B, when in reality,
4076B allocations are only made during initial mounts and the rests
are made in 204B, unnecessarily wasting memory.

In my testing, 4076B allocations are only done 4 times during mount
and the rests(millions) are in 204B.

I'd like to ask the maintainers to suggest some bright ideas on how to
tackle this correctly.
(e.g. Use kmem pool only for 204B allocations and fallback to regular
kzalloc() if (*base_size != 204)?)

Thanks.

On Fri, Jul 12, 2019 at 12:06 AM Park Ju Hyung  wrote:
>
> It's been observed that kzalloc() on lookup_all_xattrs() are called millions
> of times on Android, quickly becoming the top abuser of slub memory allocator.
>
> Use a dedicated kmem cache pool for xattr lookups to mitigate this.
>
> Signed-off-by: Park Ju Hyung 
> ---
>  fs/f2fs/f2fs.h  |  6 ++
>  fs/f2fs/super.c |  8 +++-
>  fs/f2fs/xattr.c | 33 -
>  3 files changed, 37 insertions(+), 10 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 9c6388253c9d2..3046ca2ebd121 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3510,6 +3510,12 @@ void f2fs_exit_sysfs(void);
>  int f2fs_register_sysfs(struct f2fs_sb_info *sbi);
>  void f2fs_unregister_sysfs(struct f2fs_sb_info *sbi);
>
> +/*
> + * xattr.c
> + */
> +int __init f2fs_init_xattr_caches(void);
> +void f2fs_destroy_xattr_caches(void);
> +
>  /*
>   * crypto support
>   */
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 6d262d13251cf..abb59d9e25848 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -3614,9 +3614,12 @@ static int __init init_f2fs_fs(void)
> err = init_inodecache();
> if (err)
> goto fail;
> -   err = f2fs_create_node_manager_caches();
> +   err = f2fs_init_xattr_caches();
> if (err)
> goto free_inodecache;
> +   err = f2fs_create_node_manager_caches();
> +   if (err)
> +   goto fail_xattr_caches;
> err = f2fs_create_segment_manager_caches();
> if (err)
> goto free_node_manager_caches;
> @@ -3656,6 +3659,8 @@ static int __init init_f2fs_fs(void)
> f2fs_destroy_segment_manager_caches();
>  free_node_manager_caches:
> f2fs_destroy_node_manager_caches();
> +fail_xattr_caches:
> +   f2fs_destroy_xattr_caches();
>  free_inodecache:
> destroy_inodecache();
>  fail:
> @@ -3673,6 +3678,7 @@ static void __exit exit_f2fs_fs(void)
> f2fs_destroy_checkpoint_caches();
> f2fs_destroy_segment_manager_caches();
> f2fs_destroy_node_manager_caches();
> +   f2fs_destroy_xattr_caches();
> destroy_inodecache();
> f2fs_destroy_trace_ios();
>  }
> diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
> index e791741d193b8..635b50ea3e5e8 100644
> --- a/fs/f2fs/xattr.c
> +++ b/fs/f2fs/xattr.c
> @@ -22,6 +22,23 @@
>  #include "f2fs.h"
>  #include "xattr.h"
>
> +static struct kmem_cache *f2fs_xattr_cachep;
> +
> +int __init f2fs_init_xattr_caches(void)
> +{
> +   f2fs_xattr_cachep = f2fs_kmem_cache_create("xattr_entry",
> +   VALID_XATTR_BLOCK_SIZE + XATTR_PADDING_SIZE);
> +   if (!f2fs_xattr_cachep)
> +   return -ENOMEM;
> +
> +   return 0;
> +}
> +
> +void f2fs_destroy_xattr_caches(void)
> +{
> +   kmem_cache_destroy(f2fs_xattr_cachep);
> +}
> +
>  static int f2fs_xattr_generic_get(const struct xattr_handler *handler,
> struct dentry *unused, struct inode *inode,
> const char *name, void *buffer, size_t size)
> @@ -312,7 +329,7 @@ static int lookup_all_xattrs(struct inode *inode, struct 
> page *ipage,
> return -ENODATA;
>
> *base_size = XATTR_SIZE(xnid, inode) + XATTR_PADDING_SIZE;
> -   txattr_addr = f2fs_kzalloc(F2FS_I_SB(inode), *base_size, GFP_NOFS);
> +   txattr_addr = kmem_cache_zalloc(f2fs_xattr_cachep, GFP_NOFS);
> if (!txattr_addr)
> return -ENOMEM;
>
> @@ -358,7 +375,7 @@ static int lookup_all_xattrs(struct inode *inode, struct 
> page *ipage,
> *base_addr = txattr_addr;
> return 0;
>  out:
> -   kvfree(txattr_addr);
> +   kmem_cache_free(f2fs_xattr_cachep, txattr_addr);
> return err;
>  }
>
> @@ -367,13 +384,11 @@ static int read_all_xattrs(struct inode *inode, struct 
> page *ipage,
>  {
> struct f2fs_xattr_header *header;
> nid_t xnid = F2FS_I(inode)->i_xattr_nid;
> -   unsigned int size = VALID_XATTR_BLOCK_SIZE;
> unsigned int inline_size = inline_xattr_size(inode);
> void *txattr_addr;
> int err;
>
> -   txattr_addr = f2fs_kzalloc(F2FS_I_SB(inode),
> -   inline_size + size + XATTR_PADDING_SIZE, GFP_NOFS);
> +   txattr_addr = kmem_cache_zalloc(f2fs_xattr_cachep, GFP_NOFS);
> if (!txattr_addr)
>   

Re: [f2fs-dev] [PATCH v3 2/2] f2fs-tools: relocate chksum_offset for large_nat_bitmap feature

[Ju Hyung Park]

One more bump.

afaik, there's only about a week left.

Thanks.

On Thu, Jun 27, 2019 at 7:20 PM Chao Yu  wrote:
>
> Hi Ju Hyung,
>
> Thanks for the reminding.
>
> Jaegeuk, I can send the kernel patch after you tag a new version on fsck.
>
> Thanks,
>
> On 2019/6/27 17:12, Ju Hyung Park wrote:
> > Hi Jaegeuk and Chao.
> >
> > A little bump here.
> >
> > We still need to tag a new version of fsck and update f2fs kernel code
> > to tell which version users should use as we discussed earlier.
> > -rc is closing soon, so I felt I needed to remind you.
> >
> > Thanks.
> >
> > On Tue, Jun 4, 2019 at 10:48 AM Chao Yu  wrote:
> >>
> >> On 2019/6/4 4:27, Jaegeuk Kim wrote:
> >>> On 06/04, Ju Hyung Park wrote:
> >>>> Hi Jaegeuk and Chao,
> >>>>
> >>>> A little update I thought I might share.
> >>>>
> >>>> Just went through migrating my laptop to another SSD and I've setup
> >>>> f2fs from the beginning with mkfs -i from the master branch.
> >>>> No issue as of yet and the kernel is working fine as expected :)
> >>>
> >>> Cool, thanks for your test. :)
> >>
> >> Great, thanks for the continuous test and report. :)
> >>
> >> Thanks,
> >>
> >>>
> >>>>
> >>>> Thanks.
> >>>>
> >>>>
> >>>> ___
> >>>> 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 v3 2/2] f2fs-tools: relocate chksum_offset for large_nat_bitmap feature

[Ju Hyung Park]

Hi Jaegeuk and Chao.

A little bump here.

We still need to tag a new version of fsck and update f2fs kernel code
to tell which version users should use as we discussed earlier.
-rc is closing soon, so I felt I needed to remind you.

Thanks.

On Tue, Jun 4, 2019 at 10:48 AM Chao Yu  wrote:
>
> On 2019/6/4 4:27, Jaegeuk Kim wrote:
> > On 06/04, Ju Hyung Park wrote:
> >> Hi Jaegeuk and Chao,
> >>
> >> A little update I thought I might share.
> >>
> >> Just went through migrating my laptop to another SSD and I've setup
> >> f2fs from the beginning with mkfs -i from the master branch.
> >> No issue as of yet and the kernel is working fine as expected :)
> >
> > Cool, thanks for your test. :)
>
> Great, thanks for the continuous test and report. :)
>
> Thanks,
>
> >
> >>
> >> Thanks.
> >>
> >>
> >> ___
> >> 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 v3 2/2] f2fs-tools: relocate chksum_offset for large_nat_bitmap feature

[Ju Hyung Park]

Hi Jaegeuk and Chao,

A little update I thought I might share.

Just went through migrating my laptop to another SSD and I've setup
f2fs from the beginning with mkfs -i from the master branch.
No issue as of yet and the kernel is working fine as expected :)

Thanks.


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

Re: [f2fs-dev] [PATCH 2/2] mkfs.f2fs: make the default extensions list much more sensical

[Ju Hyung Park]

Hi Chao,

On Tue, May 28, 2019 at 7:19 PM Chao Yu  wrote:
> How about adding below extensions:
>
> "zip",
> "bin",
> "dat",
> "txt",

zip is capable of random updates. I didn't add bz2 for the same reason.
But I do agree that most users won't be constantly updating zip files.

I personally use my Android device with zip treated as cold, but I'm
not sure if it makes good sense to make it as the default that's
supposed to run under various scenarios.

How much different is the random write performance from cold to hot?

But I'm against the idea of adding the rest 3 extensions.
"bin" and "dat" is way too generic. You wouldn't know if a program
happens to heavily update files named .bin/.dat.

For txt, it won't be uncommon for a user to update it frequently.
Moreover, most txt size is pretty small anyways.

And finally, circling back to your original concern, we should be more
careful adding extensions as there's a limit.

Thanks.


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

Re: [f2fs-dev] [PATCH v2 2/2] f2fs: relocate chksum_offset for large_nat_bitmap feature

[Ju Hyung Park]

Hi Jaegeuk and Chao,

I was semi-forced today to use the new kernel and test f2fs.

My Ubuntu initramfs got a bit wonky and I had to boot into live CD and
fix some stuffs. The live CD was using 4.15 kernel, and just mounting
the f2fs partition there corrupted f2fs and my 4.19(with 5.1-rc1-4.19
f2fs-stable merged) refused to mount with "SIT is corrupted node"
message.

I used the latest f2fs-tools sent by Chao including "fsck.f2fs: fix to
repair cp_loads blocks at correct position"

It spit out 140M worth of output, but at least I didn't have to run it
twice. Everything returned "Ok" in the 2nd run.
The new log is at
http://arter97.com/f2fs/final

After fixing the image, I used my 4.19 kernel with 5.2-rc1-4.19
f2fs-stable merged and it mounted.

But, I got this:
[1.047791] F2FS-fs (nvme0n1p3): layout of large_nat_bitmap is
deprecated, run fsck to repair, chksum_offset: 4092
[1.081307] F2FS-fs (nvme0n1p3): Found nat_bits in checkpoint
[1.161520] F2FS-fs (nvme0n1p3): recover fsync data on readonly fs
[1.162418] F2FS-fs (nvme0n1p3): Mounted with checkpoint version = 761c7e00

But after doing a reboot, the message is gone:
[1.098423] F2FS-fs (nvme0n1p3): Found nat_bits in checkpoint
[1.11] F2FS-fs (nvme0n1p3): recover fsync data on readonly fs
[1.178365] F2FS-fs (nvme0n1p3): Mounted with checkpoint version = 761c7eda

I'm not exactly sure why the kernel detected that I'm still using the
old layout on the first boot. Maybe fsck didn't fix it properly, or
the check from the kernel is improper.

I also noticed that Jaegeuk sent v1 of this patch to upstream. (Maybe
that's why the kernel detected old layout?) Please send v2 to upstream
soon, as running older fsck will cause much more headaches.

Thanks.


On Fri, Apr 26, 2019 at 11:26 AM Chao Yu  wrote:
>
> For large_nat_bitmap feature, there is a design flaw:
>
> Previous:
>
> struct f2fs_checkpoint layout:
> +--+  0x
> | checkpoint_ver   |
> | ..   |
> | checksum_offset  |--+
> | ..   |  |
> | sit_nat_version_bitmap[] |<-|---+
> | ..   |  |   |
> | checksum_value   |<-+   |
> +--+  0x1000  |
> |  |  nat_bitmap + sit_bitmap
> | payload blocks   |  |
> |  |  |
> +--|<-+
>
> Obviously, if nat_bitmap size + sit_bitmap size is larger than
> MAX_BITMAP_SIZE_IN_CKPT, nat_bitmap or sit_bitmap may overlap
> checkpoint checksum's position, once checkpoint() is triggered
> from kernel, nat or sit bitmap will be damaged by checksum field.
>
> In order to fix this, let's relocate checksum_value's position
> to the head of sit_nat_version_bitmap as below, then nat/sit
> bitmap and chksum value update will become safe.
>
> After:
>
> struct f2fs_checkpoint layout:
> +--+  0x
> | checkpoint_ver   |
> | ..   |
> | checksum_offset  |--+
> | ..   |  |
> | sit_nat_version_bitmap[] |<-+
> | ..   |<-+
> |  |  |
> +--+  0x1000  |
> |  |  nat_bitmap + sit_bitmap
> | payload blocks   |  |
> |  |  |
> +--|<-+
>
> Related report and discussion:
>
> https://sourceforge.net/p/linux-f2fs/mailman/message/36642346/
>
> Reported-by: Park Ju Hyung 
> Signed-off-by: Chao Yu 
> ---
> v2:
> - improve hint message suggested by Ju Hyung.
> - move verification to f2fs_sanity_check_ckpt().
>  fs/f2fs/f2fs.h  |  4 +++-
>  fs/f2fs/super.c | 13 +
>  2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 119bc5a9783e..aa71c1aa9eaa 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1909,9 +1909,11 @@ static inline void *__bitmap_ptr(struct f2fs_sb_info 
> *sbi, int flag)
> int offset;
>
> if (is_set_ckpt_flags(sbi, CP_LARGE_NAT_BITMAP_FLAG)) {
> +   unsigned int chksum_size = sizeof(__le32);
> +
> offset = (flag == SIT_BITMAP) ?
> le32_to_cpu(ckpt->nat_ver_bitmap_bytesize) : 0;
> -   return >sit_nat_version_bitmap + offset;
> +   return >sit_nat_version_bitmap + offset + chksum_size;
> }
>
> if (__cp_payload(sbi) > 0) {
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index fefc8cc6e756..22241bb866df 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -2714,6 +2714,19 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> return 1;
> }
>
> +   if (__is_set_ckpt_flags(ckpt, CP_LARGE_NAT_BITMAP_FLAG)) {
> +   unsigned int chksum_offset;
> +
> +   

Re: [f2fs-dev] [PATCH v3 2/2] f2fs-tools: relocate chksum_offset for large_nat_bitmap feature

[Ju Hyung Park]

Hi Chao,

On Sun, May 19, 2019 at 2:09 PM Chao Yu  wrote:
> I've found one bug when repairing cp_payload blocks, could you try it?
>
> [PATCH] fsck.f2fs: fix to repair cp_loads blocks at correct position

http://arter97.com/f2fs/v3__cp

After the patch, 2nd, 3rd and the 4th run all returns the same output.
I think now fsck only needs to run once :)

Final question though, is it expected that the first run to print 62MB
worth of logs?

Thanks.


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

Re: [f2fs-dev] [PATCH v3 2/2] f2fs-tools: relocate chksum_offset for large_nat_bitmap feature

[Ju Hyung Park]

Hey Chao,

Sorry for the late reply.

On Wed, May 15, 2019 at 6:23 PM Chao Yu  wrote:
> So I doubt that sit version bitmap is corrupted, could you add below log and
> reproduce this issue?

Just done it. The logs are at http://arter97.com/f2fs/v3__/

Thanks :)


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

Re: [f2fs-dev] [PATCH v3 2/2] f2fs-tools: relocate chksum_offset for large_nat_bitmap feature

[Ju Hyung Park]

Hi Chao,

The new logs are at
http://arter97.com/f2fs/v3

The first run spits out 60MB worth of log
and the second run spits out 170MB worth of log
and the third run now returns everything's ok.

Returning ok after 2nd run is good news, but the user would expect
everything to be fixed with just the first run of fsck'ing. Is this
expected?

I'll test the new kernel soon.

Thanks.

On Tue, May 14, 2019 at 6:54 PM Ju Hyung Park  wrote:
>
> Ah, sorry. I typed too soon.
>
> I'll make sure to test this sooner than later.
> Sorry for the delay.
>
> Thanks :)
>
> On Tue, May 14, 2019, 6:43 PM Chao Yu  wrote:
>>
>> Hi Ju Hyung,
>>
>> This is the change on tools part. ;)
>>
>> Thanks,
>>
>> On 2019/5/14 17:38, Ju Hyung Park wrote:
>> > Hi Chao,
>> >
>> > I believe Jaegeuk already queued v2 for Linus, I think you should probably
>> > make this as a separate patch.
>> >
>> > Thanks.
>> >
>> > On Tue, May 14, 2019, 6:35 PM Chao Yu  wrote:
>> >
>> >> For large_nat_bitmap feature, there is a design flaw:
>> >>
>> >> Previous:
>> >>
>> >> struct f2fs_checkpoint layout:
>> >> +--+  0x
>> >> | checkpoint_ver   |
>> >> | ..   |
>> >> | checksum_offset  |--+
>> >> | ..   |  |
>> >> | sit_nat_version_bitmap[] |<-|---+
>> >> | ..   |  |   |
>> >> | checksum_value   |<-+   |
>> >> +--+  0x1000  |
>> >> |  |  nat_bitmap + sit_bitmap
>> >> | payload blocks   |  |
>> >> |  |  |
>> >> +--|<-+
>> >>
>> >> Obviously, if nat_bitmap size + sit_bitmap size is larger than
>> >> MAX_BITMAP_SIZE_IN_CKPT, nat_bitmap or sit_bitmap may overlap
>> >> checkpoint checksum's position, once checkpoint() is triggered
>> >> from kernel, nat or sit bitmap will be damaged by checksum field.
>> >>
>> >> In order to fix this, let's relocate checksum_value's position
>> >> to the head of sit_nat_version_bitmap as below, then nat/sit
>> >> bitmap and chksum value update will become safe.
>> >>
>> >> After:
>> >>
>> >> struct f2fs_checkpoint layout:
>> >> +--+  0x
>> >> | checkpoint_ver   |
>> >> | ..   |
>> >> | checksum_offset  |--+
>> >> | ..   |  |
>> >> | sit_nat_version_bitmap[] |<-+
>> >> | ..   |<-+
>> >> |  |  |
>> >> +--+  0x1000  |
>> >> |  |  nat_bitmap + sit_bitmap
>> >> | payload blocks   |  |
>> >> |  |  |
>> >> +--|<-+
>> >>
>> >> Related report and discussion:
>> >>
>> >> https://sourceforge.net/p/linux-f2fs/mailman/message/36642346/
>> >>
>> >> In addition, during writing checkpoint, if large_nat_bitmap feature is
>> >> enabled, we need to set CP_LARGE_NAT_BITMAP_FLAG flag in checkpoint.
>> >>
>> >> Reported-by: Park Ju Hyung 
>> >> Signed-off-by: Chao Yu 
>> >> ---
>> >> v3:
>> >> - if large_nat_bitmap is off, fix to configure checksum_offset to
>> >> CP_CHKSUM_OFFSET.
>> >>  fsck/f2fs.h|  9 -
>> >>  fsck/fsck.c| 37 +
>> >>  fsck/fsck.h|  1 +
>> >>  fsck/main.c|  2 ++
>> >>  fsck/mount.c   |  9 -
>> >>  fsck/resize.c  |  5 +
>> >>  include/f2fs_fs.h  | 10 --
>> >>  mkfs/f2fs_format.c |  5 -
>> >>  8 files changed, 73 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/fsck/f2fs.h b/fsck/f2fs.h
>> >> index 93f01e5..4dc6698 100644
>> >> --- a/fsck/f2fs.h
>> >> +++ b/fsck/f2fs.h
>> >> @@ -270,9 +270,16 @@ static inline void *__bitmap_ptr(struct f2fs_sb_info
>> >> *sbi, int flag)
>>

Re: [f2fs-dev] [PATCH 1/3] f2fs: remove sleep_time under gc_urgent

[Ju Hyung Park]

Hi Chao,

On Tue, May 14, 2019 at 8:19 PM Chao Yu  wrote:
> > I've been using this(with a slightly different code) for years and yet to 
> > notice
> > any spikes in lags/slowdowns. Worst scenario, I'd just have to deal with an
> > added split second(100ms max?) delay in screen wake-up.
>
> I'm not sure about why this happened... maybe you need to do some test to
> analyse the root cause of it, filesystem/device fragment? too many undiscard
> space? or non-storage issue?

Um, I'm not sure you understood what I said.
What I meant is that I haven't found any issues with using an approach
like this(gc_urgent with 1ms sleep intervals) for years on various
Android devices.

> I agreed that it should done as soon as possible, but it needs to consider IO
> race in between Apps after screen wake-up and BGGC to avoid potential ANR.

I actually need to check whether vold turns off gc_urgent immediately
after screen turns itself back on.
I don't think we need to take potential ANR in to account *if* vold
stops gc_urgent right after screen-on. What do you think?

> It's userspace strategy, we can change both of them
> (vold_wait_time/gc_urgent_sleep_time) in userspace if current value doesn't 
> make
> any sense.

Even the user can set the tunables themselves, the default should be
sensical imo.
An "urgent" GC that only GCs up-to 2 segments per second doesn't sound
that "urgent" :p

Thanks.


___
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: issue discard commands proactively in high fs utilization

[Ju Hyung Park]

On Wed, May 30, 2018 at 5:51 AM Jaegeuk Kim  wrote:
>
> In the high utilization like over 80%, we don't expect huge # of large discard
> commands, but do many small pending discards which affects FTL GCs a lot.
> Let's issue them in that case.
>
> Signed-off-by: Jaegeuk Kim 
> ---
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 6e40e536dae0..8c1f7a6bf178 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -915,6 +915,38 @@ static void __check_sit_bitmap(struct f2fs_sb_info *sbi,
> +   dpolicy->max_interval = DEF_MIN_DISCARD_ISSUE_TIME;

Isn't this way too aggressive?

Discard thread will wake up on 50ms interval just because the user has
used 80% of space.
60,000ms vs 50ms is too much of a stark difference.

I feel something like 10 seconds(10,000ms) could be a much more
reasonable choice than this.

Thanks.


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

Re: [f2fs-dev] [PATCH v4] f2fs: add bio cache for IPU

[Ju Hyung Park]

Hi,

Is this still causing hangs?
Just wondering why this isn't merged yet.

Thanks.

On Sat, Mar 2, 2019 at 2:55 AM Jaegeuk Kim  wrote:
>
> On 02/28, Chao Yu wrote:
> > Ping,
>
> Ditto.
>
> >
> > On 2019/2/19 16:15, Chao Yu wrote:
> > > SQLite in Wal mode may trigger sequential IPU write in db-wal file, after
> > > commit d1b3e72d5490 ("f2fs: submit bio of in-place-update pages"), we
> > > lost the chance of merging page in inner managed bio cache, result in
> > > submitting more small-sized IO.
> > >
> > > So let's add temporary bio in writepages() to cache mergeable write IO as
> > > much as possible.
> > >
> > > Test case:
> > > 1. xfs_io -f /mnt/f2fs/file -c "pwrite 0 65536" -c "fsync"
> > > 2. xfs_io -f /mnt/f2fs/file -c "pwrite 0 65536" -c "fsync"
> > >
> > > Before:
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector 
> > > = 65544, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector 
> > > = 65552, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector 
> > > = 65560, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector 
> > > = 65568, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector 
> > > = 65576, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector 
> > > = 65584, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector 
> > > = 65592, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector 
> > > = 65600, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector 
> > > = 65608, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector 
> > > = 65616, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector 
> > > = 65624, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector 
> > > = 65632, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector 
> > > = 65640, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector 
> > > = 65648, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector 
> > > = 65656, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector 
> > > = 65664, size = 4096
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), NODE, sector 
> > > = 57352, size = 4096
> > >
> > > After:
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), DATA, sector 
> > > = 65544, size = 65536
> > > f2fs_submit_write_bio: dev = (251,0)/(251,0), rw = WRITE(S), NODE, sector 
> > > = 57368, size = 4096
> > >
> > > Signed-off-by: Chao Yu 
> > > ---
> > > v4:
> > > - fix to set *bio to NULL in f2fs_submit_ipu_bio()
> > > - spread f2fs_submit_ipu_bio()
> > > - fix to count dirty encrypted page correctly in f2fs_merge_page_bio()
> > > - remove redundant assignment of fio->last_block
> > >  fs/f2fs/data.c| 88 ++-
> > >  fs/f2fs/f2fs.h|  3 ++
> > >  fs/f2fs/segment.c |  5 ++-
> > >  3 files changed, 86 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > > index 35910ff23582..e4c183e85de8 100644
> > > --- a/fs/f2fs/data.c
> > > +++ b/fs/f2fs/data.c
> > > @@ -296,20 +296,20 @@ static void __submit_merged_bio(struct 
> > > f2fs_bio_info *io)
> > > io->bio = NULL;
> > >  }
> > >
> > > -static bool __has_merged_page(struct f2fs_bio_info *io, struct inode 
> > > *inode,
> > > +static bool __has_merged_page(struct bio *bio, struct inode *inode,
> > > struct page *page, nid_t ino)
> > >  {
> > > struct bio_vec *bvec;
> > > struct page *target;
> > > int i;
> > >
> > > -   if (!io->bio)
> > > +   if (!bio)
> > > return false;
> > >
> > > if (!inode && !page && !ino)
> > > return true;
> > >
> > > -   bio_for_each_segment_all(bvec, io->bio, i) {
> > > +   bio_for_each_segment_all(bvec, bio, i) {
> > >
> > > if (bvec->bv_page->mapping)
> > > target = bvec->bv_page;
> > > @@ -360,7 +360,7 @@ static void __submit_merged_write_cond(struct 
> > > f2fs_sb_info *sbi,
> > > struct f2fs_bio_info *io = sbi->write_io[btype] + 
> > > temp;
> > >
> > > down_read(>io_rwsem);
> > > -   ret = __has_merged_page(io, inode, page, ino);
> > > +   ret = __has_merged_page(io->bio, inode, page, ino);
> > > up_read(>io_rwsem);
> > > }
> > > if (ret)
> > > @@ -429,6 +429,61 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
> > > return 0;
> > >  }
> > >
> > > +int f2fs_merge_page_bio(struct f2fs_io_info *fio)
> > 

Re: [f2fs-dev] [PATCH] f2fs: fix to do sanity with enabled features in image

[Ju Hyung Park]

Hi Chao and Jaegeuk,

On Wed, Apr 24, 2019 at 6:49 PM Chao Yu  wrote:
>
> This patch fixes to do sanity with enabled features in image, if
> there are features kernel can not recognize, just fail the mount.
>

Surprised to see that this wasn't implemented yet.
I was actually about to suggest this method to prevent mounting of the
new extended bitmap layout images altogether for older kernels(by
renaming the new, fixed layout to v2 or something), but looks like
that isn't an option. :(

Also, something similar should be also done with fsck, if not already.
The results from using older fsck with images with newer features
would be disastrous.

I'm still very busy currently with my other projects.
Sorry for the delays in testing new patchsets for layout fixes.

And I apologize in advance in case I miss the Linux 5.2 merge window
deadline, but I'd like to see it being fixed properly before shipping
those patches to production.

> +   /* check whether kernel supports all features */
> +   if (le32_to_cpu(raw_super->feature) & (~F2FS_ALL_FEATURES)) {
> +   f2fs_msg(sb, KERN_INFO,
> +   "Unsupported feature:%u: supported:%u",
> +   le32_to_cpu(raw_super->feature),
> +   F2FS_ALL_FEATURES);
> +   return 1;
> +   }

This should probably be a KERN_ERR instead of KERN_INFO.

Thanks.


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

Re: [f2fs-dev] [PATCH 2/2] f2fs-tools: relocate chksum_offset for large_nat_bitmap feature

[Ju Hyung Park]

Hi Chao and Jaegeuk,

I'm currently unavailable to recompile the kernel and use the new
layout scheme but I ran the new fsck and attempted to mount it on the
older kernel.

The used fsck-tools is at f50234c248532528b217efff5861b29fe6ec1b36
("f2fs-tools: Fix device zoned model detection").
fsck still behaves weird, I believe.

The new logs are at
http://arter97.com/f2fs/f50234c248532528b217efff5861b29fe6ec1b36

The first run spits out 600MB worth of log
and the second run spits out 60MB worth of log
and the third run fails to fix "other corrupted bugs".

Runs after the third prints out the same log.

When I try to mount the image after third run on the older kernel, it fails:
[ 1710.824598] F2FS-fs (loop1): invalid crc value
[ 1710.855240] F2FS-fs (loop1): SIT is corrupted node# 2174236 vs 2517451
[ 1710.855243] F2FS-fs (loop1): Failed to initialize F2FS segment manager

I'll report back after I compile and run the kernel with the new
layout scheme, but I don't feel good about this after fsck printing
that much logs, and the fact that the 2nd run still spits out a lot of
logs.

Thanks.

On Mon, Apr 22, 2019 at 6:35 PM Chao Yu  wrote:
>
> For large_nat_bitmap feature, there is a design flaw:
>
> Previous:
>
> struct f2fs_checkpoint layout:
> +--+  0x
> | checkpoint_ver   |
> | ..   |
> | checksum_offset  |--+
> | ..   |  |
> | sit_nat_version_bitmap[] |<-|---+
> | ..   |  |   |
> | checksum_value   |<-+   |
> +--+  0x1000  |
> |  |  nat_bitmap + sit_bitmap
> | payload blocks   |  |
> |  |  |
> +--|<-+
>
> Obviously, if nat_bitmap size + sit_bitmap size is larger than
> MAX_BITMAP_SIZE_IN_CKPT, nat_bitmap or sit_bitmap may overlap
> checkpoint checksum's position, once checkpoint() is triggered
> from kernel, nat or sit bitmap will be damaged by checksum field.
>
> In order to fix this, let's relocate checksum_value's position
> to the head of sit_nat_version_bitmap as below, then nat/sit
> bitmap and chksum value update will become safe.
>
> After:
>
> struct f2fs_checkpoint layout:
> +--+  0x
> | checkpoint_ver   |
> | ..   |
> | checksum_offset  |--+
> | ..   |  |
> | sit_nat_version_bitmap[] |<-+
> | ..   |<-+
> |  |  |
> +--+  0x1000  |
> |  |  nat_bitmap + sit_bitmap
> | payload blocks   |  |
> |  |  |
> +--|<-+
>
> Related report and discussion:
>
> https://sourceforge.net/p/linux-f2fs/mailman/message/36642346/
>
> In addition, during writing checkpoint, if large_nat_bitmap feature is
> enabled, we need to set CP_LARGE_NAT_BITMAP_FLAG flag in checkpoint.
>
> Reported-by: Park Ju Hyung 
> Signed-off-by: Chao Yu 
> ---
>  fsck/f2fs.h|  6 +-
>  fsck/fsck.c| 16 
>  fsck/fsck.h|  1 +
>  fsck/main.c|  2 ++
>  fsck/mount.c   | 42 +++---
>  include/f2fs_fs.h  |  9 +++--
>  mkfs/f2fs_format.c |  5 -
>  7 files changed, 62 insertions(+), 19 deletions(-)
>
> diff --git a/fsck/f2fs.h b/fsck/f2fs.h
> index 93f01e5..6a6c4a5 100644
> --- a/fsck/f2fs.h
> +++ b/fsck/f2fs.h
> @@ -272,7 +272,11 @@ static inline void *__bitmap_ptr(struct f2fs_sb_info 
> *sbi, int flag)
> if (is_set_ckpt_flags(ckpt, CP_LARGE_NAT_BITMAP_FLAG)) {
> offset = (flag == SIT_BITMAP) ?
> le32_to_cpu(ckpt->nat_ver_bitmap_bytesize) : 0;
> -   return >sit_nat_version_bitmap + offset;
> +   /*
> +* if large_nat_bitmap feature is enabled, leave checksum
> +* protection for all nat/sit bitmaps.
> +*/
> +   return >sit_nat_version_bitmap + offset + 
> sizeof(__le32);
> }
>
> if (le32_to_cpu(F2FS_RAW_SUPER(sbi)->cp_payload) > 0) {
> diff --git a/fsck/fsck.c b/fsck/fsck.c
> index 7ecdee1..8d7deb5 100644
> --- a/fsck/fsck.c
> +++ b/fsck/fsck.c
> @@ -1917,6 +1917,18 @@ int fsck_chk_meta(struct f2fs_sb_info *sbi)
> return 0;
>  }
>
> +void fsck_chk_checkpoint(struct f2fs_sb_info *sbi)
> +{
> +   struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
> +
> +   if (get_cp(ckpt_flags) & CP_LARGE_NAT_BITMAP_FLAG) {
> +   if (get_cp(checksum_offset) != CP_MIN_CHKSUM_OFFSET) {
> +   ASSERT_MSG("Deprecated layout of large_nat_bitmap, "
> +   "chksum_offset:%u", get_cp(checksum_offset));
> +   }
> +   }
> +}
> +
>  void fsck_init(struct f2fs_sb_info 

Re: [f2fs-dev] [PATCH 2/2] f2fs: relocate chksum_offset for large_nat_bitmap feature

[Ju Hyung Park]

Hi Chao,

On Mon, Apr 22, 2019 at 6:34 PM Chao Yu  wrote:
> +   if (__is_set_ckpt_flags(*cp_block, CP_LARGE_NAT_BITMAP_FLAG)) {
> +   if (crc_offset != CP_MIN_CHKSUM_OFFSET) {
> +   f2fs_put_page(*cp_page, 1);
> +   f2fs_msg(sbi->sb, KERN_WARNING,
> +   "layout of large_nat_bitmap is deprecated, "
> +   "run fsck to repair, chksum_offset: %zu",
> +   crc_offset);
> +   return -EINVAL;
> +   }
> +   }
> +

I try not to be a Grammar Nazi and a jerk on every patches, but since
this is a message a user would directly encounter, I'd like to see a
bit less ambiguous message.

How about "using deprecated layout of large_nat_bitmap, please run
fsck v1.13.0 or higher to repair, chksum_offset: %zu"?
The original message seems to suggest that large_nat_bitmap is
deprecated outright.

Also, I'd like to suggest to write down an actual version of
f2fs-tools here as we've seen older versions of fsck doing even more
damage and the users might not have the latest f2fs-tools installed.

Btw, what happens if we use the latest fsck to fix the corrupted image
and use the older kernel to mount it?
Would it even mount?

Thanks.


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

Re: [f2fs-dev] [PATCH] fsck.f2fs: fix to set CP_LARGE_NAT_BITMAP_FLAG

[Ju Hyung Park]

Hi Chao and Jaegeuk.

On Fri, Apr 19, 2019 at 12:46 PM Chao Yu  wrote:
> I've sent two patches, could you please try them?
>
> Thanks

Thanks for the patches.

So it seems like I need to reformat my f2fs partitions, please confirm
if that's the case.
Also, if these patches break backward-compatibility, wouldn't it be
better to rename the feature and mark the old one deprecated?

I would like to see Jaegeuk's review before mkfs'ing again.

Thanks.


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

Re: [f2fs-dev] f2fs: SIT corruption upon fsck

[Ju Hyung Park]

Hi Jaegeuk,

On Thu, Apr 18, 2019 at 12:51 AM Jaegeuk Kim  wrote:
> nvm. I shoot the email too early. It seems that large_nat_bitmap gave old NAT
> set somehow, so that NAT points the previous checkpointed node blocks, since
> SSA were obsolete and some SIT were valid and others were invalid.
>
> When considering no node block was missing from dentry, I think GC was doing
> between two checkpoints.

For some contexts, this is the same issue I think I reported about 6
months ago, under a very similar scenario. I didn't have the luxury
back then to backup the original working image.

I was re-formatting my f2fs partition as I didn't use the large bitmap
option before. I backed up all the data via 'rsync -a' to another
filesystem, formatted via mkfs.f2fs -i, restored the data back to the
newly formatted f2fs partition(also via rsync) and did a gc_urgent
with 1ms gc_urgent_sleep_time. IIRC GC ran about 5,000 times.

After all that, deleting files from the newly formatted f2fs partition
randomly gave me truncation warnings.
I was just able to reproduce this truncation warning by taking another
btrfs snapshot of the original image and doing 'rm -rf /home'.

[ 4756.407571] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343043
[ 4756.407574] WARNING: CPU: 1 PID: 6473 at fs/f2fs/segment.c:2165
update_sit_entry+0x3a4/0x3f0
[ 4756.407590] CPU: 1 PID: 6473 Comm: rm Tainted: GW  O
4.19.35-zen+ #1
[ 4756.407591] Hardware name: System manufacturer System Product
Name/ROG MAXIMUS X HERO (WI-FI AC), BIOS 1704 09/14/2018
[ 4756.407591] RIP: 0010:update_sit_entry+0x3a4/0x3f0
[ 4756.407592] Code: ff ff 48 8b 7d 00 44 89 f9 48 c7 c2 e0 e0 58 82
48 c7 c6 93 96 54 82 44 89 4c 24 14 4c 89 44 24 08 44 88 14 24 e8 68
29 fe ff <0f> 0b f0 80 4d 48 04 66 83 03 40 45 31 ed 31 c9 44 0f b6 14
24 4c
[ 4756.407592] RSP: 0018:88849ace3c40 EFLAGS: 00010286
[ 4756.407593] RAX: 0039 RBX: c90006442fd8 RCX: 0006
[ 4756.407593] RDX: 0007 RSI: 0086 RDI: 4ec56340
[ 4756.407593] RBP: 8884b5ee3800 R08: 00063813 R09: 0026e1d0
[ 4756.407594] R10:  R11: 82d4c26d R12: 0001b3ff
[ 4756.407594] R13:  R14: 0001b3ff R15: 036afc43
[ 4756.407594] FS:  7fd799482780() GS:4ec4()
knlGS:
[ 4756.407595] CS:  0010 DS:  ES:  CR0: 80050033
[ 4756.407595] CR2: 36195d5f4000 CR3: 0005bb1d9002 CR4: 003606e0
[ 4756.407595] DR0:  DR1:  DR2: 
[ 4756.407596] DR3:  DR6: fffe0ff0 DR7: 0400
[ 4756.407596] Call Trace:
[ 4756.407597]  ? f2fs_invalidate_blocks+0x83/0x110
[ 4756.407598]  ? f2fs_truncate_data_blocks+0xb9/0x340
[ 4756.407598]  ? truncate_nodes+0x39c/0x570
[ 4756.407599]  ? f2fs_truncate_inode_blocks+0x1a0/0x530
[ 4756.407600]  ? f2fs_truncate_blocks+0x179/0x3e0
[ 4756.407601]  ? f2fs_truncate+0x7a/0xf0
[ 4756.407602]  ? f2fs_evict_inode+0x2f6/0x380
[ 4756.407602]  ? evict+0xbf/0x1e0
[ 4756.407603]  ? do_unlinkat+0x27e/0x330
[ 4756.407604]  ? do_syscall_64+0x4a/0xf0
[ 4756.407604]  ? entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 4756.407605] ---[ end trace d1bb53850401c466 ]---

I get ton of these during 'rm' with blk incrementing.
This is the same log but with "F2FS" grepped:

[ 4756.403795] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343002
[ 4756.403888] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343003
[ 4756.404012] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343004
[ 4756.404119] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343005
[ 4756.404213] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343006
[ 4756.404330] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343007
[ 4756.404443] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343008
[ 4756.404537] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343009
[ 4756.404657] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343010
[ 4756.404767] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343011
[ 4756.404860] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343012
[ 4756.404979] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343013
[ 4756.405090] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343014
[ 4756.405183] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343015
[ 4756.405301] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343016
[ 4756.405414] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343017
[ 4756.405506] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343018
[ 4756.405626] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343019
[ 4756.405736] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343020
[ 4756.405831] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343021
[ 4756.405948] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343022
[ 4756.406065] F2FS-fs (loop1): Bitmap was wrongly cleared, blk:57343023
[ 

Re: [f2fs-dev] f2fs: SIT corruption upon fsck

[Ju Hyung Park]

Oh sorry, I should have been more careful attaching the kernel log.
Disregard anything before 3091s mark and just pay attention to those 2 lines:

> [ 3091.204798] F2FS-fs (loop1): SIT is corrupted node# 2481418 vs 2517451
> [ 3091.204801] F2FS-fs (loop1): Failed to initialize F2FS segment manager

Thanks.


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

Re: [f2fs-dev] f2fs: SIT corruption upon fsck

[Ju Hyung Park]

This is an additional f2fs status from the original image when mounted
with 5.1-rc1-4.19.

Thanks.

=[ partition info(loop1). #1, RO, CP: Good]=
[SB: 1] [CP: 2] [SIT: 10] [NAT: 114] [SSA: 256] [MAIN:
130697(OverProv:1027 Resv:520)]

Utilization: 52% (34963544 valid blocks, 32181458 discard blocks)
  - Node: 2517444 (Inode: 2494236, Other: 23208)
  - Data: 32446100
  - Inline_xattr Inode: 0
  - Inline_data Inode: 0
  - Inline_dentry Inode: 0
  - Orphan/Append/Update Inode: 0, 0, 0

Main area: 130697 segs, 130697 secs 130697 zones
  - COLD  data: 21160, 21160, 21160
  - WARM  data: 22632, 22632, 22632
  - HOT   data: 2552, 2552, 2552
  - Dir   dnode: 2547, 2547, 2547
  - File   dnode: 2551, 2551, 2551
  - Indir nodes: 2486, 2486, 2486

  - Valid: 65216
  - Dirty: 4445
  - Prefree: 0
  - Free: 61036 (61036)

CP calls: 0 (BG: 0)
  - cp blocks : 0
  - sit blocks : 0
  - nat blocks : 0
  - ssa blocks : 0
GC calls: 0 (BG: 0)
  - data segments : 0 (0)
  - node segments : 0 (0)
Try to move 0 blocks (BG: 0)
  - data blocks : 0 (0)
  - node blocks : 0 (0)
Skipped : atomic write 0 (0)
BG skip : IO: 0, Other: 0

Extent Cache:
  - Hit Count: L1-1:0 L1-2:0 L2:0
  - Hit Ratio: 0% (0 / 0)
  - Inner Struct Count: tree: 0(0), node: 0

Balancing F2FS Async:
  - DIO (R:0, W:0)
  - IO_R (Data:0, Node:0, Meta:0
  - IO_W (CP:0, Data:0, Flush: (   000), Discard: (
00)) cmd:0 undiscard:   0
  - inmem:0, atomic IO:0 (Max.0), volatile IO:0 (Max.0)
  - nodes:0 in1
  - dents:0 in dirs:   0 (   0)
  - datas:0 in files:   0
  - quota datas:0 in quota files:   0
  - meta:0 in 2399
  - imeta:0
  - NATs: 7/8
  - SITs: 6/   130697
  - free_nids:  3224/ 10761266
  - alloc_nids: 0

Distribution of User Blocks: [ valid | invalid | free ]
  [--|-|---]

IPU: 0 blocks
SSR: 0 blocks in 0 segments
LFS: 0 blocks in 0 segments

BDF: 98, avg. vblocks: 302

Memory: 41409 KB
  - static: 31725 KB
  - cached: 84 KB
  - paged : 9600 KB

On Wed, Apr 17, 2019 at 7:25 PM Ju Hyung Park  wrote:
>
> Hi,
>
> I was noticing some truncation warnings on the kernel logs so I went
> ahead and ran fsck.
>
> I dumped the image to a btrfs snapshot just in case if fsck fails and
> corrupts the image even more, and turns out it did.
>
> The image is formatted with large nat bitmap enabled and using
> mkfs.f2fs v1.12.0.
> The fsck is from the latest master branch.
>
> After fsck, the image fails to mount at all:
> [ 3052.298358] [ cut here ]
> [ 3052.298359] generic_make_request: Trying to write to read-only
> block-device loop1 (partno 0)
> [ 3052.298368] WARNING: CPU: 1 PID: 3854 at block/blk-core.c:2174
> generic_make_request_checks+0x590/0x630
> [ 3052.298394] CPU: 1 PID: 3854 Comm: umount Tainted: G   O
>   4.19.35-zen+ #1
> [ 3052.298394] Hardware name: System manufacturer System Product
> Name/ROG MAXIMUS X HERO (WI-FI AC), BIOS 1704 09/14/2018
> [ 3052.298395] RIP: 0010:generic_make_request_checks+0x590/0x630
> [ 3052.298396] Code: 5c 03 00 00 48 8d 74 24 08 48 89 df c6 05 b5 cd
> 36 01 01 e8 c2 90 01 00 48 89 c6 44 89 ea 48 c7 c7 98 64 59 82 e8 d5
> 9b a7 ff <0f> 0b 48 8b 7b 08 e9 f2 fa ff ff 41 8b 86 98 02 00 00 49 8b
> 16 89
> [ 3052.298397] RSP: 0018:8882bcc97950 EFLAGS: 00010282
> [ 3052.298397] RAX: 0050 RBX: 8883f22f7500 RCX: 
> 0006
> [ 3052.298398] RDX: 0007 RSI: 0086 RDI: 
> 4ec56340
> [ 3052.298398] RBP: 437fa6c0 R08: 0004 R09: 
> 0405
> [ 3052.298398] R10: 0001 R11: 06f3 R12: 
> 1000
> [ 3052.298399] R13:  R14: 8886eb92d800 R15: 
> 8882bcc97ac0
> [ 3052.298399] FS:  7f0471dae8c0() GS:4ec4()
> knlGS:
> [ 3052.298400] CS:  0010 DS:  ES:  CR0: 80050033
> [ 3052.298400] CR2: 7f04727302c0 CR3: 000213c6f004 CR4: 
> 003606e0
> [ 3052.298400] DR0:  DR1:  DR2: 
> 
> [ 3052.298401] DR3:  DR6: fffe0ff0 DR7: 
> 0400
> [ 3052.298401] Call Trace:
> [ 3052.298403]  ? generic_make_request+0x46/0x3d0
> [ 3052.298404]  ? wait_woken+0x80/0x80
> [ 3052.298405]  ? mempool_alloc+0xb7/0x1a0
> [ 3052.298406]  ? submit_bio+0x30/0x110
> [ 3052.298407]  ? bvec_alloc+0x7c/0xd0
> [ 3052.298409]  ? __submit_merged_bio+0x68/0x390
> [ 3052.298410]  ? f2fs_submit_page_write+0x1bb/0x7f0
> [ 3052.298410]  ? f2fs_do_write_meta_page+0x7f/0x160
> [ 3052.298411]  ? __f2fs_write_meta_page+0x70/0x140
> [ 3052.298412]  ? f2fs_sync_meta_pages+0x140/0x250
> [ 3052.298413]  ? f2fs_wri

[f2fs-dev] f2fs: SIT corruption upon fsck

[Ju Hyung Park]

Hi,

I was noticing some truncation warnings on the kernel logs so I went
ahead and ran fsck.

I dumped the image to a btrfs snapshot just in case if fsck fails and
corrupts the image even more, and turns out it did.

The image is formatted with large nat bitmap enabled and using
mkfs.f2fs v1.12.0.
The fsck is from the latest master branch.

After fsck, the image fails to mount at all:
[ 3052.298358] [ cut here ]
[ 3052.298359] generic_make_request: Trying to write to read-only
block-device loop1 (partno 0)
[ 3052.298368] WARNING: CPU: 1 PID: 3854 at block/blk-core.c:2174
generic_make_request_checks+0x590/0x630
[ 3052.298394] CPU: 1 PID: 3854 Comm: umount Tainted: G   O
  4.19.35-zen+ #1
[ 3052.298394] Hardware name: System manufacturer System Product
Name/ROG MAXIMUS X HERO (WI-FI AC), BIOS 1704 09/14/2018
[ 3052.298395] RIP: 0010:generic_make_request_checks+0x590/0x630
[ 3052.298396] Code: 5c 03 00 00 48 8d 74 24 08 48 89 df c6 05 b5 cd
36 01 01 e8 c2 90 01 00 48 89 c6 44 89 ea 48 c7 c7 98 64 59 82 e8 d5
9b a7 ff <0f> 0b 48 8b 7b 08 e9 f2 fa ff ff 41 8b 86 98 02 00 00 49 8b
16 89
[ 3052.298397] RSP: 0018:8882bcc97950 EFLAGS: 00010282
[ 3052.298397] RAX: 0050 RBX: 8883f22f7500 RCX: 0006
[ 3052.298398] RDX: 0007 RSI: 0086 RDI: 4ec56340
[ 3052.298398] RBP: 437fa6c0 R08: 0004 R09: 0405
[ 3052.298398] R10: 0001 R11: 06f3 R12: 1000
[ 3052.298399] R13:  R14: 8886eb92d800 R15: 8882bcc97ac0
[ 3052.298399] FS:  7f0471dae8c0() GS:4ec4()
knlGS:
[ 3052.298400] CS:  0010 DS:  ES:  CR0: 80050033
[ 3052.298400] CR2: 7f04727302c0 CR3: 000213c6f004 CR4: 003606e0
[ 3052.298400] DR0:  DR1:  DR2: 
[ 3052.298401] DR3:  DR6: fffe0ff0 DR7: 0400
[ 3052.298401] Call Trace:
[ 3052.298403]  ? generic_make_request+0x46/0x3d0
[ 3052.298404]  ? wait_woken+0x80/0x80
[ 3052.298405]  ? mempool_alloc+0xb7/0x1a0
[ 3052.298406]  ? submit_bio+0x30/0x110
[ 3052.298407]  ? bvec_alloc+0x7c/0xd0
[ 3052.298409]  ? __submit_merged_bio+0x68/0x390
[ 3052.298410]  ? f2fs_submit_page_write+0x1bb/0x7f0
[ 3052.298410]  ? f2fs_do_write_meta_page+0x7f/0x160
[ 3052.298411]  ? __f2fs_write_meta_page+0x70/0x140
[ 3052.298412]  ? f2fs_sync_meta_pages+0x140/0x250
[ 3052.298413]  ? f2fs_write_checkpoint+0x5c5/0x17b0
[ 3052.298414]  ? f2fs_sync_fs+0x9c/0x110
[ 3052.298416]  ? sync_filesystem+0x66/0x80
[ 3052.298417]  ? generic_shutdown_super+0x1d/0x100
[ 3052.298418]  ? kill_block_super+0x1c/0x40
[ 3052.298418]  ? kill_f2fs_super+0x64/0xb0
[ 3052.298419]  ? deactivate_locked_super+0x2d/0xb0
[ 3052.298420]  ? cleanup_mnt+0x65/0xa0
[ 3052.298421]  ? task_work_run+0x7f/0xa0
[ 3052.298422]  ? exit_to_usermode_loop+0x9c/0xa0
[ 3052.298423]  ? do_syscall_64+0xc7/0xf0
[ 3052.298425]  ? entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 3052.298425] ---[ end trace d1bb538504008702 ]---
[ 3052.298481] print_req_error: I/O error, dev loop1, sector 12488
[ 3052.298486] print_req_error: I/O error, dev loop1, sector 12648
[ 3052.298488] print_req_error: I/O error, dev loop1, sector 12672
[ 3052.298488] print_req_error: I/O error, dev loop1, sector 33040
[ 3052.298489] print_req_error: I/O error, dev loop1, sector 143048
[ 3052.298491] print_req_error: I/O error, dev loop1, sector 152200
[ 3052.298492] print_req_error: I/O error, dev loop1, sector 8192
[ 3091.204798] F2FS-fs (loop1): SIT is corrupted node# 2481418 vs 2517451
[ 3091.204801] F2FS-fs (loop1): Failed to initialize F2FS segment manager

The fsck output was too huge to to attach here, please visit here instead:
http://arter97.com/f2fs.log.xz

Since I stored this f2fs image inside a btrfs snapshot, I always have
access to the original(mountable) image. Feel free to suggest risky
patches, etc.

Thanks.


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

Re: [f2fs-dev] [PATCH 2/2] mkfs.f2fs: make the default extensions list much more sensical

[Ju Hyung Park]

Hi Chao,

On Wed, Apr 17, 2019 at 6:41 PM Chao Yu  wrote:
> Actually, kernel will compare each character of extension in list with file's
> extension, rather than prefix, could you confirm that? :)

Just wrote a sample C program with namei.c's is_extension_exist() to
confirm this indeed works as intended.

Output:
Checking against "jp"
jpg: false
abc.jpg: true
abc.jpeg: true
abc.jpg.tmp: true
abc.jpeg.tmp: true
abc.jgp: false

Source:

#include 
#include 
#include 

static int is_extension_exist(const unsigned char *s, const char *sub)
{
size_t slen = strlen(s);
size_t sublen = strlen(sub);
int i;

/*
 * filename format of multimedia file should be defined as:
 * "filename + '.' + extension + (optional: '.' + temp extension)".
 */
if (slen < sublen + 2)
return 0;

for (i = 1; i < slen - sublen; i++) {
if (s[i] != '.')
continue;
if (!strncasecmp(s + i + 1, sub, sublen))
return 1;
}

return 0;
}

static const char *list[] = {
"jpg",
"abc.jpg",
"abc.jpeg",
"abc.jpg.tmp",
"abc.jpeg.tmp",
"abc.jgp",
NULL
};

#define CHECK "jp"

int main() {
printf("Checking against \"%s\"\n", CHECK);

for (int i = 0; list[i]; i++) {
if (is_extension_exist(list[i], CHECK))
printf("%s: true\n", list[i]);
else
printf("%s: false\n", list[i]);
}
}


___
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-tools: fix an improper #ifdef wrap for Android

[Ju Hyung Park]

Hi Jaegeuk,

On Wed, Apr 17, 2019 at 5:35 AM Jaegeuk Kim  wrote:
> > - if (c.kd < 0) {
> > - MSG(0, "\tInfo: No support kernel version!\n");
> > - c.kd = -2;
>
> If there are multiple devices, we don't need to get the version redundantly.

Oh, didn't think of that case.

Can we just read the kernel version and store to memory from
initialization of the program?
I see that the code for getting the kernel version is already
duplicated in 2 different files: fsck/mount.c and mkfs/f2fs_format.c

/* get kernel version */
if (c.kd >= 0) {
dev_read_version(c.version, 0, VERSION_LEN);
get_kernel_version(c.version);
} else {
get_kernel_uname_version(c.version);
}

Thanks.


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

Re: [f2fs-dev] [PATCH 2/2] mkfs.f2fs: make the default extensions list much more sensical

[Ju Hyung Park]

Hi Jaegeuk,

On Wed, Apr 17, 2019 at 5:39 AM Jaegeuk Kim  wrote:
> Thank you for heads up. Taking a look at two patches, I think it'd be fine to
> merge them into single patch. Let me know, if you have any concerns.

I'd prefer not merging it as a reorder was done prior to this patch.

If someone was looking at the extensions list closely, they'll have a
hard time understanding which extensions really changed.

But I believe this is a nitpick at this point. I see you already
merged the patches to dev-test, feel free to leave it as-is.

Thanks.


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

Re: [f2fs-dev] [PATCH 2/2] mkfs.f2fs: make the default extensions list much more sensical

[Ju Hyung Park]

Hi Jaegeuk, Chao.

This is a revival from a patchset I sent about a year ago.

The original patchset's list was about 50 entries and we talked about
leaving too small room for the users to customize.
I made much more conservative choices here to made the new one to be
36(from 34), while still being very practical and relevant on both
regular Linux distros and Android.
I hope the explanation written in the commit message is enough to persuade you.

Thanks.


On Tue, Apr 16, 2019 at 3:44 PM Park Ju Hyung  wrote:
>
> Following extensions are removed:
>  - divx: deprecated video format and it's usually wrapped with avi
>  - asf: deprecated streaming format
>  - asx: redirecting file to asf(small)
>  - wmx: redirecting file to wma/wmv(small)
>  - rm: deprecated media container
>  - video: unused
>  - wv: unpopular audio format from 1998
>
> The extensions list is limited to 64 and those don't deserve to be
> on this space-precious list.
>
> Common prefixes are introduced and are checked with
> https://en.wikipedia.org/wiki/List_of_filename_extensions
> to avoid treating possible hot files as cold:
>  - mp: covers mp3, mp4, mpeg, mpg
>  - wm: covers wma, wmb, wmv
>  - og: covers oga, ogg, ogm, ogv
>  - jp: covers jpg, jpeg, jp2
>
> Following extensions are added:
>  - webm: extremely popular free media container format from Google
>  VP8/VP9/AV1 and Vorbis/Opus is often wrapped with this container
>  - wav: uncompressed audio format, commonly used with voice recorders
>  - svg: vector image format commonly used in web
>  - webp: free lossy image format commonly used in web
>  - jar: Java archive file
>  - deb: Debian software package
>  - iso: disk image file
>  - gz: gzip compressed file, unable to randomly update
>  - xz: xz compressed file, unable to randomly update
>  - zst: zstd compressed file, unable to randomly update
>  - pdf: PDF document
>  - pyc: Python bytecode automatically generated when
> executing python to run .py files
>  - ttc, ttf: font files
>  - cnt: image alias files commonly used in Android apps
>  - exo: EXO player's cache files, commonly used in Android's YouTube app
>  - odex, vdex: Android RunTime files found in /data/app/*/oat
>
> Total entries on the list changed from 34 to 36.
>
> Signed-off-by: Park Ju Hyung 
> ---
>  mkfs/f2fs_format.c | 50 ++
>  1 file changed, 28 insertions(+), 22 deletions(-)
>
> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
> index 0ae0df3..4560611 100644
> --- a/mkfs/f2fs_format.c
> +++ b/mkfs/f2fs_format.c
> @@ -38,48 +38,54 @@ struct f2fs_checkpoint *cp;
>  static unsigned int quotatype_bits = 0;
>
>  const char *media_ext_lists[] = {
> +   /* common prefix */
> +   "mp", // Covers mp3, mp4, mpeg, mpg
> +   "wm", // Covers wma, wmb, wmv
> +   "og", // Covers oga, ogg, ogm, ogv
> +   "jp", // Covers jpg, jpeg, jp2
> +
> /* video */
> "avi",
> -   "divx",
> "m4v",
> "m4p",
> -   "mp4",
> -   "wmv",
> -   "mpeg",
> "mkv",
> "mov",
> -   "asx",
> -   "asf",
> -   "wmx",
> -   "svi",
> -   "wvx",
> -   "wm",
> -   "mpg",
> -   "mpe",
> -   "rm",
> -   "video",
> +   "webm",
>
> /* audio */
> +   "wav",
> "m4a",
> -   "mp3",
> "3gp",
> -   "wma",
> -   "wv",
> -   "ogg",
> "opus",
> "flac",
>
> /* image */
> -   "jpeg",
> -   "jpg",
> "gif",
> "png",
> -
> -   /* other */
> +   "svg",
> +   "webp",
> +
> +   /* archives */
> +   "jar",
> +   "deb",
> +   "iso",
> +   "gz",
> +   "xz",
> +   "zst",
> +
> +   /* others */
> +   "pdf",
> +   "pyc", // Python bytecode
> +   "ttc",
> +   "ttf",
> "exe",
>
> /* android */
> "apk",
> +   "cnt", // Image alias
> +   "exo", // YouTube
> +   "odex", // Android RunTime
> +   "vdex", // Android RunTime
> "so",
>
> NULL
> --
> 2.21.0
>


___
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: avoid retrying quota flushes while the device is congested

[Ju Hyung Park]

Hi Chao,

On Tue, Apr 16, 2019 at 11:51 AM Chao Yu  wrote:
> /* maximum retry quota flush count */
> #define DEFAULT_RETRY_QUOTA_FLUSH_COUNT 8
>
> I added above flush count number to limit cycle index, so that we won't be 
> stuck
> for long time once we are under heavy racy in between checkpoint() and quota
> updating.

I saw this. I actually first attempted increasing this to 16. It didn't fix it.

> Once we skip flushing quota, in current checkpoint, quota sysfile may be
> corrupted, but if there is no sudden power-cut, I expect we can have chance to
> flush all data of quota file in next checkpoint, then the quota file is
> integrated again.
>
> So could you track the root cause why we set the CP_QUOTA_NEED_FSCK_FLAG flag 
> in
> checkpoint() from umount?

Ok, I'll check this out. Please allow me a day or two for this.

> Do we skip flush quota due to flush count exceeds the
> DEFAULT_RETRY_QUOTA_FLUSH_COUNT?

I'll set this to an unrealistic number(e.g. 500) and add a log to check too.

> Could you check your source code? did you apply dfede78aa918 ("fsck.f2fs: 
> detect
> and recover corrupted quota file")? This patch can enable fsck to repair quota
> file corruption once kernel set CP_QUOTA_NEED_FSCK_FLAG flag.

Oh right. Yeah my fsck.f2fs was not up-to-date(pie-release branch).
False alarm.

Thanks.


___
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: avoid retrying quota flushes while the device is congested

[Ju Hyung Park]

Hi Chao,

On Tue, Apr 16, 2019 at 10:58 AM Chao Yu  wrote:
> I knew this works in scenario cell phone suffers sudden power-cut frequently,
> actually, in a normal case, if there is race in between checkpoint() and quota
> updater, checkpoint() may suffer longer delay than before.

This was actually not a sudden power-cut scenario.
After passing the setup wizard
(the quota flush timeout might have happened during initial dex2oat process)
and simply rebooting, I always see "quota file may be corrupted, skip
loading it".

I personally investigated this quota corruption issue as I frequently
saw it on my
daily driver phone even when I never had sudden power-cuts.

> So I suggest we keep the logic as it is, since sudden power-cut is not a 
> common
> case,

As I said, the issue is caused from a totally normal scenario.
A solution must be implemented imo.
Feel free to suggest a better idea/patch for me to test with.

> even in such case, quota sysfile is corrupted, we have another chance to
> repair it with fsck-tools, right?

This is an another potential issue. Android does fsck.f2fs -a on boot
and it does not
restore quota. Doing a run with -f fixes it, but not with -a.

This ultimately led the storage settings under the settings app to misbehave.

Thanks.


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

Re: [f2fs-dev] [PATCH 13/13] f2fs: don't recovery orphan inode on readonly device

[Ju Hyung Park]

Thanks for the explanation.

And yes, this patch fixed it, the kernel log is now clean.

Thanks!

[   22.506553] F2FS-fs (loop0): write access unavailable, skipping
orphan cleanup
[   22.506555] F2FS-fs (loop0): recover fsync data on readonly fs
[   22.506556] F2FS-fs (loop0): quota file may be corrupted, skip loading it
[   22.507015] F2FS-fs (loop0): Mounted with checkpoint version = 26e7ba3e

On Mon, Apr 15, 2019 at 5:57 PM Chao Yu  wrote:
>
> On 2019/4/15 16:10, Ju Hyung Park wrote:
> > Hi,
> >
> > Thanks for the fix. I'll try this sooner than later.
> >
> > One minor request though, can you change
> > "JuHyung Park "
> > to
> > "Park Ju Hyung "?
> >
> > That's my preference and I'd like to avoid any inconsistencies.
>
> Sure, will update it in next version. :)
>
> >
> > One additional question from reviewing the code surrounding it:
> > does it really makes sense to cleanup orphan inodes even when the "ro"
> > mount option is passed?
> > It's an explicit request from the user not to write to the block 
> > device/image.
>
> Now, f2fs follows the rule that ext4 kept, you can check codes in
> ext4_orphan_cleanup()
>
> if (bdev_read_only(sb->s_bdev)) {
> ext4_msg(sb, KERN_ERR, "write access "
> "unavailable, skipping orphan cleanup");
> return;
> }
> ...
> if (s_flags & SB_RDONLY) {
> ext4_msg(sb, KERN_INFO, "orphan cleanup on readonly fs");
> sb->s_flags &= ~SB_RDONLY;
> }
>
> There are two points in above codes:
> - if block device is readonly, filesystem should not execute any recovery flow
> which can trigger write IO.
> - if filesystem was mounted as readonly one, and recovery is needed, it will
> ignore readonly flag and update data in device for journal recovery during 
> mount.
>
> So IMO, readonly mountoption sematics is only try to restrict data/meta update
> behavior that is triggered by user from mountpoint, but filesystem still can 
> do
> any updates on a writable device if it needs, mostly like recovery flow.
>
> Anyway, if you want to limit any updates on block device, making it readonly
> will be a good choice. :)
>
> Thanks,
>
> >
> > Thanks.
> >
> > On Mon, Apr 15, 2019 at 4:31 PM Chao Yu  wrote:
> >>
> >> As JuHyung Park reported in mailing list:
> >>
> >> https://sourceforge.net/p/linux-f2fs/mailman/message/36639787/
> >>
> >> generic_make_request: Trying to write to read-only block-device loop0 
> >> (partno 0)
> >> WARNING: CPU: 0 PID: 23437 at block/blk-core.c:2174 
> >> generic_make_request_checks+0x594/0x630
> >>
> >>  generic_make_request+0x46/0x3d0
> >>  submit_bio+0x30/0x110
> >>  __submit_merged_bio+0x68/0x390
> >>  f2fs_submit_page_write+0x1bb/0x7f0
> >>  f2fs_do_write_meta_page+0x7f/0x160
> >>  __f2fs_write_meta_page+0x70/0x140
> >>  f2fs_sync_meta_pages+0x140/0x250
> >>  f2fs_write_checkpoint+0x5c5/0x17b0
> >>  f2fs_sync_fs+0x9c/0x110
> >>  sync_filesystem+0x66/0x80
> >>  f2fs_recover_fsync_data+0x790/0xa30
> >>  f2fs_fill_super+0xe4e/0x1980
> >>  mount_bdev+0x518/0x610
> >>  mount_fs+0x34/0x13f
> >>  vfs_kern_mount.part.11+0x4f/0x120
> >>  do_mount+0x2d1/0xe40
> >>  __x64_sys_mount+0xbf/0xe0
> >>  do_syscall_64+0x4a/0xf0
> >>  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> >>
> >> print_req_error: I/O error, dev loop0, sector 4096
> >>
> >> If block device is readonly, we should never trigger write IO from
> >> filesystem layer, but previously, orphan recovery didn't consider
> >> such condition, result in triggering above warning, fix it.
> >>
> >> Reported-by: JuHyung Park 
> >> Signed-off-by: Chao Yu 
> >> ---
> >>  fs/f2fs/checkpoint.c | 6 ++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> >> index a7ad1b1e5750..90e1bab86269 100644
> >> --- a/fs/f2fs/checkpoint.c
> >> +++ b/fs/f2fs/checkpoint.c
> >> @@ -674,6 +674,12 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info 
> >> *sbi)
> >> if (!is_set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG))
> >> return 0;
> >>
> >> +   if (bdev_read_only(sbi->sb->s_bdev)) {
> >> +   f2fs_msg(sbi->sb, KERN_INFO, "write access "
> >> +   "unavailable, skipping orphan cleanup");
> >> +   return 0;
> >> +   }
> >> +
> >> if (s_flags & SB_RDONLY) {
> >> f2fs_msg(sbi->sb, KERN_INFO, "orphan cleanup on readonly 
> >> fs");
> >> sbi->sb->s_flags &= ~SB_RDONLY;
> >> --
> >> 2.18.0.rc1
> >>
> >>
> >>
> >> ___
> >> 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 13/13] f2fs: don't recovery orphan inode on readonly device

[Ju Hyung Park]

Hi,

Thanks for the fix. I'll try this sooner than later.

One minor request though, can you change
"JuHyung Park "
to
"Park Ju Hyung "?

That's my preference and I'd like to avoid any inconsistencies.

One additional question from reviewing the code surrounding it:
does it really makes sense to cleanup orphan inodes even when the "ro"
mount option is passed?
It's an explicit request from the user not to write to the block device/image.

Thanks.

On Mon, Apr 15, 2019 at 4:31 PM Chao Yu  wrote:
>
> As JuHyung Park reported in mailing list:
>
> https://sourceforge.net/p/linux-f2fs/mailman/message/36639787/
>
> generic_make_request: Trying to write to read-only block-device loop0 (partno 
> 0)
> WARNING: CPU: 0 PID: 23437 at block/blk-core.c:2174 
> generic_make_request_checks+0x594/0x630
>
>  generic_make_request+0x46/0x3d0
>  submit_bio+0x30/0x110
>  __submit_merged_bio+0x68/0x390
>  f2fs_submit_page_write+0x1bb/0x7f0
>  f2fs_do_write_meta_page+0x7f/0x160
>  __f2fs_write_meta_page+0x70/0x140
>  f2fs_sync_meta_pages+0x140/0x250
>  f2fs_write_checkpoint+0x5c5/0x17b0
>  f2fs_sync_fs+0x9c/0x110
>  sync_filesystem+0x66/0x80
>  f2fs_recover_fsync_data+0x790/0xa30
>  f2fs_fill_super+0xe4e/0x1980
>  mount_bdev+0x518/0x610
>  mount_fs+0x34/0x13f
>  vfs_kern_mount.part.11+0x4f/0x120
>  do_mount+0x2d1/0xe40
>  __x64_sys_mount+0xbf/0xe0
>  do_syscall_64+0x4a/0xf0
>  entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> print_req_error: I/O error, dev loop0, sector 4096
>
> If block device is readonly, we should never trigger write IO from
> filesystem layer, but previously, orphan recovery didn't consider
> such condition, result in triggering above warning, fix it.
>
> Reported-by: JuHyung Park 
> Signed-off-by: Chao Yu 
> ---
>  fs/f2fs/checkpoint.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index a7ad1b1e5750..90e1bab86269 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -674,6 +674,12 @@ int f2fs_recover_orphan_inodes(struct f2fs_sb_info *sbi)
> if (!is_set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG))
> return 0;
>
> +   if (bdev_read_only(sbi->sb->s_bdev)) {
> +   f2fs_msg(sbi->sb, KERN_INFO, "write access "
> +   "unavailable, skipping orphan cleanup");
> +   return 0;
> +   }
> +
> if (s_flags & SB_RDONLY) {
> f2fs_msg(sbi->sb, KERN_INFO, "orphan cleanup on readonly fs");
> sbi->sb->s_flags &= ~SB_RDONLY;
> --
> 2.18.0.rc1
>
>
>
> ___
> 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

[f2fs-dev] f2fs: trying to write to read-only block-device

[Ju Hyung Park]

Hi,

While I was playing around a f2fs image dumped from an Android device,
I observed that f2fs were trying to write to the image when I mounted
it via ro.

The host is Linux 4.19 with 5.1-rc1-4.19 merged.

f2fs might be trying to write to the device when the quota is
corrupted, regardless of ro being passed to the mount option or not.

I believe this is an easy issue to fix.

Thanks.

[17431.491120] F2FS-fs (loop0): orphan cleanup on readonly fs
[17431.491121] F2FS-fs (loop0): quota file may be corrupted, skip loading it
[17431.491710] F2FS-fs (loop0): recover fsync data on readonly fs
[17431.491711] F2FS-fs (loop0): quota file may be corrupted, skip loading it
[17431.492021] [ cut here ]
[17431.492022] generic_make_request: Trying to write to read-only
block-device loop0 (partno 0)
[17431.492035] WARNING: CPU: 0 PID: 23437 at block/blk-core.c:2174
generic_make_request_checks+0x594/0x630
[17431.492081] CPU: 0 PID: 23437 Comm: mount Tainted: G U  W
  4.19.33-zen+ #1
[17431.492082] Hardware name: LG Electronics
14ZD980-GX50K/14Z980, BIOS K2ZC0250 X64 03/08/2018
[17431.492084] RIP: 0010:generic_make_request_checks+0x594/0x630
[17431.492085] Code: 5c 03 00 00 48 8d 74 24 08 48 89 df c6 05 71 ec
36 01 01 e8 6e 91 01 00 48 89 c6 44 89 ea 48 c7 c7 10 61 59 82 e8 21
bb a7 ff <0f> 0b 48 8b 7b 08 e9 ee fa ff ff 65 8b 05 8a 71 91 7e 89 c0
48 0f
[17431.492086] RSP: 0018:88825a94b700 EFLAGS: 00010286
[17431.492087] RAX: 0050 RBX: 88842d3e0b00 RCX: 0006
[17431.492088] RDX: 0007 RSI: 0082 RDI: 888433416340
[17431.492088] RBP: 8884303d1d10 R08: 0004 R09: 0542
[17431.492089] R10: 0001 R11: 027f R12: 1000
[17431.492090] R13:  R14: 88840a179000 R15: 88825a94b870
[17431.492091] FS:  7fd885c988c0() GS:88843340()
knlGS:
[17431.492091] CS:  0010 DS:  ES:  CR0: 80050033
[17431.492092] CR2: 7fd88636eee0 CR3: 00017a4c3006 CR4: 003606f0
[17431.492093] Call Trace:
[17431.492096]  ? generic_make_request+0x46/0x3d0
[17431.492098]  ? wait_woken+0x80/0x80
[17431.492099]  ? mempool_alloc+0xb7/0x1a0
[17431.492101]  ? submit_bio+0x30/0x110
[17431.492102]  ? bvec_alloc+0x7c/0xd0
[17431.492104]  ? __submit_merged_bio+0x68/0x390
[17431.492106]  ? f2fs_submit_page_write+0x1bb/0x7f0
[17431.492108]  ? f2fs_do_write_meta_page+0x7f/0x160
[17431.492109]  ? __f2fs_write_meta_page+0x70/0x140
[17431.492110]  ? f2fs_sync_meta_pages+0x140/0x250
[17431.492112]  ? f2fs_write_checkpoint+0x5c5/0x17b0
[17431.492115]  ? __schedule+0x1b9/0x6b0
[17431.492116]  ? f2fs_sync_fs+0x9c/0x110
[17431.492119]  ? sync_filesystem+0x66/0x80
[17431.492120]  ? f2fs_recover_fsync_data+0x790/0xa30
[17431.492122]  ? f2fs_fill_super+0xe4e/0x1980
[17431.492123]  ? pointer+0x149/0x320
[17431.492124]  ? pcpu_alloc_area+0xdd/0x120
[17431.492126]  ? snprintf+0x39/0x40
[17431.492127]  ? mount_bdev+0x518/0x610
[17431.492128]  ? mount_bdev+0x518/0x610
[17431.492129]  ? f2fs_commit_super+0x160/0x160
[17431.492130]  ? mount_fs+0x34/0x13f
[17431.492132]  ? vfs_kern_mount.part.11+0x4f/0x120
[17431.492133]  ? do_mount+0x2d1/0xe40
[17431.492135]  ? memdup_user+0x39/0x60
[17431.492137]  ? __x64_sys_mount+0xbf/0xe0
[17431.492139]  ? do_syscall_64+0x4a/0xf0
[17431.492140]  ? entry_SYSCALL_64_after_hwframe+0x44/0xa9
[17431.492141] ---[ end trace 74417a97d9cf58c5 ]---
[17431.492237] print_req_error: I/O error, dev loop0, sector 16872
[17431.492243] print_req_error: I/O error, dev loop0, sector 21032
[17431.492245] print_req_error: I/O error, dev loop0, sector 47728
[17431.492247] print_req_error: I/O error, dev loop0, sector 4096
[17431.492258] F2FS-fs (loop0): Mounted with checkpoint version = 26e7ba3f


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

[f2fs-dev] f2fs: prevent mount with "extended node bitmap" on older kernels

[Ju Hyung Park]

Hi,

I was testing around the "extended node bitmap" feature within f2fs
and found an issue with fsck.f2fs resulting in false-positives
and causing further corruptions when data is written by older kernels.

I can reliably reproduce this issue with a simple image creation
and fsck with Linux 4.15.

# fallocate -l 2G f2fs.img
# mkfs.f2fs -i f2fs.img
# mount f2fs.img /mnt
# rsync -a /var/ /mnt/
# umount /mnt
# fsck.f2fs -f --dry-run f2fs.img

This results in the following behavior with the latest f2fs-tools
v1.12.0 and Linux 4.15:
https://pastebin.com/MTKpa2AC
(A side note, 2nd run of fsck still shows some errors being fixed.)

I cannot reproduce this issue with Linux 4.19.
Is this new feature supposed to be only used with recent kernels?

If so, mount should be prevented on older kernels, at least writes.
(I'm noticing reads are fine with older kernels.)

Thanks,


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

Re: [f2fs-dev] Patch "Revert "f2fs: fix to clear PG_checked flag in set_page_dirty()"" has been added to the 4.18-stable tree

[Ju Hyung Park]

Hi,

This bug was present only in 4.19.
Is it still applicable to older branches?

Thanks.
On Sun, Nov 11, 2018 at 6:13 PM Ju Hyung Park  wrote:
>
> Hi,
>
> This bug was present only in 4.19.
> Is it still applicable to older branches?
>
> Thanks.
>
> On Sun, Nov 11, 2018, 12:26 PM >
>>
>> This is a note to let you know that I've just added the patch titled
>>
>> Revert "f2fs: fix to clear PG_checked flag in set_page_dirty()"
>>
>> to the 4.18-stable tree which can be found at:
>> 
>> http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>>
>> The filename of the patch is:
>>  revert-f2fs-fix-to-clear-pg_checked-flag-in-set_page_dirty.patch
>> and it can be found in the queue-4.18 subdirectory.
>>
>> If you, or anyone else, feels it should not be added to the stable tree,
>> please let  know about it.
>>
>>
>> From 164a63fa6b384e30ceb96ed80bc7dc3379bc0960 Mon Sep 17 00:00:00 2001
>> From: Jaegeuk Kim 
>> Date: Tue, 16 Oct 2018 19:30:13 -0700
>> Subject: Revert "f2fs: fix to clear PG_checked flag in set_page_dirty()"
>>
>> From: Jaegeuk Kim 
>>
>> commit 164a63fa6b384e30ceb96ed80bc7dc3379bc0960 upstream.
>>
>> This reverts commit 66110abc4c931f879d70e83e1281f891699364bf.
>>
>> If we clear the cold data flag out of the writeback flow, we can miscount
>> -1 by end_io, which incurs a deadlock caused by all I/Os being blocked during
>> heavy GC.
>>
>> Balancing F2FS Async:
>>  - IO (CP:1, Data:   -1, Flush: (   001), Discard: (   ...
>>
>> GC thread:  IRQ
>> - move_data_page()
>>  - set_page_dirty()
>>   - clear_cold_data()
>> - f2fs_write_end_io()
>>  - type = WB_DATA_TYPE(page);
>>here, we get wrong type
>>  - dec_page_count(sbi, type);
>>  - f2fs_wait_on_page_writeback()
>>
>> Cc: 
>> Reported-and-Tested-by: Park Ju Hyung 
>> Reviewed-by: Chao Yu 
>> Signed-off-by: Jaegeuk Kim 
>> Signed-off-by: Greg Kroah-Hartman 
>>
>> ---
>>  fs/f2fs/data.c |4 
>>  1 file changed, 4 deletions(-)
>>
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -2501,10 +2501,6 @@ static int f2fs_set_data_page_dirty(stru
>> if (!PageUptodate(page))
>> SetPageUptodate(page);
>>
>> -   /* don't remain PG_checked flag which was set during GC */
>> -   if (is_cold_data(page))
>> -   clear_cold_data(page);
>> -
>> if (f2fs_is_atomic_file(inode) && 
>> !f2fs_is_commit_atomic_write(inode)) {
>> if (!IS_ATOMIC_WRITTEN_PAGE(page)) {
>> f2fs_register_inmem_page(inode, page);
>>
>>
>> Patches currently in stable-queue which might be from jaeg...@kernel.org are
>>
>> queue-4.18/f2fs-fix-to-recover-inode-s-i_flags-during-por.patch
>> queue-4.18/f2fs-fix-to-recover-inode-s-crtime-during-por.patch
>> queue-4.18/f2fs-fix-to-account-io-correctly-for-cgroup-writeback.patch
>> queue-4.18/f2fs-fix-to-recover-cold-bit-of-inode-block-during-por.patch
>> queue-4.18/revert-f2fs-fix-to-clear-pg_checked-flag-in-set_page_dirty.patch
>> queue-4.18/f2fs-clear-pageerror-on-the-read-path.patch
>> queue-4.18/f2fs-report-error-if-quota-off-error-during-umount.patch
>> queue-4.18/f2fs-fix-to-account-io-correctly.patch
>> queue-4.18/f2fs-avoid-sleeping-under-spin_lock.patch


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

Re: [f2fs-dev] [PATCH v2] f2fs: clear cold data flag if IO is not counted

[Ju Hyung Park]

This does seem to fix the problem I've been having.

I was encountering all I/Os to be blocked on my Android device
almost on a daily basis, but after this patch I'm rock solid for almost a week.

I'd appreciate it if you could add the following tags:
Reported-by: Park Ju Hyung 
Tested-by: Park Ju Hyung 

Also, a little additional note would be helpful
for those who are unfamiliar with f2fs:
This fixes a deadlock causing all I/Os to be blocked during heavy GC.

Thanks,
On Wed, Oct 17, 2018 at 11:34 AM Jaegeuk Kim  wrote:
>
> This reverts commit 66110abc4c931f879d70e83e1281f891699364bf.
>
> If we clear the cold data flag out of the writeback flow, we can miscount
> -1 by end_io.
>
> Balancing F2FS Async:
>  - IO (CP:1, Data:   -1, Flush: (   001), Discard: (   ...
>
> GC thread:  IRQ
> - move_data_page()
>  - set_page_dirty()
>   - clear_cold_data()
> - f2fs_write_end_io()
>  - type = WB_DATA_TYPE(page);
>here, we get wrong type
>  - dec_page_count(sbi, type);
>  - f2fs_wait_on_page_writeback()
>
> Cc: 
> Signed-off-by: Jaegeuk Kim 
> ---
>  fs/f2fs/data.c | 4 
>  1 file changed, 4 deletions(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 3f272c18fb61..0d0b4dd55b04 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -2650,10 +2650,6 @@ static int f2fs_set_data_page_dirty(struct page *page)
> if (!PageUptodate(page))
> SetPageUptodate(page);
>
> -   /* don't remain PG_checked flag which was set during GC */
> -   if (is_cold_data(page))
> -   clear_cold_data(page);
> -
> if (f2fs_is_atomic_file(inode) && 
> !f2fs_is_commit_atomic_write(inode)) {
> if (!IS_ATOMIC_WRITTEN_PAGE(page)) {
> f2fs_register_inmem_page(inode, page);
> --
> 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

Re: [f2fs-dev] [PATCH 1/2] f2fs: set 4KB discard granularity by default

[Ju Hyung Park]

Hi,

I was wondering what would be the negatives on reducing the discard granularity
other than making discard more aggressive(hence higher overhead and latency?).

Thanks.

On Tue, Aug 14, 2018 at 4:08 PM Chao Yu  wrote:
>
> On 2018/8/14 12:13, Jaegeuk Kim wrote:
> > On 08/10, Chao Yu wrote:
> >> Small granularity (size < 64K) fragmentation will cause f2fs suspending
> >> all pending discards, result in performance regression, so let's set
> >> 4KB discard granularity by default.
> >>
> >> So that without fstrim, we also have the ability to avoid any performance
> >> regression caused by non-alignment mapping between fs and flash device.
> >
> > This is why we added a sysfs entry. Why do we need to change the default
> > value every time?
>
> Of course, I didn't forget it. But, mostly, our user didn't know very well 
> about
> our filesystem including each configuration's meaning, mechanism, or usage
> scenario, most of the time, they will choose to test f2fs with all default
> option, and then make the conclusion.
>
> Currently, with default 64k granularity, if we simulate fragmentation scenario
> of filesystem, like by a)writing 4k file and b)deleting even indexing file, 
> then
> all 4k discards won't be issued, result in exhaustion of free space of flash
> storage, and performance degradation.
>
> So I think we'd better to consider and set default value of configuration more
> elaborately to avoid obvious weakness.
>
> Thoughts?
>
> Thanks,
>
> >
> >>
> >> Signed-off-by: Chao Yu 
> >> ---
> >>  fs/f2fs/f2fs.h | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> >> index 58431b9bfd8f..273ffdaf4891 100644
> >> --- a/fs/f2fs/f2fs.h
> >> +++ b/fs/f2fs/f2fs.h
> >> @@ -248,7 +248,7 @@ struct discard_entry {
> >>  };
> >>
> >>  /* default discard granularity of inner discard thread, unit: block count 
> >> */
> >> -#define DEFAULT_DISCARD_GRANULARITY 16
> >> +#define DEFAULT_DISCARD_GRANULARITY 1
> >>
> >>  /* max discard pend list number */
> >>  #define MAX_PLIST_NUM   512
> >> --
> >> 2.18.0.rc1
> >
> > .
> >
>
>
> --
> 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

--
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 1/4] mkfs.f2fs: update extension lists

[Ju Hyung Park]

Hi Chao,

> As I said, now we'd better to add a option in mkfs to disable default list
> first. If you have time to work on this, I'm glad to review the patch.;)

Yeah, I'll add that to my TODOs list :)

> What I mean is we'd better to unify the list into one place

A..
Now I understand why you had issues with the idea of kernelspace extension list.

I've just checked the implementation of sysfs extension list update.
I thought it'd be just a separate list handled within kernelspace f2fs
volatile memory.

Looks like it's an update to the existing superblock list, then you
would be right,
it'll get much more complicated.

Now, the best idea I can come up with is:
 - Move the default list from mkfs.f2fs to kernelspace and remove the
list from mkfs.
 - Store user's added/removed extensions separately in the superblock.

This will break compatibility with older f2fs and f2fs-tools, so I
don't think this is a good idea.
Can you come up with a better idea?

In summary, we need:
 - Always up-to-date default list, not fixed at the time of mkfs
 - Maintaining compatibility with different f2fs and f2fs-tools versions
 - Respecting user's choice to add or remove extensions

Thanks.
(This is more complicated than I originally anticipated, xD)


On Fri, Mar 30, 2018 at 8:19 PM, Chao Yu <yuch...@huawei.com> wrote:
> Hi Park,
>
> On 2018/3/29 23:54, Ju Hyung Park wrote:
>> Hi Chao,
>>
>>> I think this is real hardcoded one
>>
>> Agreed, but I can't figure out a better way of doing this.
>> I still don't think fixing it at mkfs point isn't a good idea.
>>
>> Doing this entirely on the userspace also won't make much sense since
>> I would not trust the distros to ship "good extension lists".
>
> Hmm, if user can not trust these recommended extension list, we can introduce
> mount option to let user to choose to disable all these default extensions, 
> and
> do the configuration with -e to add their own extensions. Once they want to
> update the list, sysfs entry will be recommended to make change.
>
>>
>>> which will be hard to configure with if user
>>> don't want this list at all.
>>
>> The new list is very conservative.
>> Stuffs like .tar and .zip, which most users won't randomly write to,
>> were excluded intentionally just by the fact that
>> those are *capable* of random updates.
>> Even compression methods like .bz2 were excluded since those are
>> consisted of independent compressed blocks,
>> which would mean random updates might be possible.
>>
>> I don't see why anyone would want to disable any of those,
>> (even video editors won't be able to randomly write on mp4/mkv files)
>> but they are still free to do so with the sysfs interface.
>
> Of course, but I doubt that in some private system, people may define .mp4 as
> own extension of one kind of non-audio file, and do not need to add this type
> extension, I think our filesystem should consider to allow user's custom
> extension, instead of current hardcoded one.
>
> As I said, now we'd better to add a option in mkfs to disable default list
> first. If you have time to work on this, I'm glad to review the patch.;)
>
>>
>> The way I see it is that doing this in the kernel's default
>> should be fine with almost every single users.
>>
>>> we have to use mount option
>>
>> Adding a mount option to bypass and disable
>> any sort of kernelspace / mkfs list would be also nice.
>>
>>> We can add an new mkfs option to disable old common list,
>>
>> This would be also nice.
>> The current code only appends user's list on the existing list.
>>
>>> then user can
>>> enable/disable cold/hot file type as they prefer on mkfs/run-time.
>>
>> My point was that most users won't even know that the list has
>> changed from f2fs-tools.
>> Some users won't even aware of cold/hot concept in f2fs.
>> If we do this from the kernel, just using the latest version of f2fs
>> will bring their extension list up-to-date.
>>
>>> One question, how can you handle the conflict in between old list generated
>>> during mkfs and new common list in run-time kernel?
>>
>> Valid point, but I think the added overhead of comparing both lists,
>> possibly re-comparing the same extension, would be negligible.
>> Can strcmp'ing ~70 entries be a considerable amount of overhead?
>
> What I mean is we'd better to unify the list into one place, now, I think
> default place in super block is not bad. If we add one hardcoded list, we will
> hard to know which one is deleted by user.
>
> Before:
> thin: .mp4, .mp3...
>
> User disable .mp4 extension
> th

Re: [f2fs-dev] [PATCH 1/4] mkfs.f2fs: update extension lists

[Ju Hyung Park]

Hi Chao,

> I think this is real hardcoded one

Agreed, but I can't figure out a better way of doing this.
I still don't think fixing it at mkfs point isn't a good idea.

Doing this entirely on the userspace also won't make much sense since
I would not trust the distros to ship "good extension lists".

> which will be hard to configure with if user
> don't want this list at all.

The new list is very conservative.
Stuffs like .tar and .zip, which most users won't randomly write to,
were excluded intentionally just by the fact that
those are *capable* of random updates.
Even compression methods like .bz2 were excluded since those are
consisted of independent compressed blocks,
which would mean random updates might be possible.

I don't see why anyone would want to disable any of those,
(even video editors won't be able to randomly write on mp4/mkv files)
but they are still free to do so with the sysfs interface.

The way I see it is that doing this in the kernel's default
should be fine with almost every single users.

> we have to use mount option

Adding a mount option to bypass and disable
any sort of kernelspace / mkfs list would be also nice.

> We can add an new mkfs option to disable old common list,

This would be also nice.
The current code only appends user's list on the existing list.

> then user can
> enable/disable cold/hot file type as they prefer on mkfs/run-time.

My point was that most users won't even know that the list has
changed from f2fs-tools.
Some users won't even aware of cold/hot concept in f2fs.
If we do this from the kernel, just using the latest version of f2fs
will bring their extension list up-to-date.

> One question, how can you handle the conflict in between old list generated
> during mkfs and new common list in run-time kernel?

Valid point, but I think the added overhead of comparing both lists,
possibly re-comparing the same extension, would be negligible.
Can strcmp'ing ~70 entries be a considerable amount of overhead?

Thanks.

On Thu, Mar 29, 2018 at 1:04 PM, Chao Yu <yuch...@huawei.com> wrote:
> Hi Park,
>
> On 2018/3/29 10:18, Ju Hyung Park wrote:
>> Hi Chao.
>>
>>> Do you mean we need add a new option to enable common list?
>>
>> No. I meant to move the list to the kernelspace's fs/f2fs or
>> include/linux/f2fs_fs.h.
>
> I think this is real hardcoded one, which will be hard to configure with if 
> user
> don't want this list at all. How can you disable partial type in this common
> list? we have to use mount option or sysfs entry to do this at every mount 
> time...
>
>> This eliminates the concern of the list getting too big and exceeding
>> the 64 limit,
>> and we can modify the list upon shipping new versions of f2fs.
>>
>> As I complained multiple times before,
>> I simply don't think that this list should be hardcoded at the time of mkfs.
>>
>> While users can still dynamically add/remove some extensions,
>> they can still have an older version of the "common list",
>> which then they have to use the sysfs interface everytime
>> whenever we update this list on mkfs.f2fs.
>
> We can add an new mkfs option to disable old common list, then user can
> enable/disable cold/hot file type as they prefer on mkfs/run-time.
>
>>
>> I'm not proposing to remove the list altogether in mkfs.f2fs.
>> I'm proposing to leave the old list in f2fs-tools, and write the new
>> list to the kernelspace code.
>> This way, older version of f2fs will still remain properly working.
>
> One question, how can you handle the conflict in between old list generated
> during mkfs and new common list in run-time kernel?
>
> Thanks,
>
>>
>> I'd love to hear Jaegeuk's thoughts on this.
>>
>>> -o extlist=classic means we enable thin cold/hot file type list, mostly we
>>> recommended enable this in android system.
>>
>> I strongly disagree with this.
>> Even more so with the case of Android,
>> the users/OEMs won't typically add more entries.
>>
>> And the new list is very relevant on Android as well imo.
>>
>> Thanks.
>>
>> On Wed, Mar 28, 2018 at 11:40 AM, Chao Yu <yuch...@huawei.com> wrote:
>>> Hi Park,
>>>
>>> On 2018/3/24 14:55, Ju Hyung Park wrote:
>>>> Hi Chao,
>>>>
>>>> Sorry myself as well for the late reply :)
>>>> Got caught up with something.
>>>>
>>>>> Like .so?
>>>>
>>>> Yeah, that makes sense.
>>>> I've reran the command without the size limit.
>>>>
>>>> >From Ubuntu, I think we can add '*.pyc' files as it's compiled Python
>>>> bytecode format.
>>>>

Re: [f2fs-dev] [PATCH 1/4] mkfs.f2fs: update extension lists

[Ju Hyung Park]

Hi Chao.

> Do you mean we need add a new option to enable common list?

No. I meant to move the list to the kernelspace's fs/f2fs or
include/linux/f2fs_fs.h.
This eliminates the concern of the list getting too big and exceeding
the 64 limit,
and we can modify the list upon shipping new versions of f2fs.

As I complained multiple times before,
I simply don't think that this list should be hardcoded at the time of mkfs.

While users can still dynamically add/remove some extensions,
they can still have an older version of the "common list",
which then they have to use the sysfs interface everytime
whenever we update this list on mkfs.f2fs.

I'm not proposing to remove the list altogether in mkfs.f2fs.
I'm proposing to leave the old list in f2fs-tools, and write the new
list to the kernelspace code.
This way, older version of f2fs will still remain properly working.

I'd love to hear Jaegeuk's thoughts on this.

> -o extlist=classic means we enable thin cold/hot file type list, mostly we
> recommended enable this in android system.

I strongly disagree with this.
Even more so with the case of Android,
the users/OEMs won't typically add more entries.

And the new list is very relevant on Android as well imo.

Thanks.

On Wed, Mar 28, 2018 at 11:40 AM, Chao Yu <yuch...@huawei.com> wrote:
> Hi Park,
>
> On 2018/3/24 14:55, Ju Hyung Park wrote:
>> Hi Chao,
>>
>> Sorry myself as well for the late reply :)
>> Got caught up with something.
>>
>>> Like .so?
>>
>> Yeah, that makes sense.
>> I've reran the command without the size limit.
>>
>>>From Ubuntu, I think we can add '*.pyc' files as it's compiled Python
>> bytecode format.
>>>From Android, I saw some new '*.cnt' files which are just jpeg files.
>> Notably, Facebook, Skype, Tumblr and Twitter uses it.
>>
>> Everything else seemed not that much interesting.
>>
>>> I agree that we'd better support the superset list of common static file, 
>>> but
>>> also I hope there is flexible usage of common list, old list and self 
>>> defined
>>> list, so I think we'd better leave enough free space of cold list to let 
>>> user
>>> define private cold file type extension as they wish, meanwhile support an
>>> option to make user have a chance to choose the common list or old list.
>>
>> If I understood you correctly, you want to leave an option for the old list
>> so that users have more room to add many more extensions, correct?
>
> Yup.
>
>>
>> If so, how about just leaving the old list and move the new ones to
>> the kernel code?
>
> Do you mean we need add a new option to enable common list? e.g.
>
> -o extlist=classic means we enable thin cold/hot file type list, mostly we
> recommended enable this in android system.
> -o extlist=full means we enable full cold/hot file type list, it is 
> recommended
> in other system, like server or pc.
>
> Like this?
>
> Thanks,
>
>> As I said in the previous comment, I don't think it makes sense to
>> ship new lists only to at the time of mkfs.
>> We can ship new lists as we update f2fs kernel code.
>>
>> While the new version of f2fs allows users to dynamically add or
>> remove extensions,
>> it doesn't ship the new common lists.
>>
>> Thanks.
>>
>> On Wed, Mar 21, 2018 at 9:00 PM, Chao Yu <yuch...@huawei.com> wrote:
>>> Hi Park,
>>>
>>> Sorry for late replying.
>>>
>>> On 2018/3/19 11:53, Ju Hyung Park wrote:
>>>> Hi Chao,
>>>>
>>>>> Do you run this script in android environment to get the cold type?
>>>> Yes, both on Ubuntu and Android(on /data with root permission).
>>>>
>>>>> Actually, I doubt that '+1M' condition can't indicate that the file is 
>>>>> cold or
>>>>> not, and after run this script in my cell phone,
>>>> Would it make sense to set a file that's < 1M as cold?
>>>
>>> Like .so?
>>>
>>>> I didn't think so. Please let me know if I'm wrong.
>>>>
>>>>> I didn't see so many type as your patch adds.
>>>> Of course, most of those were added from vlc and p7zip.
>>>> There are tons more, but I added ones that are most common.
>>>> While I personally don't have that much many types myself as well,
>>>> I can easily see one having those extensions stored under f2fs.
>>>>
>>>> Previous list was not enough, imo.
>>>> (After running the command, I've added exo and ?dex files for Android.)
>>>>
>>>>> If that is a com

Re: [f2fs-dev] [PATCH 1/4] mkfs.f2fs: update extension lists

[Ju Hyung Park]

Hi Chao,

Sorry myself as well for the late reply :)
Got caught up with something.

> Like .so?

Yeah, that makes sense.
I've reran the command without the size limit.

>From Ubuntu, I think we can add '*.pyc' files as it's compiled Python
bytecode format.
>From Android, I saw some new '*.cnt' files which are just jpeg files.
Notably, Facebook, Skype, Tumblr and Twitter uses it.

Everything else seemed not that much interesting.

> I agree that we'd better support the superset list of common static file, but
> also I hope there is flexible usage of common list, old list and self defined
> list, so I think we'd better leave enough free space of cold list to let user
> define private cold file type extension as they wish, meanwhile support an
> option to make user have a chance to choose the common list or old list.

If I understood you correctly, you want to leave an option for the old list
so that users have more room to add many more extensions, correct?

If so, how about just leaving the old list and move the new ones to
the kernel code?
As I said in the previous comment, I don't think it makes sense to
ship new lists only to at the time of mkfs.
We can ship new lists as we update f2fs kernel code.

While the new version of f2fs allows users to dynamically add or
remove extensions,
it doesn't ship the new common lists.

Thanks.

On Wed, Mar 21, 2018 at 9:00 PM, Chao Yu <yuch...@huawei.com> wrote:
> Hi Park,
>
> Sorry for late replying.
>
> On 2018/3/19 11:53, Ju Hyung Park wrote:
>> Hi Chao,
>>
>>> Do you run this script in android environment to get the cold type?
>> Yes, both on Ubuntu and Android(on /data with root permission).
>>
>>> Actually, I doubt that '+1M' condition can't indicate that the file is cold 
>>> or
>>> not, and after run this script in my cell phone,
>> Would it make sense to set a file that's < 1M as cold?
>
> Like .so?
>
>> I didn't think so. Please let me know if I'm wrong.
>>
>>> I didn't see so many type as your patch adds.
>> Of course, most of those were added from vlc and p7zip.
>> There are tons more, but I added ones that are most common.
>> While I personally don't have that much many types myself as well,
>> I can easily see one having those extensions stored under f2fs.
>>
>> Previous list was not enough, imo.
>> (After running the command, I've added exo and ?dex files for Android.)
>>
>>> If that is a common cold file type list that user may not do random updates 
>>> in
>>> the file after its creation,
>> That's exactly what I intended.
>>
>>> I suggest that we can add one common list instead
>>> of changing old one controlled by mkfs option
>> The new list is superset of the old list.
>> A few extensions were removed as those are mostly deprecated formats
>> and to make room for much more important extensions to be added such as m4a.
>
> I agree that we'd better support the superset list of common static file, but
> also I hope there is flexible usage of common list, old list and self defined
> list, so I think we'd better leave enough free space of cold list to let user
> define private cold file type extension as they wish, meanwhile support an
> option to make user have a chance to choose the common list or old list.
>
> How do you think?
>
> Hi Jaegeuk, what's your opinion?
>
> Thanks,
>
>>
>> Thanks.
>>
>>
>> On Mon, Mar 19, 2018 at 12:42 PM, Chao Yu <yuch...@huawei.com> wrote:
>>> Hi Park,
>>>
>>> On 2018/3/17 23:02, Park Ju Hyung wrote:
>>>> Those formats are large in size and rarely updated.
>>>>
>>>> Formats such as tar and zip were intentionally excluded as
>>>> those are capable of random updates.
>>>>
>>>> (Added from vlc, p7zip and running
>>>> 'find . -type f -size +1M |
>>>> while read FILE; do echo ${FILE##*.}; done |
>>>> sort | uniq -c | sort -nr'
>>>> manually)
>>>
>>> Do you run this script in android environment to get the cold type?
>>>
>>> Actually, I doubt that '+1M' condition can't indicate that the file is cold 
>>> or
>>> not, and after run this script in my cell phone, I didn't see so many type 
>>> as
>>> your patch adds.
>>>
>>> If that is a common cold file type list that user may not do random updates 
>>> in
>>> the file after its creation, I suggest that we can add one common list 
>>> instead
>>> of changing old one controlled by mkfs option, anyway, to use which one, the
>>> option can be decided b

Re: [f2fs-dev] [PATCH 1/4] mkfs.f2fs: update extension lists

[Ju Hyung Park]

Hi Chao,

> Do you run this script in android environment to get the cold type?
Yes, both on Ubuntu and Android(on /data with root permission).

> Actually, I doubt that '+1M' condition can't indicate that the file is cold or
> not, and after run this script in my cell phone,
Would it make sense to set a file that's < 1M as cold?
I didn't think so. Please let me know if I'm wrong.

> I didn't see so many type as your patch adds.
Of course, most of those were added from vlc and p7zip.
There are tons more, but I added ones that are most common.
While I personally don't have that much many types myself as well,
I can easily see one having those extensions stored under f2fs.

Previous list was not enough, imo.
(After running the command, I've added exo and ?dex files for Android.)

> If that is a common cold file type list that user may not do random updates in
> the file after its creation,
That's exactly what I intended.

> I suggest that we can add one common list instead
> of changing old one controlled by mkfs option
The new list is superset of the old list.
A few extensions were removed as those are mostly deprecated formats
and to make room for much more important extensions to be added such as m4a.

Thanks.


On Mon, Mar 19, 2018 at 12:42 PM, Chao Yu  wrote:
> Hi Park,
>
> On 2018/3/17 23:02, Park Ju Hyung wrote:
>> Those formats are large in size and rarely updated.
>>
>> Formats such as tar and zip were intentionally excluded as
>> those are capable of random updates.
>>
>> (Added from vlc, p7zip and running
>> 'find . -type f -size +1M |
>> while read FILE; do echo ${FILE##*.}; done |
>> sort | uniq -c | sort -nr'
>> manually)
>
> Do you run this script in android environment to get the cold type?
>
> Actually, I doubt that '+1M' condition can't indicate that the file is cold or
> not, and after run this script in my cell phone, I didn't see so many type as
> your patch adds.
>
> If that is a common cold file type list that user may not do random updates in
> the file after its creation, I suggest that we can add one common list instead
> of changing old one controlled by mkfs option, anyway, to use which one, the
> option can be decided by user.
>
> Thanks,
>
>>
>> Signed-off-by: Park Ju Hyung 
>> ---
>>  mkfs/f2fs_format.c | 86 
>> --
>>  1 file changed, 64 insertions(+), 22 deletions(-)
>>
>> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
>> index 65692bb..3c7ce16 100644
>> --- a/mkfs/f2fs_format.c
>> +++ b/mkfs/f2fs_format.c
>> @@ -37,34 +37,76 @@ struct f2fs_checkpoint *cp;
>>
>>  static unsigned int quotatype_bits = 0;
>>
>> -const char *media_ext_lists[] = {
>> - "jpg",
>> - "gif",
>> - "png",
>> +const char *cold_ext_lists[] = {
>> + /* video */
>>   "avi",
>>   "divx",
>> - "mp4",
>> - "mp3",
>> - "3gp",
>> - "wmv",
>> - "wma",
>> - "mpeg",
>> + "flv",
>> + "m2ts",
>> + "m4p",
>> + "m4v",
>>   "mkv",
>>   "mov",
>> - "asx",
>> - "asf",
>> - "wmx",
>> - "svi",
>> - "wvx",
>> - "wm",
>> + "mp4",
>> + "mpeg",
>> + "mpeg4",
>>   "mpg",
>> - "mpe",
>> - "rm",
>>   "ogg",
>> + "ogm",
>> + "ogv",
>> + "ts",
>> + "vob",
>> + "wmb",
>> + "wmv",
>> + "webm",
>> +
>> + /* audio */
>> + "aac",
>> + "ac3",
>> + "dts",
>> + "flac",
>> + "m4a",
>> + "mka",
>> + "mp3",
>> + "oga",
>> + "wav",
>> + "wma",
>> +
>> + /* image */
>> + "bmp",
>> + "gif",
>> + "jpg",
>>   "jpeg",
>> - "video",
>> - "apk",  /* for android system */
>> - "so",   /* for android system */
>> + "png",
>> + "svg",
>> + "webp",
>> +
>> + /* archive */
>> + "7z",
>> + "a",
>> + "deb",
>> + "gz",
>> + "gzip",
>> + "iso",
>> + "jar",
>> + "lzma",
>> + "rar",
>> + "tgz",
>> + "txz",
>> + "udf",
>> + "xz",
>> +
>> + /* other */
>> + "pdf",
>> + "ttf",
>> + "ttc",
>> +
>> + /* android */
>> + "apk",
>> + "exo", // YouTube
>> + "odex", // Android RunTime
>> + "vdex", // Android RunTime
>> + "so",
>>   NULL
>>  };
>>
>> @@ -74,7 +116,7 @@ const char *hot_ext_lists[] = {
>>  };
>>
>>  const char **default_ext_list[] = {
>> - media_ext_lists,
>> + cold_ext_lists,
>>   hot_ext_lists
>>  };
>>
>>
>

--
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 1/2] f2fs-tools: improve loggings

[Ju Hyung Park]

Hi Sheng,

> Shall we handle the last '\n' in different print functions the same way?
Some MSG() calls didn't have '\n' at the end with a very good reason.
For consistencies between MSG, INFO_MSG and ERR_MSG, I left "\n" to the caller.

> And what is the purpose of TRACE_ERR_MSG()
TRACE_ERR_MSG() was previously ERR_MSG().
It prints the function and line number for developers to easily
identify where the log came from.

> and PERR_MSG()?
perror() + ERR_MSG().
It prints out POSIX error's description to tell users why if failed.
"No such file or directory" or "Permission denied" is a good example.

On Mon, Mar 19, 2018 at 12:02 PM, Sheng Yong  wrote:
> Hi, Ju Hyung and list,
>
> On 2018/3/17 23:04, Park Ju Hyung wrote:
>>
>>   - Print errors and warnings to stderr
>>   - Print errors and warnings regardless of debugging level
>>   - Make info/error/warning logs consistent
>>   - Print POSIX errors when possible
>>   - Use more consistent terms and grammar
>>
> Shall we handle the last '\n' in different print functions the same way?
> ASSERT_MSG() and ASSERT() have '\n' by default, while others don't.
> And what is the purpose of TRACE_ERR_MSG() and PERR_MSG()?
>
> thanks,
> Sheng
>
>> Signed-off-by: Park Ju Hyung 
>> ---
>>   fsck/defrag.c|   4 +-
>>   fsck/dir.c   |  20 +++
>>   fsck/dump.c  |  12 ++--
>>   fsck/fsck.c  |  14 ++---
>>   fsck/main.c  |  73 
>>   fsck/mount.c |  62 ++---
>>   fsck/quotaio_tree.c  |   2 +-
>>   fsck/resize.c|   8 +--
>>   fsck/segment.c   |   4 +-
>>   fsck/sload.c |  29 +-
>>   include/f2fs_fs.h|  34 ++--
>>   lib/libf2fs.c|  51 +
>>   lib/libf2fs_io.c |  10 ++--
>>   lib/libf2fs_zoned.c  |  21 ---
>>   mkfs/f2fs_format.c   | 140
>> +++
>>   mkfs/f2fs_format_main.c  |  54 +-
>>   mkfs/f2fs_format_utils.c |  14 ++---
>>   17 files changed, 291 insertions(+), 261 deletions(-)
>>
>> diff --git a/fsck/defrag.c b/fsck/defrag.c
>> index bea0293..6ceb1f5 100644
>> --- a/fsck/defrag.c
>> +++ b/fsck/defrag.c
>> @@ -78,12 +78,12 @@ int f2fs_defragment(struct f2fs_sb_info *sbi, u64
>> from, u64 len, u64 to, int lef
>> continue;
>> if (find_next_free_block(sbi, , left, se->type)) {
>> -   MSG(0, "Not enough space to migrate blocks");
>> +   ERR_MSG("Not enough space to migrate blocks");
>> return -1;
>> }
>> if (migrate_block(sbi, idx, target)) {
>> -   ASSERT_MSG("Found inconsistency: please run
>> FSCK");
>> +   ASSERT_MSG("Found inconsistency, please run
>> FSCK");
>> return -1;
>> }
>> }
>> diff --git a/fsck/dir.c b/fsck/dir.c
>> index 567a4e9..2e45838 100644
>> --- a/fsck/dir.c
>> +++ b/fsck/dir.c
>> @@ -235,7 +235,7 @@ int f2fs_add_link(struct f2fs_sb_info *sbi, struct
>> f2fs_node *parent,
>> return -EINVAL;
>> if (!pino) {
>> -   ERR_MSG("Wrong parent ino:%d \n", pino);
>> +   TRACE_ERR_MSG("Wrong parent ino: %d\n", pino);
>> return -EINVAL;
>> }
>>   @@ -246,7 +246,7 @@ int f2fs_add_link(struct f2fs_sb_info *sbi, struct
>> f2fs_node *parent,
>>   start:
>> if (current_depth == MAX_DIR_HASH_DEPTH) {
>> free(dentry_blk);
>> -   ERR_MSG("\tError: MAX_DIR_HASH\n");
>> +   TRACE_ERR_MSG("MAX_DIR_HASH\n");
>> return -ENOSPC;
>> }
>>   @@ -524,7 +524,7 @@ int convert_inline_dentry(struct f2fs_sb_info *sbi,
>> struct f2fs_node *node,
>> ret = dev_write_block(dentry_blk, dn.data_blkaddr);
>> ASSERT(ret >= 0);
>>   - MSG(1, "%s: copy inline entry to block\n", __func__);
>> +   MSG(1, "%s: copying inline entry to block\n", __func__);
>> free(dentry_blk);
>> return ret;
>> @@ -561,9 +561,9 @@ int convert_inline_dentry(struct f2fs_sb_info *sbi,
>> struct f2fs_node *node,
>> le32_to_cpu(de->ino),
>> de->file_type, p_blkaddr, 0);
>> if (ret)
>> -   MSG(0, "Convert file \"%s\" ERR=%d\n", filename,
>> ret);
>> +   MSG(0, "Failed to convert file \"%s\", ERR =
>> %d\n", filename, ret);
>> else
>> -   MSG(1, "%s: add inline entry to block\n",
>> __func__);
>> +   MSG(1, "%s: adding inline entry to block\n",
>> __func__);
>> bit_pos += GET_DENTRY_SLOTS(namelen);
>> }
>> @@ -582,7 +582,7 @@ int f2fs_create(struct f2fs_sb_info *sbi, 

Re: [f2fs-dev] [PATCH] mkfs.f2fs: set .so to cold files

[Ju Hyung Park]

I'm not entirely familiar with how cold and hot files are treated in f2fs,
but I'm going to assume it has to do something with GC and
less-prioritizing for random R/W operations.

If my assumption is correct, I see some things problematic here.

Android still does a lot of random reads inside .apk files.
While the Java bytecodes are compiled and stored separately in
/data/app/*/base.odex and base.vdex,
resources(graphics, assets, etc) are still directly accessed from the
.apk files(which btw, I really want Google to change to access via
Linux filesystem directly).

And .so extensions are not Android specific. Those are shared
libraries and desktop distros also use them.
A simple `find /usr /lib* -name '*.so' | wc -l` run on my Ubuntu setup
returns 7382 files. While most Android's /system partition is
formatted as ext4, I'm not sure if you still want to do that for the
rest of the user app's libraries(stored in /data/app/*/lib).

Are these really intended?
I'd appreciate if someone explain how cold and hot files are treated
differently.

And also, I don't understand why one would want this hardcoded to the
block device at the time of mkfs.
Wouldn't it make much more sense to ship new lists on each Linux merge
windows and allow users to customize on runtime via sysfs?

On Fri, Nov 3, 2017 at 12:13 PM, Jaegeuk Kim  wrote:
> This patch adds .so in cold file extention list.
>
> Signed-off-by: Jaegeuk Kim 
> ---
>  mkfs/f2fs_format.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
> index 2103f9d..2ba8dd3 100644
> --- a/mkfs/f2fs_format.c
> +++ b/mkfs/f2fs_format.c
> @@ -62,6 +62,7 @@ const char *media_ext_lists[] = {
> "jpeg",
> "video",
> "apk",  /* for android system */
> +   "so",   /* for android system */
> NULL
>  };
>
> --
> 2.14.0.rc1.383.gd1ce394fe2-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

--
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] [RFC PATCH 1/3] f2fs: introduce sysfs readdir_ra to readahead inode block in readdir

[Ju Hyung Park]

May I ask why is this disabled by default?

On Thu, Nov 23, 2017 at 10:11 PM, Chao Yu  wrote:
> On 2017/11/22 18:23, Sheng Yong wrote:
>> This patch introduces a sysfs interface readdir_ra to enable/disable
>> readaheading inode block in f2fs_readdir. When readdir_ra is enabled,
>> it improves the performance of "readdir + stat".
>>
>> For 300,000 files:
>>   time find /data/test > /dev/null
>> disable readdir_ra: 1m25.69s real  0m01.94s user  0m50.80s system
>> enable  readdir_ra: 0m18.55s real  0m00.44s user  0m15.39s system
>>
>> Signed-off-by: Sheng Yong 
>
> Reviewed-by: Chao Yu 
>
> Thanks,
>
>> ---
>>  Documentation/ABI/testing/sysfs-fs-f2fs | 6 ++
>>  fs/f2fs/dir.c   | 4 
>>  fs/f2fs/f2fs.h  | 1 +
>>  fs/f2fs/sysfs.c | 2 ++
>>  4 files changed, 13 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs 
>> b/Documentation/ABI/testing/sysfs-fs-f2fs
>> index a7799c2fca28..d870b5514d15 100644
>> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
>> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
>> @@ -186,3 +186,9 @@ Date: August 2017
>>  Contact: "Jaegeuk Kim" 
>>  Description:
>>Controls sleep time of GC urgent mode
>> +
>> +What:/sys/fs/f2fs//readdir_ra
>> +Date:November 2017
>> +Contact: "Sheng Yong" 
>> +Description:
>> +  Controls readahead inode block in readdir.
>> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
>> index 2d98d877c09d..724304dc6143 100644
>> --- a/fs/f2fs/dir.c
>> +++ b/fs/f2fs/dir.c
>> @@ -798,6 +798,7 @@ int f2fs_fill_dentries(struct dir_context *ctx, struct 
>> f2fs_dentry_ptr *d,
>>   unsigned int bit_pos;
>>   struct f2fs_dir_entry *de = NULL;
>>   struct fscrypt_str de_name = FSTR_INIT(NULL, 0);
>> + struct f2fs_sb_info *sbi = F2FS_I_SB(d->inode);
>>
>>   bit_pos = ((unsigned long)ctx->pos % d->max);
>>
>> @@ -836,6 +837,9 @@ int f2fs_fill_dentries(struct dir_context *ctx, struct 
>> f2fs_dentry_ptr *d,
>>   le32_to_cpu(de->ino), d_type))
>>   return 1;
>>
>> + if (sbi->readdir_ra == 1)
>> + ra_node_page(sbi, le32_to_cpu(de->ino));
>> +
>>   bit_pos += GET_DENTRY_SLOTS(le16_to_cpu(de->name_len));
>>   ctx->pos = start_pos + bit_pos;
>>   }
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index ca6b0c9bc621..a269d795ba7c 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -1095,6 +1095,7 @@ struct f2fs_sb_info {
>>   int dir_level;  /* directory level */
>>   int inline_xattr_size;  /* inline xattr size */
>>   unsigned int trigger_ssr_threshold; /* threshold to trigger ssr */
>> + int readdir_ra; /* readahead inode in readdir 
>> */
>>
>>   block_t user_block_count;   /* # of user blocks */
>>   block_t total_valid_block_count;/* # of valid blocks */
>> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
>> index 9835348b6e5d..93c3364250dd 100644
>> --- a/fs/f2fs/sysfs.c
>> +++ b/fs/f2fs/sysfs.c
>> @@ -299,6 +299,7 @@ F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, dir_level, 
>> dir_level);
>>  F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, cp_interval, interval_time[CP_TIME]);
>>  F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, idle_interval, 
>> interval_time[REQ_TIME]);
>>  F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, iostat_enable, iostat_enable);
>> +F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, readdir_ra, readdir_ra);
>>  #ifdef CONFIG_F2FS_FAULT_INJECTION
>>  F2FS_RW_ATTR(FAULT_INFO_RATE, f2fs_fault_info, inject_rate, inject_rate);
>>  F2FS_RW_ATTR(FAULT_INFO_TYPE, f2fs_fault_info, inject_type, inject_type);
>> @@ -346,6 +347,7 @@ static struct attribute *f2fs_attrs[] = {
>>   ATTR_LIST(cp_interval),
>>   ATTR_LIST(idle_interval),
>>   ATTR_LIST(iostat_enable),
>> + ATTR_LIST(readdir_ra),
>>  #ifdef CONFIG_F2FS_FAULT_INJECTION
>>   ATTR_LIST(inject_rate),
>>   ATTR_LIST(inject_type),
>>
>
> --
> 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

--
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] f2fs: Fix bool initialization/comparison

[Ju Hyung Park]

Isn't this bogus?

"bool" type in Linux kernel is a typedef to "_Bool"
and true/false is defined as 1 and 0 by enum at include/linux/stddef.h.

On Sat, Oct 7, 2017 at 11:02 PM, Thomas Meyer  wrote:
> Bool initializations should use true and false. Bool tests don't need
> comparisons.
>
> Signed-off-by: Thomas Meyer 
> ---
>
> diff -u -p a/fs/f2fs/data.c b/fs/f2fs/data.c
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -419,7 +419,7 @@ next:
> bio_page = fio->encrypted_page ? fio->encrypted_page : fio->page;
>
> /* set submitted = 1 as a return value */
> -   fio->submitted = 1;
> +   fio->submitted = true;
>
> inc_page_count(sbi, WB_DATA_TYPE(bio_page));
>
>
> --
> 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

--
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 v4] f2fs: introduce discard_granularity sysfs entry

[Ju Hyung Park]

Hi Chao.

Yep, that patch seems to have fixed it.
Doing "while true; do fstrim -v /; done" while "rm -rf"ing a 2GB
kbuild directory
(with lots of small .o files and stuff) ended flawlessly.

I hope to see this patch merged with next 4.14 merge cycle.

Thanks :)

On Tue, Oct 3, 2017 at 12:59 AM, Chao Yu <c...@kernel.org> wrote:
> Hi Park,
>
> Thanks for the report, could have a try with below patch:
>
> From 5fa30e8cdcb93f210e25142c48a884be383c6121 Mon Sep 17 00:00:00 2001
> From: Chao Yu <yuch...@huawei.com>
> Date: Mon, 2 Oct 2017 02:50:16 +0800
> Subject: [PATCH] f2fs: fix potential panic during fstrim
>
> As Ju Hyung Park reported:
>
> "When 'fstrim' is called for manual trim, a BUG() can be triggered
> randomly with this patch.
>
> I'm seeing this issue on both x86 Desktop and arm64 Android phone.
>
> On x86 Desktop, this was caused during Ubuntu boot-up. I have a
> cronjob installed which calls 'fstrim -v /' during boot. On arm64
> Android, this was caused during GC looping with 1ms gc_min_sleep_time
> & gc_max_sleep_time."
>
> Root cause of this issue is that f2fs_wait_discard_bios can only be
> used by f2fs_put_super, because during put_super there must be no
> other referrers, so it can ignore discard entry's reference count
> when removing the entry, otherwise in other caller we will hit bug_on
> in __remove_discard_cmd as there may be other issuer added reference
> count in discard entry.
>
> Thread AThread B
> - issue_discard_thread
> - f2fs_ioc_fitrim
>  - f2fs_trim_fs
>   - f2fs_wait_discard_bios
>- __issue_discard_cmd
> - __submit_discard_cmd
>  - __wait_discard_cmd
>   - dc->ref++
>   - __wait_one_discard_bio
>    - __wait_discard_cmd
> - __remove_discard_cmd
>  - f2fs_bug_on(sbi, dc->ref)
>
> Fixes: 969d1b180d987c2be02de890d0fff0f66a0e80de
> Reported-by: Ju Hyung Park <qkrwngud...@gmail.com>
> Signed-off-by: Chao Yu <yuch...@huawei.com>
> ---
>  fs/f2fs/f2fs.h| 2 +-
>  fs/f2fs/segment.c | 6 +++---
>  fs/f2fs/super.c   | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 9a7c90386947..4b4a72f392be 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2525,7 +2525,7 @@ void invalidate_blocks(struct f2fs_sb_info *sbi, 
> block_t addr);
>  bool is_checkpointed_data(struct f2fs_sb_info *sbi, block_t blkaddr);
>  void refresh_sit_entry(struct f2fs_sb_info *sbi, block_t old, block_t new);
>  void stop_discard_thread(struct f2fs_sb_info *sbi);
> -void f2fs_wait_discard_bios(struct f2fs_sb_info *sbi);
> +void f2fs_wait_discard_bios(struct f2fs_sb_info *sbi, bool umount);
>  void clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control 
> *cpc);
>  void release_discard_addrs(struct f2fs_sb_info *sbi);
>  int npages_for_summary_flush(struct f2fs_sb_info *sbi, bool for_ra);
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index dedf0209d820..e28245b7e44e 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -1210,11 +1210,11 @@ void stop_discard_thread(struct f2fs_sb_info *sbi)
>  }
>
>  /* This comes from f2fs_put_super and f2fs_trim_fs */
> -void f2fs_wait_discard_bios(struct f2fs_sb_info *sbi)
> +void f2fs_wait_discard_bios(struct f2fs_sb_info *sbi, bool umount)
>  {
> __issue_discard_cmd(sbi, false);
> __drop_discard_cmd(sbi);
> -   __wait_discard_cmd(sbi, false);
> +   __wait_discard_cmd(sbi, !umount);
>  }
>
>  static void mark_discard_range_all(struct f2fs_sb_info *sbi)
> @@ -2244,7 +2244,7 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct 
> fstrim_range *range)
> }
> /* It's time to issue all the filed discards */
> mark_discard_range_all(sbi);
> -   f2fs_wait_discard_bios(sbi);
> +   f2fs_wait_discard_bios(sbi, false);
>  out:
> range->len = F2FS_BLK_TO_BYTES(cpc.trimmed);
> return err;
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 89f61eb3d167..933c3d529e65 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -801,7 +801,7 @@ static void f2fs_put_super(struct super_block *sb)
> }
>
> /* be sure to wait for any on-going discard commands */
> -   f2fs_wait_discard_bios(sbi);
> +   f2fs_wait_discard_bios(sbi, true);
>
> if (f2fs_discard_en(sbi) && !sbi->discard_blks) {
> struct cp_control cpc = {
> --
> 2.14.1.145.gb3622a4ee
>
> On 2017/10/2 3:29

Re: [f2fs-dev] [PATCH v4] f2fs: introduce discard_granularity sysfs entry

[Ju Hyung Park]

When 'fstrim' is called for manual trim, a BUG() can be triggered
randomly with this patch.

I'm seeing this issue on both x86 Desktop and arm64 Android phone.

On x86 Desktop, this was caused during Ubuntu boot-up. I have a
cronjob installed
which calls 'fstrim -v /' during boot.
On arm64 Android, this was caused during GC looping with
1ms gc_min_sleep_time & gc_max_sleep_time.

Thanks.

[26671.666421] [ cut here ]
[26671.666426] WARNING: CPU: 8 PID: 103479 at fs/f2fs/segment.c:797
__remove_discard_cmd+0xb9/0xd0
[26671.666427] Modules linked in: ftdi_sio usbserial uas usb_storage
vmnet(O) vmw_vsock_vmci_transport vsock vmw_vmci vmmon(O) rfcomm
xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4
iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4
xt_conntrack nf_conntrack ipt_REJECT nf_reject_ipv4 xt_tcpudp bridge
stp llc bnep ebtable_filter ebtables ip6table_filter ip6_tables
xt_multiport iptable_filter binfmt_misc snd_hda_codec_hdmi eeepc_wmi
asus_wmi sparse_keymap video wmi_bmof mxm_wmi nls_iso8859_1 btusb
btrtl joydev btbcm btintel input_leds bluetooth edac_mce_amd
snd_hda_codec_realtek snd_hda_codec_generic kvm_amd kvm irqbypass
snd_seq_dummy snd_hda_intel snd_hda_codec snd_seq_oss snd_seq_midi
snd_hda_core snd_seq_midi_event snd_hwdep snd_pcm snd_rawmidi snd_seq
snd_seq_device
[26671.666450]  snd_timer snd soundcore k10temp i2c_piix4
nvidia_uvm(PO) shpchp wmi 8250_dw mac_hid parport_pc ppdev nfsd lp
auth_rpcgss parport oid_registry nfs_acl lockd grace sunrpc ip_tables
x_tables autofs4 raid10 raid456 async_raid6_recov async_memcpy
async_pq async_xor async_tx raid1 multipath linear hid_generic
hid_apple usbhid nvidia_drm(PO) nvidia_modeset(PO) nvidia(PO)
drm_kms_helper syscopyarea sysfillrect sysimgblt igb fb_sys_fops dca
ptp drm pps_core i2c_algo_bit ahci libahci gpio_amdpt gpio_generic
[26671.666471] CPU: 8 PID: 103479 Comm: fstrim Tainted: P   O
  4.13.4-zen+ #1
[26671.666472] Hardware name: System manufacturer System Product
Name/PRIME X399-A, BIOS 0318 08/11/2017
[26671.666472] task: 8804ad535800 task.stack: 88047ee38000
[26671.666474] RIP: 0010:__remove_discard_cmd+0xb9/0xd0
[26671.666474] RSP: 0018:88047ee3bd00 EFLAGS: 00010202
[26671.666475] RAX: 88081801a500 RBX: 88047eeaed00 RCX: 88047eeaedf8
[26671.666475] RDX: 0001 RSI: 88047eeaed00 RDI: 880802555800
[26671.666476] RBP: 8808134d R08: 8804ad535800 R09: 0001
[26671.666476] R10: 88047ee3bd18 R11:  R12: 880802555800
[26671.666476] R13:  R14: 88047eeaedd0 R15: 8808134d
[26671.666477] FS:  7f44cddad2c0() GS:88081ca0()
knlGS:
[26671.666478] CS:  0010 DS:  ES:  CR0: 80050033
[26671.666478] CR2: 7f88f5776328 CR3: 00058ce4a000 CR4: 003406e0
[26671.666479] Call Trace:
[26671.666481]  ? __wait_discard_cmd+0x7a/0xc0
[26671.666482]  ? f2fs_trim_fs+0x1c1/0x210
[26671.666484]  ? f2fs_ioctl+0x75a/0x2320
[26671.666486]  ? do_filp_open+0x99/0xe0
[26671.666487]  ? cp_new_stat+0x138/0x150
[26671.666489]  ? do_vfs_ioctl+0x88/0x5c0
[26671.666490]  ? SyS_newfstat+0x29/0x40
[26671.666491]  ? SyS_ioctl+0x6f/0x80
[26671.666493]  ? entry_SYSCALL_64_fastpath+0x1e/0xa9
[26671.666493] Code: 48 89 de 8b 43 1c 48 8b 3d 4d 8c 31 01 29 85 74
22 00 00 e8 fa 01 d5 ff f0 ff 8d 80 22 00 00 5b 5d c3 c7 43 64 00 00
00 00 eb 92 <0f> ff f0 80 4f 20 04 e9 53 ff ff ff 90 66 2e 0f 1f 84 00
00 00
[26671.666506] ---[ end trace 613553f7a4728b5a ]---
[26672.553742] general protection fault:  [#1] PREEMPT SMP
[26672.553746] Modules linked in: ftdi_sio usbserial uas usb_storage
vmnet(O) vmw_vsock_vmci_transport vsock vmw_vmci vmmon(O) rfcomm
xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4
iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4
xt_conntrack nf_conntrack ipt_REJECT nf_reject_ipv4 xt_tcpudp bridge
stp llc bnep ebtable_filter ebtables ip6table_filter ip6_tables
xt_multiport iptable_filter binfmt_misc snd_hda_codec_hdmi eeepc_wmi
asus_wmi sparse_keymap video wmi_bmof mxm_wmi nls_iso8859_1 btusb
btrtl joydev btbcm btintel input_leds bluetooth edac_mce_amd
snd_hda_codec_realtek snd_hda_codec_generic kvm_amd kvm irqbypass
snd_seq_dummy snd_hda_intel snd_hda_codec snd_seq_oss snd_seq_midi
snd_hda_core snd_seq_midi_event snd_hwdep snd_pcm snd_rawmidi snd_seq
snd_seq_device
[26672.553771]  snd_timer snd soundcore k10temp i2c_piix4
nvidia_uvm(PO) shpchp wmi 8250_dw mac_hid parport_pc ppdev nfsd lp
auth_rpcgss parport oid_registry nfs_acl lockd grace sunrpc ip_tables
x_tables autofs4 raid10 raid456 async_raid6_recov async_memcpy
async_pq async_xor async_tx raid1 multipath linear hid_generic
hid_apple usbhid nvidia_drm(PO) nvidia_modeset(PO) nvidia(PO)
drm_kms_helper syscopyarea sysfillrect sysimgblt igb fb_sys_fops dca
ptp drm pps_core i2c_algo_bit ahci libahci gpio_amdpt gpio_generic
[26672.553792] CPU: 10 PID: 1287