[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150713232437.GA64479@jaegeuk-mac02.mot.com>
Date: Mon, 13 Jul 2015 16:25:57 -0700
From: Jaegeuk Kim <jaegeuk@...nel.org>
To: Chao Yu <yuchaochina@...mail.com>
Cc: linux-kernel@...r.kernel.org,
linux-f2fs-devel@...ts.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH] f2fs: fix to release inode page in
get_new_data_page
Hi Chao,
On Sat, Jul 11, 2015 at 10:02:51AM +0800, Chao Yu wrote:
> Hi Jaegeuk,
>
> > -----Original Message-----
> > From: Jaegeuk Kim [mailto:jaegeuk@...nel.org]
> > Sent: Saturday, July 11, 2015 8:17 AM
> > To: Chao Yu; Chao Yu
> > Cc: linux-kernel@...r.kernel.org; linux-f2fs-devel@...ts.sourceforge.net;
> > linux-kernel@...r.kernel.org; linux-f2fs-devel@...ts.sourceforge.net
> > Subject: Re: [f2fs-dev] [PATCH] f2fs: fix to release inode page in get_new_data_page
> >
> > Hi Chao,
> >
> > On Thu, Jul 09, 2015 at 06:20:08PM +0800, Chao Yu wrote:
> > > get_new_data_page should release inode page when we encounter any
> > > error in its procedure, but there is one error path didn't cover
> > > this, fix it.
> >
> > Nice catch.
> > But, I think we should fix its caller:
> >
> > in init_inode_metadata(),
> > err = make_empty_dir();
> > if (err)
> > goto put_error;
> > ---------------
>
> Previously, I fixed in the same way, but I got an oops when I test the
> patch with xfstest suit, it shows we will meet an error in this call
> path IIRC:
>
> ->f2fs_mkdir
> ->__f2fs_add_link
> ->init_inode_metadata
> ->make_empty_dir
> ->get_new_data_page
> ->f2fs_reserve_block
> ->reserve_new_block
> ->inc_valid_block_count
> return -ENOSPC;
> ->f2fs_put_dnode
> ->f2fs_put_page(ipage, 1)
>
> put_error:
> ->f2fs_put_page(ipage, 1);
> f2fs_bug_on(F2FS_P_SB(page), !PageLocked(page));
>
> And I don't think we should change error handling method of f2fs_put_dnode
> for just fixing this issue.
>
> How do you think?
Indeed. I cannot think about other clean way for now.
Instead, how about adding this description in the patch and some comments in
the codes?
Thanks,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists