[PATCH] f2fs-tools: don't finalize device early

[PATCH] f2fs-tools: don't finalize device early

[Jaegeuk Kim]

We should close the device by main body of each tools.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fsck/mount.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fsck/mount.c b/fsck/mount.c
index 7a28314..c78314e 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -1941,9 +1941,6 @@ void write_checkpoint(struct f2fs_sb_info *sbi)
 	if (flags & CP_NAT_BITS_FLAG)
 		write_nat_bits(sbi, sb, cp, sbi->cur_cp);
 
-	/* in case of sudden power off */
-	f2fs_finalize_device();
-
 	/* write the last cp */
 	ret = dev_write_block(cp, cp_blk_no++);
 	ASSERT(ret >= 0);
-- 
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

Re: [PATCH] f2fs-tools: don't finalize device early

[Chao Yu]

On 2017/10/15 3:22, Jaegeuk Kim wrote:
> We should close the device by main body of each tools.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fsck/mount.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/fsck/mount.c b/fsck/mount.c
> index 7a28314..c78314e 100644
> --- a/fsck/mount.c
> +++ b/fsck/mount.c
> @@ -1941,9 +1941,6 @@ void write_checkpoint(struct f2fs_sb_info *sbi)
>  	if (flags & CP_NAT_BITS_FLAG)
>  		write_nat_bits(sbi, sb, cp, sbi->cur_cp);
>  
> -	/* in case of sudden power off */
> -	f2fs_finalize_device();

IIRC, Yunlei wants to fsync previous data in checkpoint area before write last
CP pack, in order to avoid checkpoint corruption in sudden-power-off case.

Shouldn't we use fsync(main-device-fd) here instead?

Thanks,

> -
>  	/* write the last cp */
>  	ret = dev_write_block(cp, cp_blk_no++);
>  	ASSERT(ret >= 0);
> 


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH] f2fs-tools: don't finalize device early

[Jaegeuk Kim]

On 10/16, Chao Yu wrote:
> On 2017/10/15 3:22, Jaegeuk Kim wrote:
> > We should close the device by main body of each tools.
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >  fsck/mount.c | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/fsck/mount.c b/fsck/mount.c
> > index 7a28314..c78314e 100644
> > --- a/fsck/mount.c
> > +++ b/fsck/mount.c
> > @@ -1941,9 +1941,6 @@ void write_checkpoint(struct f2fs_sb_info *sbi)
> >  	if (flags & CP_NAT_BITS_FLAG)
> >  		write_nat_bits(sbi, sb, cp, sbi->cur_cp);
> >  
> > -	/* in case of sudden power off */
> > -	f2fs_finalize_device();
> 
> IIRC, Yunlei wants to fsync previous data in checkpoint area before write last
> CP pack, in order to avoid checkpoint corruption in sudden-power-off case.
> 
> Shouldn't we use fsync(main-device-fd) here instead?

Yunlei,

If it's correct, could you write a patch introducing a generic function to call
fsync() only? I should have reviewed the original patch tho.

Thanks,

> 
> Thanks,
> 
> > -
> >  	/* write the last cp */
> >  	ret = dev_write_block(cp, cp_blk_no++);
> >  	ASSERT(ret >= 0);
> > 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot