From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Huang Ying <ying.huang@intel.com>
Cc: Changman Lee <cm224.lee@samsung.com>,
linux-f2fs-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org, huang.ying.caritas@gmail.com
Subject: Re: [PATCH] f2fs: Fix recover when nid of non-inode dnode < nid of inode
Date: Tue, 9 Sep 2014 00:09:35 -0700 [thread overview]
Message-ID: <20140909070811.GA26657@jaegeuk-mac02.mot-mobility.com> (raw)
In-Reply-To: <1410241170.732.373.camel@yhuang-dev>
Hi,
On Tue, Sep 09, 2014 at 01:39:30PM +0800, Huang Ying wrote:
> On Mon, 2014-09-08 at 22:23 -0700, Jaegeuk Kim wrote:
> > Hi Huang,
> >
> > On Mon, Sep 08, 2014 at 07:38:26PM +0800, Huang Ying wrote:
> > > For fsync, if the nid of a non-inode dnode < nid of inode and the
> > > inode is not checkpointed. The non-inode dnode may be written before
> > > inode. So in find_fsync_dnodes, f2fs_iget will fail, cause the
> > > recovery fail.
> > >
> > > Usually, inode will be allocated before non-inode dnode, so the nid of
> > > inode < nid of non-inode dnode. But it is possible for the reverse.
> > > For example, because of alloc_nid_failed.
> > >
> > > This is fixed via ignoring non-inode dnode before inode dnode in
> > > find_fsync_dnodes.
> > >
> > > The patch was tested via allocating nid reversely via a debugging
> > > patch, that is, from big number to small number.
> > >
> > > Signed-off-by: Huang, Ying <ying.huang@intel.com>
> > > ---
> > > fs/f2fs/recovery.c | 7 ++++---
> > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > --- a/fs/f2fs/recovery.c
> > > +++ b/fs/f2fs/recovery.c
> > > @@ -172,8 +172,8 @@ static int find_fsync_dnodes(struct f2fs
> > > if (IS_INODE(page) && is_dent_dnode(page))
> > > set_inode_flag(F2FS_I(entry->inode),
> > > FI_INC_LINK);
> > > - } else {
> > > - if (IS_INODE(page) && is_dent_dnode(page)) {
> >
> > If this is not inode block, we should add this inode to recover its data blocks.
>
> Is it possible that there is only non-inode dnode but no inode when
> find_fsync_dnodes checking dnodes? Per my understanding, any changes to
> file will cause inode page dirty (for example, mtime changed), so that
> we will write inode block. Is it right? If so, the solution in this
> patch should work too.
Your description says that f2fs_iget will fail, which causes the recovery fail.
So, I thought it would be better to handle the f2fs_iget failure directly.
In addition, we cannot guarantee the write order of dnode and inode.
For exmaple,
1. the inode is written by flusher or kswapd, then,
2. f2fs_sync_file writes its dnode.
In that case, we can get only non-inode dnode in the node chain, since the inode
has not fsync_mark.
Thanks,
>
> Best Regards,
> Huang, Ying
>
> > Rather than this tweak, if iget is failed, we'd better go to next instead of
> > break.
> > Can you test that?
> >
> > > + } else if (IS_INODE(page)) {
> > > + if (is_dent_dnode(page)) {
> > > err = recover_inode_page(sbi, page);
> > > if (err)
> > > break;
> > > @@ -193,7 +193,8 @@ static int find_fsync_dnodes(struct f2fs
> > > break;
> > > }
> > > list_add_tail(&entry->list, head);
> > > - }
> > > + } else
> > > + goto next;
> > > entry->blkaddr = blkaddr;
> > >
> > > err = recover_inode(entry->inode, page);
next prev parent reply other threads:[~2014-09-09 7:09 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-08 11:38 [PATCH] f2fs: Fix recover when nid of non-inode dnode < nid of inode Huang Ying
2014-09-09 5:23 ` Jaegeuk Kim
2014-09-09 5:39 ` Huang Ying
2014-09-09 7:09 ` Jaegeuk Kim [this message]
[not found] ` <CAC=cRTMA9AqmHjQqK3=5pAs8yqi25Rzmz8MaZ8=oTDaxHAXU+A@mail.gmail.com>
2014-09-10 7:21 ` Jaegeuk Kim
[not found] ` <CAC=cRTMGJfD_SZ=bE8pi5U6n5W1MF17emLC3VZDbpLSQtbNcKg@mail.gmail.com>
2014-09-11 5:37 ` Jaegeuk Kim
2014-09-11 10:47 ` [f2fs-dev] " Chao Yu
2014-09-11 12:31 ` Huang Ying
2014-09-11 12:25 ` Huang Ying
2014-09-12 5:13 ` Jaegeuk Kim
2014-09-12 7:34 ` Huang Ying
2014-09-13 14:23 ` Huang Ying
2014-09-14 7:48 ` Jaegeuk Kim
2014-09-15 5:14 ` Huang Ying
2014-09-18 5:47 ` Jaegeuk Kim
2014-09-14 7:38 ` Jaegeuk Kim
2014-09-15 1:32 ` Huang Ying
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140909070811.GA26657@jaegeuk-mac02.mot-mobility.com \
--to=jaegeuk@kernel.org \
--cc=cm224.lee@samsung.com \
--cc=huang.ying.caritas@gmail.com \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=ying.huang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).