* [PATCH 1/3] f2fs: avoid calling i_size_read when it's unnecessary @ 2015-12-30 8:49 Fan Li 2015-12-30 18:27 ` Jaegeuk Kim 0 siblings, 1 reply; 5+ messages in thread From: Fan Li @ 2015-12-30 8:49 UTC (permalink / raw) To: 'Jaegeuk Kim'; +Cc: linux-f2fs-devel 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); + mutex_lock(&inode->i_mutex); if (start >= isize) goto out; -- 1.7.9.5 ------------------------------------------------------------------------------ ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] f2fs: avoid calling i_size_read when it's unnecessary 2015-12-30 8:49 [PATCH 1/3] f2fs: avoid calling i_size_read when it's unnecessary Fan Li @ 2015-12-30 18:27 ` Jaegeuk Kim 2015-12-31 2:53 ` Fan Li 0 siblings, 1 reply; 5+ messages in thread From: Jaegeuk Kim @ 2015-12-30 18:27 UTC (permalink / raw) To: Fan Li; +Cc: linux-f2fs-devel 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. Thanks, > + > mutex_lock(&inode->i_mutex); > if (start >= isize) > goto out; > -- > 1.7.9.5 > ------------------------------------------------------------------------------ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] f2fs: avoid calling i_size_read when it's unnecessary 2015-12-30 18:27 ` Jaegeuk Kim @ 2015-12-31 2:53 ` Fan Li 2016-01-01 3:07 ` Jaegeuk Kim 0 siblings, 1 reply; 5+ messages in thread From: Fan Li @ 2015-12-31 2:53 UTC (permalink / raw) To: 'Jaegeuk Kim'; +Cc: linux-f2fs-devel > -----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 > > ------------------------------------------------------------------------------ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] f2fs: avoid calling i_size_read when it's unnecessary 2015-12-31 2:53 ` Fan Li @ 2016-01-01 3:07 ` Jaegeuk Kim 2016-01-04 6:13 ` Fan Li 0 siblings, 1 reply; 5+ messages in thread From: Jaegeuk Kim @ 2016-01-01 3:07 UTC (permalink / raw) To: Fan Li; +Cc: linux-f2fs-devel Hi Fan, On Thu, Dec 31, 2015 at 10:53:32AM +0800, Fan Li wrote: > > > > -----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? Right, but I prefer to use i_size_read consistently. TOH, I'm not convincing why you're treating this as a so costly operation. Thanks, > > > > > Thanks, > > > > > + > > > mutex_lock(&inode->i_mutex); > > > if (start >= isize) > > > goto out; > > > -- > > > 1.7.9.5 > > > ------------------------------------------------------------------------------ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] f2fs: avoid calling i_size_read when it's unnecessary 2016-01-01 3:07 ` Jaegeuk Kim @ 2016-01-04 6:13 ` Fan Li 0 siblings, 0 replies; 5+ messages in thread From: Fan Li @ 2016-01-04 6:13 UTC (permalink / raw) To: 'Jaegeuk Kim'; +Cc: linux-f2fs-devel > -----Original Message----- > From: Jaegeuk Kim [mailto:jaegeuk@kernel.org] > Sent: Friday, January 01, 2016 11:08 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 Thu, Dec 31, 2015 at 10:53:32AM +0800, Fan Li wrote: > > > > > > > -----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? > > Right, but I prefer to use i_size_read consistently. > TOH, I'm not convincing why you're treating this as a so costly operation. Yes, the difference between i_size_read and assignment is very small. I just think it's best to squeeze every bit of performance out of codes. If the consistence is a more pressing issue, then i_size_read it is. > > Thanks, > > > > > > > > > Thanks, > > > > > > > + > > > > mutex_lock(&inode->i_mutex); > > > > if (start >= isize) > > > > goto out; > > > > -- > > > > 1.7.9.5 > > > > ------------------------------------------------------------------------------ ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-01-04 6:13 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-12-30 8:49 [PATCH 1/3] f2fs: avoid calling i_size_read when it's unnecessary Fan Li 2015-12-30 18:27 ` Jaegeuk Kim 2015-12-31 2:53 ` Fan Li 2016-01-01 3:07 ` Jaegeuk Kim 2016-01-04 6:13 ` Fan Li
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).