All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Hou Pengyang <houpengyang@huawei.com>
Cc: chao@kernel.org, linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [PATCH] f2fs: fix conflict between atomic_write and truncate
Date: Mon, 12 Dec 2016 17:21:09 -0800	[thread overview]
Message-ID: <20161213012109.GA8811@jaegeuk.local> (raw)
In-Reply-To: <584F4AD8.4000008@huawei.com>

Hi,

On 12/13, Hou Pengyang wrote:
> On 2016/12/13 3:18, Jaegeuk Kim wrote:
> > Hi Pengyang,
> >
> Hi, kim,
> > On 12/12, Hou Pengyang wrote:
> >> In fsync_node_pages path, after locking the last_page for setting dirty,
> >> we should check if the page has been truncated. Or there may be a mem leak,
> >> as this dirty last_page will NOT be found in next page-cache travese.
> >>
> >> This patch adds page->mapping checking, and will NOT rewrite the page if
> >> it has been truncated.
> >>
> >> Signed-off-by: Hou Pengyang <houpengyang@huawei.com>
> >> ---
> >>   fs/f2fs/node.c | 5 +++++
> >>   1 file changed, 5 insertions(+)
> >>
> >> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> >> index c1bbfdc..78c5b50 100644
> >> --- a/fs/f2fs/node.c
> >> +++ b/fs/f2fs/node.c
> >> @@ -1410,6 +1410,11 @@ int fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode,
> >>   			"Retry to write fsync mark: ino=%u, idx=%lx",
> >>   					ino, last_page->index);
> >>   		lock_page(last_page);
> >
> > We grabbed a reference count for this last_page, and thus, it won't be released.
> > Something that I missed?
> >
> Some path (e.g. shrink) would NOT release the page if we've grabbed it,
> But truncate operation will invalidate the page from page-cache
> regardless of the page reference.

This is a node page, and we guarantee that nobody will truncate them.
Did you hit an issue regarding to this?

Thanks,

> Patch: 0fac558b965 f2fs: make atomic/volatile operation exclusive
> seems fix this issue by adding inode_lock(inode) to avoid conflict
> between atomic/volatile and truncate operation.
> 
> But this patch makes codes more robust.
> 
> Thanks.
> 
> > Thanks,
> >
> >> +		if (unlikely(last_page->mapping != NODE_MAPPING(sbi))) {
> >> +			unlock_page(last_page);
> >> +			f2fs_put_page(last_page, 0);
> >> +			goto out;
> >> +		}
> >>   		f2fs_wait_on_page_writeback(last_page, NODE, true);
> >>   		set_page_dirty(last_page);
> >>   		unlock_page(last_page);
> >> --
> >> 2.10.1
> >>
> >>
> >> ------------------------------------------------------------------------------
> >> Developer Access Program for Intel Xeon Phi Processors
> >> Access to Intel Xeon Phi processor-based developer platforms.
> >> With one year of Intel Parallel Studio XE.
> >> Training and support from Colfax.
> >> Order your platform today.http://sdm.link/xeonphi
> >> _______________________________________________
> >> Linux-f2fs-devel mailing list
> >> Linux-f2fs-devel@lists.sourceforge.net
> >> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> >
> > .
> >
> 
> 
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most 
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

  reply	other threads:[~2016-12-13  1:21 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-12 13:16 [PATCH] f2fs: fix conflict between atomic_write and truncate Hou Pengyang
2016-12-12 19:18 ` Jaegeuk Kim
2016-12-13  1:11   ` Hou Pengyang
2016-12-13  1:21     ` Jaegeuk Kim [this message]
2016-12-13  1:34       ` Hou Pengyang

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=20161213012109.GA8811@jaegeuk.local \
    --to=jaegeuk@kernel.org \
    --cc=chao@kernel.org \
    --cc=houpengyang@huawei.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.