Re: [PATCH] f2fs: fix to recover cold bit of inode block during POR

From: Jaegeuk Kim
Date: Tue Oct 02 2018 - 20:35:09 EST


On 10/02, Chao Yu wrote:
> From: Chao Yu <yuchao0@xxxxxxxxxx>
>
> Testcase to reproduce this bug:
> 1. mkfs.f2fs /dev/sdd
> 2. mount -t f2fs /dev/sdd /mnt/f2fs
> 3. touch /mnt/f2fs/file
> 4. sync
> 5. chattr +A /mnt/f2fs/file
> 6. xfs_io -f /mnt/f2fs/file -c "fsync"
> 7. godown /mnt/f2fs
> 8. umount /mnt/f2fs
> 9. mount -t f2fs /dev/sdd /mnt/f2fs
> 10. chattr -A /mnt/f2fs/file
> 11. xfs_io -f /mnt/f2fs/file -c "fsync"
> 12. umount /mnt/f2fs
> 13. mount -t f2fs /dev/sdd /mnt/f2fs
> 14. lsattr /mnt/f2fs/file
>
> -----------------N- /mnt/f2fs/file
>
> But actually, we expect the corrct result is:
>
> -------A---------N- /mnt/f2fs/file
>
> The reason is in step 9) we missed to recover cold bit flag in inode
> block, so later, in fsync, we will skip write inode block due to below
> condition check, result in lossing data in another SPOR.
>
> f2fs_fsync_node_pages()
> if (!IS_DNODE(page) || !is_cold_node(page))
> continue;
>
> Note that, I guess that some non-dir inode has already lost cold bit
> during POR, so in order to reenable recovery for those inode, let's
> try to recover cold bit in f2fs_iget() to save more fsynced data.

Please check that this breaks inode checksum when mounting the fresh partition.

>
> Fixes: c56675750d7c ("f2fs: remove unneeded set_cold_node()")
> Cc: <stable@xxxxxxxxxxxxxxx> 4.17+
> Signed-off-by: Chao Yu <yuchao0@xxxxxxxxxx>
> ---
> fs/f2fs/inode.c | 4 ++++
> fs/f2fs/node.c | 2 +-
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> index 3c278e63d1a3..4ce4d6b298f9 100644
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -365,6 +365,10 @@ static int do_read_inode(struct inode *inode)
> if (f2fs_has_inline_data(inode) && !f2fs_exist_data(inode))
> __recover_inline_status(inode, node_page);
>
> + /* try to recover cold bit for non-dir inode */
> + if (!S_ISDIR(inode->i_mode) && !is_cold_node(node_page))
> + set_cold_node(node_page, false);
> +
> /* get rdev by using inline_info */
> __get_inode_rdev(inode, ri);
>
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index b3cf18eb12f0..b1edc7525597 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -2549,7 +2549,7 @@ int f2fs_recover_inode_page(struct f2fs_sb_info *sbi, struct page *page)
> if (!PageUptodate(ipage))
> SetPageUptodate(ipage);
> fill_node_footer(ipage, ino, ino, 0, true);
> - set_cold_node(page, false);
> + set_cold_node(ipage, false);
>
> src = F2FS_INODE(page);
> dst = F2FS_INODE(ipage);
> --
> 2.18.0