RE: [PATCH 2/2] f2fs: don't discard next free dnode page for an umount checkpoint

[Chao Yu <chao2.yu@samsung.com>]

Hi Jaegeuk,

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Tuesday, February 03, 2015 7:38 AM
> To: Chao Yu
> Cc: Changman Lee; linux-f2fs-devel@lists.sourceforge.net; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 2/2] f2fs: don't discard next free dnode page for an umount checkpoint
> 
> Hi Chao,
> 
> On Sat, Jan 31, 2015 at 05:06:59PM +0800, Chao Yu wrote:
> > Previously, discard_next_dnode is added before a checkpoint to prevent that we
> > may meet a garbage dnode page readed from next free blkaddr in recover flow.
> >
> > Since f2fs will skip recovery flow for a clean umount image, this condition will
> > never happen.
> >
> > So it's safe for us to leave next free dnode as it is in an umount checkpoint.
> >
> > Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> > ---
> >  fs/f2fs/checkpoint.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> > index f7cdcad..991fd0a 100644
> > --- a/fs/f2fs/checkpoint.c
> > +++ b/fs/f2fs/checkpoint.c
> > @@ -905,8 +905,12 @@ static void do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control
> *cpc)
> >  	/*
> >  	 * This avoids to conduct wrong roll-forward operations and uses
> >  	 * metapages, so should be called prior to sync_meta_pages below.
> > +	 * But if we are in an umount checkpoint, we'd better skip this
> > +	 * because we will not enter recovery flow to use the next free
> > +	 * blkaddr when mounting it.
> >  	 */
> > -	discard_next_dnode(sbi, NEXT_FREE_BLKADDR(sbi, curseg));
> > +	if (cpc->reason != CP_UMOUNT)
> > +		discard_next_dnode(sbi, NEXT_FREE_BLKADDR(sbi, curseg));
> 
> The reason for discard_next_dnode is to avoid wrong execution due to old
> mkfs.f2fs which remains gabage data.
> It needs to do all the time.

Maybe my previously understanding is wrong.

Is this issue due to old mkfs.f2fs do not discard entire flash storage device
when formating? Or another bug of mkfs? If so, can you please offer a fix
commit id of mkfs?

I'm curious about how this happened. :-)

Thanks,
> 
> Thanks,
> 
> >
> >  	/* Flush all the NAT/SIT pages */
> >  	while (get_pages(sbi, F2FS_DIRTY_META)) {
> > --
> > 2.2.1