Re: [PATCH] f2fs: invalidate inode and data pages if inode is no longer used

[Jaegeuk Kim <jaegeuk@kernel.org>]

On Thu, Aug 25, 2016 at 10:00:37AM -0700, Jaegeuk Kim wrote:
> On Thu, Aug 25, 2016 at 07:59:45PM +0800, Chao Yu wrote:
> > Hi Jaegeuk,
> > 
> > On 2016/8/25 10:34, Jaegeuk Kim wrote:
> > > When a file is closed, let's deactivate inode page to mitigate further memory
> > > pressure. We can do data pages as well in the corner case of f2fs_drop_inode.
> > > 
> > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > > ---
> > >  fs/f2fs/file.c  | 4 ++++
> > >  fs/f2fs/super.c | 1 +
> > >  2 files changed, 5 insertions(+)
> > > 
> > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > > index e460211..5f9a6dc 100644
> > > --- a/fs/f2fs/file.c
> > > +++ b/fs/f2fs/file.c
> > > @@ -1465,6 +1465,10 @@ static int f2fs_release_file(struct inode *inode, struct file *filp)
> > >  		filemap_fdatawrite(inode->i_mapping);
> > >  		clear_inode_flag(inode, FI_DROP_CACHE);
> > >  	}
> > > +
> > > +	/* deactivate written inode page */
> > > +	invalidate_mapping_pages(NODE_MAPPING(F2FS_I_SB(inode)),
> > > +					inode->i_ino, inode->i_ino);
> > 
> > Seems if the file was opened by two processes, one process closes this file,
> > another one will loss page cache of inode. I'm not sure whether this predication
> > is friendly to user.
> 
> No, this only calls when it's the last writer. BTW, I need to add reader case
> as well. :)

	if (!(filp->f_mode & FMODE_WRITE) ||
			atomic_read(&inode->i_writecount) != 1)
	return 0;

So, it drops its inode page only when the last writer is closed.

Thanks,

> 
> > 
> > >  	return 0;
> > >  }
> > >  
> > > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> > > index 7f863a6..f84696d 100644
> > > --- a/fs/f2fs/super.c
> > > +++ b/fs/f2fs/super.c
> > > @@ -618,6 +618,7 @@ static int f2fs_drop_inode(struct inode *inode)
> > >  			sb_end_intwrite(inode->i_sb);
> > >  
> > >  			fscrypt_put_encryption_info(inode, NULL);
> > > +			invalidate_mapping_pages(inode->i_mapping, 0, -1);
> > 
> > Hmm, shouldn't we truncate page cache before f2fs_truncate() like we do in
> > f2fs_evict?
> 
> Then, it needs to be done under i_lock, and IMO, we don't need to wait for
> writeback here.
> 
> Thanks,
> 
> > 
> > Thanks,
> > 
> > >  			spin_lock(&inode->i_lock);
> > >  			atomic_dec(&inode->i_count);
> > >  		}
> > > 
> > 
> > 
> > ------------------------------------------------------------------------------
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 
> ------------------------------------------------------------------------------
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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