[Top] [All Lists]

Re: [PATCH] Introduce SEEK_DATA/SEEK_HOLE support V8

To: jeff.liu@xxxxxxxxxx
Subject: Re: [PATCH] Introduce SEEK_DATA/SEEK_HOLE support V8
From: Mark Tinguely <tinguely@xxxxxxx>
Date: Wed, 22 Feb 2012 08:26:45 -0600
Cc: xfs@xxxxxxxxxxx, Ben Myers <bpm@xxxxxxx>, Christoph Hellwig <hch@xxxxxxxxxxxxx>, Dave Chinner <david@xxxxxxxxxxxxx>, Chris Mason <chris.mason@xxxxxxxxxx>
In-reply-to: <4F445B73.4000501@xxxxxxxxxx>
References: <4F3E532E.6000708@xxxxxxxxxx> <4F43B081.3000300@xxxxxxx> <4F445B73.4000501@xxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:9.0) Gecko/20120122 Thunderbird/9.0
On 02/21/12 21:05, Jeff Liu wrote:
On 02/21/2012 10:56 PM, Mark Tinguely wrote:

On 02/17/12 07:16, Jeff Liu wrote:

This is the revised patch according to Dave's comments for V7.

Changes to V8:
1. If there is an internal error raised at extent reading routine, just
return it rather than ENXIO.
2. Add the commit message.
3. Remove the for(;;) loop since there is no continuous holes shown even
if create a Petabyte sparse file with hole extent length longer than
32-bit.  Thanks Dave for helping verify that!
4. In xfs_seek_data(), s/len/end/, looks 'end' is more meaningful here
to indicate the range of extents mapped.
5. Remove BUG() from xfs_seek_data() since xfs_bmapi_read() have found
any corruption during the lookup, it should not occurred at all.

Any comments are appreciated!


Signed-off-by: Jie Liu<jeff.liu@xxxxxxxxxx>


+STATIC loff_t
+    fsbno = XFS_B_TO_FSBT(mp, start);
+    error = xfs_bmap_first_unused(NULL, ip, 1,&fsbno, XFS_DATA_FORK);
+    if (error)
+        goto out_unlock;
+    holeoff = XFS_FSB_TO_B(mp, fsbno);
+    if (holeoff<= start)
+        offset = start;
+    else
+        offset = min_t(loff_t, holeoff, isize);


Very Nice. Much more concise.

Can xfs_bmap_first_unused() return something larger than the end of file?

I think it could be happen if the file has no holes past the given
offset. In this case, it will return the first block past the end of
file. That is why "min_t()" is used to determine the final value.


Reviewed-by: Mark Tinguely<tinguely@xxxxxxx>

Okay. Looks good.

--Mark Tinguely.

<Prev in Thread] Current Thread [Next in Thread>