Hello, On Mon, Jan 04, 2016 at 01:57:27PM +0800, Fan Li wrote: > > > > -----Original Message----- > > From: Chao Yu [mailto:chao2...@samsung.com] > > Sent: Thursday, December 31, 2015 2:34 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 > > > > > -----Original Message----- > > > From: Fan Li [mailto:fanofcode...@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:c...@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. > > I'm not sure what you mean by avoiding, I think the comment and reply I > written has already stated the issue and > limitation of this patch. > Now there are two suggestions: > 1. support one more scenario, and all old scenarios are dealt like before, > but it still can't support discontinuous extent after > isize. > 2. support all scenarios, but sacrifice performance for lots of common > scenarios by checking about 10^9 blocks.
IMO, we can think about #2 whether there is an efficient way. How many cases does this incur? One is fallocate with keeping i_size, ana other? How about adding FADVISE_OVER_ISIZE to represent inode has blocks beyond i_size? Then, we can set this flag in fallocate and reset it in f2fs_truncate. Thanks, > > I think we can all agree both ideas have their flaws. > The only divergence is that I vote the first, and you the second. I think the > most important thing is that it works fluently in most > scenarios, and you think is that it works in every scenarios even it's very > slow. > > I think my method is more pratical, but balance between performance and > utility seems to be an Eternal problem. > > > > > > > > > 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...@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 ------------------------------------------------------------------------------ _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
