* [PATCH 2/2] f2fs: don't discard next free dnode page for an umount checkpoint @ 2015-01-31 9:06 Chao Yu 2015-02-02 23:37 ` Jaegeuk Kim 0 siblings, 1 reply; 4+ messages in thread From: Chao Yu @ 2015-01-31 9:06 UTC (permalink / raw) To: Jaegeuk Kim, Changman Lee; +Cc: linux-f2fs-devel, linux-kernel 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)); /* Flush all the NAT/SIT pages */ while (get_pages(sbi, F2FS_DIRTY_META)) { -- 2.2.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] f2fs: don't discard next free dnode page for an umount checkpoint 2015-01-31 9:06 [PATCH 2/2] f2fs: don't discard next free dnode page for an umount checkpoint Chao Yu @ 2015-02-02 23:37 ` Jaegeuk Kim 2015-02-05 8:16 ` Chao Yu 0 siblings, 1 reply; 4+ messages in thread From: Jaegeuk Kim @ 2015-02-02 23:37 UTC (permalink / raw) To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel 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. Thanks, > > /* Flush all the NAT/SIT pages */ > while (get_pages(sbi, F2FS_DIRTY_META)) { > -- > 2.2.1 ------------------------------------------------------------------------------ Dive into the World of Parallel Programming. The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/ ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH 2/2] f2fs: don't discard next free dnode page for an umount checkpoint 2015-02-02 23:37 ` Jaegeuk Kim @ 2015-02-05 8:16 ` Chao Yu 2015-02-06 6:22 ` Jaegeuk Kim 0 siblings, 1 reply; 4+ messages in thread From: Chao Yu @ 2015-02-05 8:16 UTC (permalink / raw) To: 'Jaegeuk Kim' Cc: 'Changman Lee', linux-f2fs-devel, linux-kernel 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] f2fs: don't discard next free dnode page for an umount checkpoint 2015-02-05 8:16 ` Chao Yu @ 2015-02-06 6:22 ` Jaegeuk Kim 0 siblings, 0 replies; 4+ messages in thread From: Jaegeuk Kim @ 2015-02-06 6:22 UTC (permalink / raw) To: Chao Yu; +Cc: 'Changman Lee', linux-f2fs-devel, linux-kernel Hi Chao, On Thu, Feb 05, 2015 at 04:16:04PM +0800, Chao Yu wrote: > 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? commit ffbada4298c7aed059bb99a8d553d6dd555963c0 Author: Jaegeuk Kim <jaegeuk.kim@samsung.com> Date: Mon Nov 11 13:29:11 2013 +0900 mkfs: remove stale node blocks If the device does not support discard, we should write zero blocks to avoid roll-forward recovery. Thanks, > > 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-02-06 6:22 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-01-31 9:06 [PATCH 2/2] f2fs: don't discard next free dnode page for an umount checkpoint Chao Yu 2015-02-02 23:37 ` Jaegeuk Kim 2015-02-05 8:16 ` Chao Yu 2015-02-06 6:22 ` Jaegeuk Kim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).