On 12/15, heyunlei wrote:
>
>
> >-----Original Message-----
> >From: Jaegeuk Kim [mailto:[email protected]]
> >Sent: Friday, December 15, 2017 3:46 AM
> >To: heyunlei
> >Cc: Yuchao (T); [email protected]; Wangbintian; Jianing
> >(Euler)
> >Subject: Re: [f2fs-dev][PATCH RFC] f2fs: fix an error case of missing update
> >inode page
> >
> >On 12/05, Yunlei He wrote:
> >> -Thread A Thread B
> >>
> >> -write_checkpoint
> >> -block_operations
> >> -f2fs_unlock_all -f2fs_sync_file
> >> -f2fs_write_inode
> >>
> >> -f2fs_inode_synced
> >>
> >> -f2fs_sync_inode_meta
> >> -sync_node_pages
> >>
> >> -set_page_drity
> >>
> >> In this case, if sudden power off without next new checkpoint,
> >> the last inode page update will lost. wb_writeback is same with
> >> fsync.
> >
> >Does this patch fix the problem?
> >
> I modify code as below:
> @@ -366,7 +366,7 @@ int update_inode(struct inode *inode, struct page
> *node_page)
> struct extent_tree *et = F2FS_I(inode)->extent_tree;
>
> f2fs_inode_synced(inode);
> -
> + msleep(10000);
> f2fs_wait_on_page_writeback(node_page, NODE, true);
>
> shell 1: shell2:
> dd if=/dev/zero of=./test bs=1M count=10
> sync
> echo "hello" >> ./test
> fsync test // sleep 10s
> sync //return quickly
>
> echo c > /proc/sysrq-trigger //before fsync return
> fsck will find iblocks mismatch, and with this patch it 's ok.
So, do you mean it was fixed by adding msleep() on top of this patch as well?
Like:
- f2fs_write_inode()
- set_page_drity
- f2fs_inode_synced
- msleep(10000);
>
> Besides,I came across another error like this:
>
> f2fs_inode-> i_xattr_nid is non-zero, but node blkaddr in raw nat entry is
> NULL.
>
> This case makes sense to this problem also.
>
> > -write_checkpoint
> > -block_operations
> > -f2fs_unlock_all -f2fs_sync_file
> > -f2fs_write_inode
> > -set_page_drity
> >
> > -f2fs_inode_synced
> > -f2fs_sync_inode_meta
> > -sync_node_pages
> >
> >I think, we need to cover f2fs_lock_op() for f2fs_write_inode() again.
>
> It can also work well with f2fs_lock_op(), I am afraid other cases will
> update inode
> like f2fs_write_inode without f2fs_lock_op
>
> Thanks
> >
> >Thanks,
> >
> >>
> >> Signed-off-by: Yunlei He <[email protected]>
> >> ---
> >> fs/f2fs/f2fs.h | 4 ++--
> >> fs/f2fs/inode.c | 15 +++++++--------
> >> 2 files changed, 9 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> >> index 82f1dc3..38f9324 100644
> >> --- a/fs/f2fs/f2fs.h
> >> +++ b/fs/f2fs/f2fs.h
> >> @@ -2513,8 +2513,8 @@ int f2fs_getattr(const struct path *path, struct
> >> kstat *stat,
> >> struct inode *f2fs_iget(struct super_block *sb, unsigned long ino);
> >> struct inode *f2fs_iget_retry(struct super_block *sb, unsigned long ino);
> >> int try_to_free_nats(struct f2fs_sb_info *sbi, int nr_shrink);
> >> -int update_inode(struct inode *inode, struct page *node_page);
> >> -int update_inode_page(struct inode *inode);
> >> +void update_inode(struct inode *inode, struct page *node_page);
> >> +void update_inode_page(struct inode *inode);
> >> int f2fs_write_inode(struct inode *inode, struct writeback_control *wbc);
> >> void f2fs_evict_inode(struct inode *inode);
> >> void handle_failed_inode(struct inode *inode);
> >> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> >> index b4c4f2b..10d3c7c 100644
> >> --- a/fs/f2fs/inode.c
> >> +++ b/fs/f2fs/inode.c
> >> @@ -360,14 +360,15 @@ struct inode *f2fs_iget_retry(struct super_block
> >> *sb, unsigned long ino)
> >> return inode;
> >> }
> >>
> >> -int update_inode(struct inode *inode, struct page *node_page)
> >> +void update_inode(struct inode *inode, struct page *node_page)
> >> {
> >> struct f2fs_inode *ri;
> >> struct extent_tree *et = F2FS_I(inode)->extent_tree;
> >>
> >> - f2fs_inode_synced(inode);
> >> -
> >> f2fs_wait_on_page_writeback(node_page, NODE, true);
> >> + set_page_dirty(node_page);
> >> +
> >> + f2fs_inode_synced(inode);
> >>
> >> ri = F2FS_INODE(node_page);
> >>
> >> @@ -426,10 +427,9 @@ int update_inode(struct inode *inode, struct page
> >> *node_page)
> >> if (inode->i_nlink == 0)
> >> clear_inline_node(node_page);
> >>
> >> - return set_page_dirty(node_page);
> >> }
> >>
> >> -int update_inode_page(struct inode *inode)
> >> +void update_inode_page(struct inode *inode)
> >> {
> >> struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> >> struct page *node_page;
> >> @@ -444,11 +444,10 @@ int update_inode_page(struct inode *inode)
> >> } else if (err != -ENOENT) {
> >> f2fs_stop_checkpoint(sbi, false);
> >> }
> >> - return 0;
> >> + return;
> >> }
> >> - ret = update_inode(inode, node_page);
> >> + update_inode(inode, node_page);
> >> f2fs_put_page(node_page, 1);
> >> - return ret;
> >> }
> >>
> >> int f2fs_write_inode(struct inode *inode, struct writeback_control *wbc)
> >> --
> >> 1.9.1
------------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
