Re: [PATCH 2/3] f2fs: support finding extents after isize

[Chao Yu <chao2.yu@samsung.com>]

> -----Original Message-----
> From: Fan Li [mailto:fanofcode.li@samsung.com]
> Sent: Thursday, December 31, 2015 11:37 AM
> To: 'Chao Yu'; 'Jaegeuk Kim'
> Cc: linux-f2fs-devel@lists.sourceforge.net
> Subject: Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents after isize
> 
> 
> 
> > -----Original Message-----
> > From: Chao Yu [mailto:chao@kernel.org]
> > Sent: Wednesday, December 30, 2015 9:28 PM
> > To: Fan Li; 'Jaegeuk Kim'
> > Cc: linux-f2fs-devel@lists.sourceforge.net
> > Subject: Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents after isize
> >
> > Hi,
> >
> > On 12/30/15 5:17 PM, Fan Li wrote:
> > > f2fs allows preallocation beyond isize, but f2fs_fiemap only look up
> > > extents within isize. Therefore add this support.
> > >
> > > Note: It's possible that there are holes after isize, for example,
> > > fallocate  multiple discontinuous extents after isize with
> > > FALLOC_FL_KEEP_SIZE set. Since I can tell no differences between EOF
> > > and holes from return of get_data_block, I'm afaid this patch can't
> > > support such scenarios.
> >
> > As you mentioned, preallocated block beyond isize can be allocated in f2fs, and we are trying
> to support mapping extents across
> > whole data space of inode, so why we treat theses extents inside i_size and outside i_size
> separately? IMO, instead using i_size,
> we
> > should use max blocks as boundary.
> >
> > Most important, this interface still can't support finding all extents after i_size, which
> looks buggy for our user.
> 
> Notice that this issue exists before my patch, by adding this patch, at least now it can support
> more scenarios
> such as  fallocate a range right after isize. I'd say it's an improvement.

Nope, what I'm talking about is *correctness* of our ->fiemap interface,
but you're trying to avoid it by saying "support more cases, it's an
improvement". That doesn't make any sense to me, since correctness issue
still not be fixed.

> 
> use max blocks as boundary would get us the precise result, but it also means after we reach
> the EOF, we still
> need to look up every block between the EOF and  sb-> s_maxbytes to make sure the EOF is true,
> that's
> about 4TB or 10^9 blocks.
> And it affects all scenarios where the search range covers the last extent in the file, not
> just extents beyond
> isize. I think this price is too high to pay.

That's another performance issue.

> 
> I was hoping that I can make f2fs_map_block return an EOF to solve this problem some time later,
> or anyone
> have a better idea?

At least we can seek valid dnode block like the way llseek use.

In addition, for most cases, few of i_nid[5] in f2fs_inode will be
NULL, we could skip searching all dnode block in such non-allocated
indirect node, instead of searching dnode block f2fs_map_block one
by one.

Thanks,

> 
> >
> > Thanks,
> >
> > >
> > >
> > > Signed-off-by: Fan li <fanofcode.li@samsung.com>
> > > ---
> > >  fs/f2fs/data.c |    7 +------
> > >  1 file changed, 1 insertion(+), 6 deletions(-)
> > >
> > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index a9a4d89..f89cf07
> > > 100644
> > > --- a/fs/f2fs/data.c
> > > +++ b/fs/f2fs/data.c
> > > @@ -798,12 +798,6 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
> > >  	isize = i_size_read(inode);
> > >
> > >  	mutex_lock(&inode->i_mutex);
> > > -	if (start >= isize)
> > > -		goto out;
> > > -
> > > -	if (start + len > isize)
> > > -		len = isize - start;
> > > -
> > >  	if (logical_to_blk(inode, len) == 0)
> > >  		len = blk_to_logical(inode, 1);
> > >
> > > @@ -829,6 +823,7 @@ next:
> > >  		 * punch holes beyond isize and keep size unchanged.
> > >  		 */
> > >  		flags |= FIEMAP_EXTENT_LAST;
> > > +		last_blk = start_blk - 1;
> > >  	}
> > >
> > >  	if (size)
> > >
> 
> 
> ------------------------------------------------------------------------------
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


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