Re: [PATCH 1/3] f2fs: avoid calling i_size_read when it's unnecessary

[Fan Li <fanofcode.li@samsung.com>]

> -----Original Message-----
> From: Jaegeuk Kim [mailto:jaegeuk@kernel.org]
> Sent: Thursday, December 31, 2015 2:28 AM
> To: Fan Li
> Cc: linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH 1/3] f2fs: avoid calling i_size_read when it's unnecessary
> 
> Hi Fan,
> 
> On Wed, Dec 30, 2015 at 04:49:56PM +0800, Fan Li wrote:
> > i_size_read does more than reading a value, it's best that we use it
> > only when we need it.
> >
> > Signed-off-by: Fan li <fanofcode.li@samsung.com>
> > ---
> >  fs/f2fs/data.c |    4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index d67c599..a9a4d89
> > 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -780,7 +780,7 @@ int f2fs_fiemap(struct inode *inode, struct
> > fiemap_extent_info *fieinfo,  {
> >  	struct buffer_head map_bh;
> >  	sector_t start_blk, last_blk;
> > -	loff_t isize = i_size_read(inode);
> > +	loff_t isize;
> >  	u64 logical = 0, phys = 0, size = 0;
> >  	u32 flags = 0;
> >  	int ret = 0;
> > @@ -795,6 +795,8 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> >  			return ret;
> >  	}
> >
> > +	isize = i_size_read(inode);
> 
> It seems that we need to get isize after grabbing i_mutex below in order to avoid data race.

See if I got this right, isize should remain unchanged during the entire procedure, so we need
to add i_mutex upon it, and since there is already a i_mutex,  we can get inode->i_size
directly instead of calling i_read_size.
Is that right?

> 
> Thanks,
> 
> > +
> >  	mutex_lock(&inode->i_mutex);
> >  	if (start >= isize)
> >  		goto out;
> > --
> > 1.7.9.5
> >


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