diff mbox

[1/2] xfs: Fix missed holes in SEEK_HOLE implementation

Message ID 20170511165023.29887-1-jack@suse.cz (mailing list archive)
State Superseded
Headers show

Commit Message

Jan Kara May 11, 2017, 4:50 p.m. UTC
XFS SEEK_HOLE implementation could miss a hole in an unwritten extent as
can be seen by the following command:

xfs_io -c "falloc 0 256k" -c "pwrite 0 56k" -c "pwrite 128k 8k"
       -c "seek -h 0" file
wrote 57344/57344 bytes at offset 0
56 KiB, 14 ops; 0.0000 sec (49.312 MiB/sec and 12623.9856 ops/sec)
wrote 8192/8192 bytes at offset 131072
8 KiB, 2 ops; 0.0000 sec (70.383 MiB/sec and 18018.0180 ops/sec)
Whence	Result
HOLE	139264

Where we can see that hole at offset 56k was just ignored by SEEK_HOLE
implementation. The bug is in xfs_find_get_desired_pgoff() which does
not properly detect the case when a first page in the pagevec has larger
index than expected (and even if the condition was right, we would fail
to update the returned offset).

Fix the problem by properly detecting when found page has larger offset
than expected.

CC: stable@vger.kernel.org
Fixes: d126d43f631f996daeee5006714fed914be32368
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/xfs/xfs_file.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comments

Darrick J. Wong May 12, 2017, 4:23 p.m. UTC | #1
On Thu, May 11, 2017 at 06:50:22PM +0200, Jan Kara wrote:
> XFS SEEK_HOLE implementation could miss a hole in an unwritten extent as
> can be seen by the following command:
> 
> xfs_io -c "falloc 0 256k" -c "pwrite 0 56k" -c "pwrite 128k 8k"
>        -c "seek -h 0" file
> wrote 57344/57344 bytes at offset 0
> 56 KiB, 14 ops; 0.0000 sec (49.312 MiB/sec and 12623.9856 ops/sec)
> wrote 8192/8192 bytes at offset 131072
> 8 KiB, 2 ops; 0.0000 sec (70.383 MiB/sec and 18018.0180 ops/sec)
> Whence	Result
> HOLE	139264
> 
> Where we can see that hole at offset 56k was just ignored by SEEK_HOLE
> implementation. The bug is in xfs_find_get_desired_pgoff() which does
> not properly detect the case when a first page in the pagevec has larger
> index than expected (and even if the condition was right, we would fail
> to update the returned offset).
> 
> Fix the problem by properly detecting when found page has larger offset
> than expected.
> 
> CC: stable@vger.kernel.org
> Fixes: d126d43f631f996daeee5006714fed914be32368
> Signed-off-by: Jan Kara <jack@suse.cz>

Looks ok, will test...
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/xfs_file.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
> index 35703a801372..df51c025adfe 100644
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -1077,13 +1077,12 @@ xfs_find_get_desired_pgoff(
>  		}
>  
>  		/*
> -		 * At lease we found one page.  If this is the first time we
> -		 * step into the loop, and if the first page index offset is
> -		 * greater than the given search offset, a hole was found.
> +		 * At least we found one page. If the current offset is smaller
> +		 * than the first page offset, a hole was found.
>  		 */
> -		if (type == HOLE_OFF && lastoff == startoff &&
> -		    lastoff < page_offset(pvec.pages[0])) {
> +		if (type == HOLE_OFF && lastoff < page_offset(pvec.pages[0])) {
>  			found = true;
> +			*offset = lastoff;
>  			break;
>  		}
>  
> -- 
> 2.12.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 35703a801372..df51c025adfe 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1077,13 +1077,12 @@  xfs_find_get_desired_pgoff(
 		}
 
 		/*
-		 * At lease we found one page.  If this is the first time we
-		 * step into the loop, and if the first page index offset is
-		 * greater than the given search offset, a hole was found.
+		 * At least we found one page. If the current offset is smaller
+		 * than the first page offset, a hole was found.
 		 */
-		if (type == HOLE_OFF && lastoff == startoff &&
-		    lastoff < page_offset(pvec.pages[0])) {
+		if (type == HOLE_OFF && lastoff < page_offset(pvec.pages[0])) {
 			found = true;
+			*offset = lastoff;
 			break;
 		}