linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
From: Jaegeuk Kim <jaegeuk@kernel.org>
To: heyunlei <heyunlei@huawei.com>
Cc: heyunlei@huwei.com, linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [PATCH] f2fs: walk around a panic caused by nrpages is not zero in clear_inode
Date: Fri, 9 Sep 2016 18:27:49 -0700	[thread overview]
Message-ID: <20160910012749.GA33194@jaegeuk> (raw)
In-Reply-To: <28bd8111-d4ec-4340-3909-d0bcd3886c90@huawei.com>

Hi Yunlei,

I just finished to understand how your 4.1 looks like.
Could you please merge the below patches in terms of roll-forward recovery?
When I tested with the closest 4.1 that I can make, I could see several bugs
specially in the error handling paths.

1. f2fs: factor out fsync inode entry operations
2. f2fs: fix inode cache leak
3. f2fs: report error for f2fs_parent_dir
4. f2fs: fix to report error number of f2fs_find_entry
5. f2fs: propagate error given by f2fs_find_entry
6. f2fs: fix to do security initialization of encrypted inode with original filename
7. f2fs: add roll-forward recovery process for encrypted dentry

I'll post the below patches soon.

8. f2fs: add common iget in add_fsync_inode
9. f2fs: avoid ENOMEM during roll-forward recovery

Thanks,

