From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chao Yu Subject: Re: [PATCH 2/3] f2fs: support finding extents after isize Date: Tue, 05 Jan 2016 17:36:40 +0800 Message-ID: <00a301d1479c$accb9b30$0662d190$@samsung.com> References: <000d01d142e2$fab05fc0$f0111f40$@samsung.com> <5683DBD5.1050509@kernel.org> <001901d1437c$9b4b15d0$d1e14170$@samsung.com> <011501d14395$6127a030$2376e090$@samsung.com> <002d01d146b4$e27808b0$a7681a10$@samsung.com> <20160104065544.GB18606@jaegeuk-2.local> <009801d14767$fba23410$f2e69c30$@samsung.com> <20160105032953.GA863@jaegeuk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from sog-mx-2.v43.ch3.sourceforge.com ([172.29.43.192] helo=mx.sourceforge.net) by sfs-ml-1.v29.ch3.sourceforge.com with esmtp (Exim 4.76) (envelope-from ) id 1aGO3U-0001My-W4 for linux-f2fs-devel@lists.sourceforge.net; Tue, 05 Jan 2016 09:37:29 +0000 Received: from mailout4.samsung.com ([203.254.224.34]) by sog-mx-2.v43.ch3.sourceforge.com with esmtps (TLSv1:AES128-SHA:128) (Exim 4.76) id 1aGO3T-0007QO-I0 for linux-f2fs-devel@lists.sourceforge.net; Tue, 05 Jan 2016 09:37:28 +0000 Received: from epcpsbgm1new.samsung.com (epcpsbgm1 [203.254.230.26]) by mailout4.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0O0H02C934267C30@mailout4.samsung.com> for linux-f2fs-devel@lists.sourceforge.net; Tue, 05 Jan 2016 18:37:20 +0900 (KST) In-reply-to: <20160105032953.GA863@jaegeuk> Content-language: zh-cn List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-f2fs-devel-bounces@lists.sourceforge.net To: 'Jaegeuk Kim' Cc: linux-f2fs-devel@lists.sourceforge.net > -----Original Message----- > From: Jaegeuk Kim [mailto:jaegeuk@kernel.org] > Sent: Tuesday, January 05, 2016 11:30 AM > To: Chao Yu > Cc: 'Fan Li'; linux-f2fs-devel@lists.sourceforge.net > Subject: Re: [f2fs-dev] [PATCH 2/3] f2fs: support finding extents after isize > > Hi, > > ... > > > > > > 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? > > > > AFAIK, no more. > > > > > > > > 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. > > > > Append write in such inode could destroy this convention, right? > > Right, but we have another chance to reset the flag, when fiemap checks the end > of allocated space. Agreed, :) Thanks, > > Thanks, > > > > > Thanks, > > > > > > > > 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 > > > > > > > > --- > > > > > > > > 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 ------------------------------------------------------------------------------