On Wed, Sep 07, 2016 at 10:14:56AM +0800, heyunlei wrote:
> 
> Hi, kim
> On 2016/9/7 9:47, Jaegeuk Kim wrote:
> > On Tue, Sep 06, 2016 at 09:17:32AM +0800, Yunlei He wrote:
> > > I came across a panic twice:
> > > 
> > > [<ffffffc000088e70>] dump_backtrace+0x0/0x124
> > > [<ffffffc000088fb4>] show_stack+0x14/0x1c
> > > [<ffffffc000ce1c38>] dump_stack+0x20/0x28
> > > [<ffffffc000cdfbd4>] panic+0x13c/0x258
> > > [<ffffffc0001b2244>] clear_inode+0x8c/0xd4
> > > [<ffffffc00030575c>] f2fs_evict_inode+0x194/0x3e0
> > > [<ffffffc0001b30b4>] evict+0xa0/0x1cc
> > > [<ffffffc0001b399c>] iput+0xe4/0x180
> > > [<ffffffc0003274fc>] recover_fsync_data+0x6ec/0xe5c
> > > [<ffffffc00030d420>] f2fs_fill_super+0xa5c/0xb9c
> > > [<ffffffc00019cd00>] mount_bdev+0x1ac/0x1d4
> > > [<ffffffc00030b66c>] f2fs_mount+0x14/0x1c
> > > [<ffffffc00019d5e4>] mount_fs+0x3c/0x1bc
> > > [<ffffffc0001b7560>] vfs_kern_mount+0x4c/0xf0
> > > [<ffffffc0001b9970>] do_mount+0x218/0x960
> > > [<ffffffc0001ba148>] SyS_mount+0x90/0xd0
> > 
> > Hmm, could you share the previous recovery messages?
> 
> Here is the previous recovery message:
> 
> <5>[    8.929199s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_inode: ino = 4e8, name = packages.xml
> <5>[    8.930267s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_dentry: ino = 4e8, name = packages.xml, dir = 8d, err = 0
> <5>[    8.930572s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 4e8, recovered = 85 blocks, err = 0
> <5>[    8.930603s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_inode: ino = 4e9, name = packages.list.tmp
> <5>[    8.930603s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_dentry: ino = 4e9, name = packages.list.tmp, dir = 8d, err = 0
> <5>[    8.930633s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 4e9, recovered = 2 blocks, err = 0
> <5>[    8.930664s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_inode: ino = 4ea, name = package-restrictions.xml
> <5>[    8.931243s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_dentry: ino = 4ea, name = package-restrictions.xml, dir = 1a3, err = 0
> <5>[    8.931243s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 4ea, recovered = 0 blocks, err = 0
> <5>[    8.931274s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 23e, recovered = 0 blocks, err = 0
> <5>[    8.931274s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_inode: ino = 23e, name = entropy.dat
> <5>[    8.931274s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 23e, recovered = 0 blocks, err = 0
> <5>[    8.931304s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_inode: ino = 4eb, name = accounts.xml
> <5>[    8.931884s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_dentry: ino = 4eb, name = accounts.xml, dir = 248, err = 0
> <5>[    8.931915s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 4eb, recovered = 0 blocks, err = 0
> <5>[    8.931915s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_inode: ino = 4ec, name = status.bin
> <5>[    8.932098s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_dentry: ino = 4ec, name = status.bin, dir = 248, err = 0
> <5>[    8.932128s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 4ec, recovered = 0 blocks, err = 0
> <5>[    8.932128s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_inode: ino = 24b, name = pending.xml
> <5>[    8.932159s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 24b, recovered = 0 blocks, err = 0
> <5>[    8.932159s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_inode: ino = 4ed, name = stats.bin
> <5>[    8.932342s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_dentry: ino = 4ed, name = stats.bin, dir = 248, err = 0
> <5>[    8.932373s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 4ed, recovered = 0 blocks, err = 0
> <5>[    8.932373s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_inode: ino = 4ee, name = runtime-permissions.xml
> <5>[    8.932556s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_dentry: ino = 4ee, name = runtime-permissions.xml, dir = 1a3, err = 0
> <5>[    8.932586s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 4ee, recovered = 3 blocks, err = 0
> <5>[    8.932617s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_inode: ino = 4ef, name = .temp.0TNPDd
> <5>[    8.932922s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_dentry: ino = 4ef, name = .temp.0TNPDd, dir = 7e, err = 0
> <5>[    8.932952s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 4ef, recovered = 0 blocks, err = 0
> <5>[    8.932952s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_inode: ino = 4f0, name = packages.xml
> <5>[    8.933013s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_dentry: ino = 4f0, name = packages.xml, dir = 8d, err = 0
> <5>[    8.933105s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 4f0, recovered = 85 blocks, err = 0
> <5>[    8.933135s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_inode: ino = 4f2, name = packages.list.tmp
> <5>[    8.933166s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_dentry: ino = 4f2, name = packages.list.tmp, dir = 8d, err = 0
> <5>[    8.933197s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 4f2, recovered = 2 blocks, err = 0
> <5>[    8.933197s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_inode: ino = 4f4, name = package-restrictions.xml
> <5>[    8.933227s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_dentry: ino = 4f4, name = package-restrictions.xml, dir = 1a3, err = 0
> <5>[    8.933258s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 4f4, recovered = 0 blocks, err = 0
> <5>[    8.933258s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_inode: ino = 4f8, name = runtime-permissions.xml
> <5>[    8.933288s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_dentry: ino = 4f8, name = runtime-permissions.xml, dir = 1a3, err = 0
> <5>[    8.933319s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 4f8, recovered = 3 blocks, err = 0
> <5>[    8.933349s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_inode: ino = 4f9, name = .temp.t7N4AT
> <5>[    8.933349s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_dentry: ino = 4f9, name = .temp.t7N4AT, dir = 7e, err = 0
> <5>[    8.933380s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 4f9, recovered = 0 blocks, err = 0
> <5>[    8.933715s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_dentry: ino = 4fa, name = journal-1732203582.tmp, dir = 26c, err = 0
> <5>[    8.933715s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 4fa, recovered = 0 blocks, err = 0
> <5>[    8.933746s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 4fa, recovered = 0 blocks, err = 0
> <5>[    8.933746s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 4fa, recovered = 0 blocks, err = 0
> <5>[    8.933746s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_inode: ino = 4fa, name = journal-1732203582.tmp
> <5>[    8.933746s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 4fa, recovered = 0 blocks, err = 0
> <5>[    8.933776s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_inode: ino = 4fd, name = .temp.rIQKTq
> <5>[    8.933776s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_dentry: ino = 4fd, name = .temp.rIQKTq, dir = 7e, err = 0
> <5>[    8.933807s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 4fd, recovered = 0 blocks, err = 0
> <5>[    8.933807s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_inode: ino = 4fe, name = com.android.keyguard_preferences.xml
> <5>[    8.934295s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_dentry: ino = 4fe, name = com.android.keyguard_preferences.xml, dir = 2a1, err = 0
> <5>[    8.934326s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 4fe, recovered = 0 blocks, err = 0
> <5>[    8.934326s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_inode: ino = 4ff, name = com.android.keyguard_preferences.xml
> <5>[    8.934356s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_dentry: ino = 4ff, name = com.android.keyguard_preferences.xml, dir = 2a1, err = 0
> <5>[    8.934387s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_data: ino = 4ff, recovered = 0 blocks, err = 0
> <5>[    8.934387s][pid:263,cpu4,init]F2FS-fs (mmcblk0p42): recover_inode: ino = 500, name = com.android.providers.media_preferences.xml
> <4>[    8.934844s][pid:263,cpu4,init]BUG: failure at /fs/inode.c:511/clear_inode()!
> 
> one file named com.android.keyguard_preferences.xml enter recover_dentry twice with different ino(4fe vs 4ff).
> Maybe this error happened when deleted entry previous recovered.
> 
> Thanks.
> 
> 
> > In the truncation part, f2fs_evict_inode starts with setting i_size to zero.
> > So, the only possible way would be caused by enabled cache_only given by an
> > inline_data case. But the assumption is that inode must have more than 2
> > i_blocks, which is not normal case though.
> > 
> > Can we gather more information to narrow down the root cause?
> > 
> > BTW, I found that the below grab_cache_page() should be find_lock_page() to
> > check its cached page only.
> > 
> > > From a584542d25e7dfc91af53a8f4a4866b939d29fef Mon Sep 17 00:00:00 2001
> > From: Jaegeuk Kim <jaegeuk@kernel.org>
> > Date: Tue, 6 Sep 2016 15:55:54 -0700
> > Subject: [PATCH] f2fs: avoid page allocation for truncating partial
> >  inline_data
> > 
> > When truncating cached inline_data, we don't need to allocate a new page
> > all the time. Instead, it must check its page cache only.
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >  fs/f2fs/file.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index a8aa6fd..0144ed4 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -562,7 +562,7 @@ static int truncate_partial_data_page(struct inode *inode, u64 from,
> >  		return 0;
> > 
> >  	if (cache_only) {
> > -		page = f2fs_grab_cache_page(mapping, index, false);
> > +		page = find_lock_page(mapping, index);
> >  		if (page && PageUptodate(page))
> >  			goto truncate_out;
> >  		f2fs_put_page(page, 1);
> > 

------------------------------------------------------------------------------

      reply	other threads:[~2016-09-10  1:28 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-06  1:17 [PATCH] f2fs: walk around a panic caused by nrpages is not zero in clear_inode Yunlei He
2016-09-07  1:47 ` Jaegeuk Kim
2016-09-07  2:14   ` heyunlei
2016-09-10  1:27     ` Jaegeuk Kim [this message]

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=20160910012749.GA33194@jaegeuk \
    --to=jaegeuk@kernel.org \
    --cc=heyunlei@huawei.com \
    --cc=heyunlei@huwei.com \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    /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).