* [PATCH 0/3 v2] xfs: Fix SEEK_HOLE implementation
@ 2017-05-17 12:10 Jan Kara
2017-05-17 12:10 ` Jan Kara
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Jan Kara @ 2017-05-17 12:10 UTC (permalink / raw)
To: Darrick J . Wong; +Cc: linux-xfs, Brian Foster, Jan Kara
Hello,
this is the second revision of the patches to fix bugs in XFS's SEEK_HOLE
implementation and cleanup the code a bit.
Changes since v1:
* Fixed some more buggy cases
* Simplified code a bit as suggested by Darrick
* Fixed range check as spotted by Brian
Honza
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/3] xfs: Fix missed holes in SEEK_HOLE implementation
2017-05-17 12:10 [PATCH 0/3 v2] xfs: Fix SEEK_HOLE implementation Jan Kara
@ 2017-05-17 12:10 ` Jan Kara
2017-05-17 12:10 ` [PATCH 2/3] xfs: Fix off-by-in in loop termination in xfs_find_get_desired_pgoff() Jan Kara
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Jan Kara @ 2017-05-17 12:10 UTC (permalink / raw)
To: Darrick J . Wong; +Cc: linux-xfs, Brian Foster, Jan Kara, stable
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 pages are not contiguous.
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 | 29 +++++++++--------------------
1 file changed, 9 insertions(+), 20 deletions(-)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 35703a801372..f371812e20c6 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1076,17 +1076,6 @@ xfs_find_get_desired_pgoff(
break;
}
- /*
- * 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.
- */
- if (type == HOLE_OFF && lastoff == startoff &&
- lastoff < page_offset(pvec.pages[0])) {
- found = true;
- break;
- }
-
for (i = 0; i < nr_pages; i++) {
struct page *page = pvec.pages[i];
loff_t b_offset;
@@ -1098,18 +1087,18 @@ xfs_find_get_desired_pgoff(
* file mapping. However, page->index will not change
* because we have a reference on the page.
*
- * Searching done if the page index is out of range.
- * If the current offset is not reaches the end of
- * the specified search range, there should be a hole
- * between them.
+ * If current page offset is beyond where we've ended,
+ * we've found a hole.
*/
- if (page->index > end) {
- if (type == HOLE_OFF && lastoff < endoff) {
- *offset = lastoff;
- found = true;
- }
+ if (type == HOLE_OFF && lastoff < endoff &&
+ lastoff < page_offset(pvec.pages[i])) {
+ found = true;
+ *offset = lastoff;
goto out;
}
+ /* Searching done if the page index is out of range. */
+ if (page->index > end)
+ goto out;
lock_page(page);
/*
--
2.12.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 1/3] xfs: Fix missed holes in SEEK_HOLE implementation
@ 2017-05-17 12:10 ` Jan Kara
0 siblings, 0 replies; 10+ messages in thread
From: Jan Kara @ 2017-05-17 12:10 UTC (permalink / raw)
To: Darrick J . Wong; +Cc: linux-xfs, Brian Foster, Jan Kara, stable
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 pages are not contiguous.
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 | 29 +++++++++--------------------
1 file changed, 9 insertions(+), 20 deletions(-)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 35703a801372..f371812e20c6 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1076,17 +1076,6 @@ xfs_find_get_desired_pgoff(
break;
}
- /*
- * 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.
- */
- if (type == HOLE_OFF && lastoff == startoff &&
- lastoff < page_offset(pvec.pages[0])) {
- found = true;
- break;
- }
-
for (i = 0; i < nr_pages; i++) {
struct page *page = pvec.pages[i];
loff_t b_offset;
@@ -1098,18 +1087,18 @@ xfs_find_get_desired_pgoff(
* file mapping. However, page->index will not change
* because we have a reference on the page.
*
- * Searching done if the page index is out of range.
- * If the current offset is not reaches the end of
- * the specified search range, there should be a hole
- * between them.
+ * If current page offset is beyond where we've ended,
+ * we've found a hole.
*/
- if (page->index > end) {
- if (type == HOLE_OFF && lastoff < endoff) {
- *offset = lastoff;
- found = true;
- }
+ if (type == HOLE_OFF && lastoff < endoff &&
+ lastoff < page_offset(pvec.pages[i])) {
+ found = true;
+ *offset = lastoff;
goto out;
}
+ /* Searching done if the page index is out of range. */
+ if (page->index > end)
+ goto out;
lock_page(page);
/*
--
2.12.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] xfs: Fix off-by-in in loop termination in xfs_find_get_desired_pgoff()
2017-05-17 12:10 [PATCH 0/3 v2] xfs: Fix SEEK_HOLE implementation Jan Kara
2017-05-17 12:10 ` Jan Kara
@ 2017-05-17 12:10 ` Jan Kara
2017-05-17 12:10 ` [PATCH 3/3] xfs: Move handling of missing page into one place " Jan Kara
2017-05-17 12:31 ` [PATCH 0/3 v2] xfs: Fix SEEK_HOLE implementation Eryu Guan
3 siblings, 0 replies; 10+ messages in thread
From: Jan Kara @ 2017-05-17 12:10 UTC (permalink / raw)
To: Darrick J . Wong; +Cc: linux-xfs, Brian Foster, Jan Kara
There is an off-by-one error in loop termination conditions in
xfs_find_get_desired_pgoff(). It doesn't have any visible effects but
still it is good to fix it.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/xfs/xfs_file.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index f371812e20c6..8acb8ab79267 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1097,7 +1097,7 @@ xfs_find_get_desired_pgoff(
goto out;
}
/* Searching done if the page index is out of range. */
- if (page->index > end)
+ if (page->index >= end)
goto out;
lock_page(page);
@@ -1153,7 +1153,7 @@ xfs_find_get_desired_pgoff(
index = pvec.pages[i - 1]->index + 1;
pagevec_release(&pvec);
- } while (index <= end);
+ } while (index < end);
out:
pagevec_release(&pvec);
--
2.12.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] xfs: Move handling of missing page into one place in xfs_find_get_desired_pgoff()
2017-05-17 12:10 [PATCH 0/3 v2] xfs: Fix SEEK_HOLE implementation Jan Kara
2017-05-17 12:10 ` Jan Kara
2017-05-17 12:10 ` [PATCH 2/3] xfs: Fix off-by-in in loop termination in xfs_find_get_desired_pgoff() Jan Kara
@ 2017-05-17 12:10 ` Jan Kara
2017-05-17 12:31 ` [PATCH 0/3 v2] xfs: Fix SEEK_HOLE implementation Eryu Guan
3 siblings, 0 replies; 10+ messages in thread
From: Jan Kara @ 2017-05-17 12:10 UTC (permalink / raw)
To: Darrick J . Wong; +Cc: linux-xfs, Brian Foster, Jan Kara
Currently several places in xfs_find_get_desired_pgoff() handle the case
of a missing page. Make them all handled in one place after the loop has
terminated.
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/xfs/xfs_file.c | 38 ++++++++------------------------------
1 file changed, 8 insertions(+), 30 deletions(-)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 8acb8ab79267..ff599ab45e2e 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -1052,29 +1052,8 @@ xfs_find_get_desired_pgoff(
want = min_t(pgoff_t, end - index, PAGEVEC_SIZE);
nr_pages = pagevec_lookup(&pvec, inode->i_mapping, index,
want);
- /*
- * No page mapped into given range. If we are searching holes
- * and if this is the first time we got into the loop, it means
- * that the given offset is landed in a hole, return it.
- *
- * If we have already stepped through some block buffers to find
- * holes but they all contains data. In this case, the last
- * offset is already updated and pointed to the end of the last
- * mapped page, if it does not reach the endpoint to search,
- * that means there should be a hole between them.
- */
- if (nr_pages == 0) {
- /* Data search found nothing */
- if (type == DATA_OFF)
- break;
-
- ASSERT(type == HOLE_OFF);
- if (lastoff == startoff || lastoff < endoff) {
- found = true;
- *offset = lastoff;
- }
+ if (nr_pages == 0)
break;
- }
for (i = 0; i < nr_pages; i++) {
struct page *page = pvec.pages[i];
@@ -1140,21 +1119,20 @@ xfs_find_get_desired_pgoff(
/*
* The number of returned pages less than our desired, search
- * done. In this case, nothing was found for searching data,
- * but we found a hole behind the last offset.
+ * done.
*/
- if (nr_pages < want) {
- if (type == HOLE_OFF) {
- *offset = lastoff;
- found = true;
- }
+ if (nr_pages < want)
break;
- }
index = pvec.pages[i - 1]->index + 1;
pagevec_release(&pvec);
} while (index < end);
+ /* No page at lastoff and we are not done - we found a hole. */
+ if (type == HOLE_OFF && lastoff < endoff) {
+ *offset = lastoff;
+ found = true;
+ }
out:
pagevec_release(&pvec);
return found;
--
2.12.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3 v2] xfs: Fix SEEK_HOLE implementation
2017-05-17 12:10 [PATCH 0/3 v2] xfs: Fix SEEK_HOLE implementation Jan Kara
` (2 preceding siblings ...)
2017-05-17 12:10 ` [PATCH 3/3] xfs: Move handling of missing page into one place " Jan Kara
@ 2017-05-17 12:31 ` Eryu Guan
2017-05-17 14:57 ` Jan Kara
3 siblings, 1 reply; 10+ messages in thread
From: Eryu Guan @ 2017-05-17 12:31 UTC (permalink / raw)
To: Jan Kara; +Cc: Darrick J . Wong, linux-xfs, Brian Foster
Hi Jan,
On Wed, May 17, 2017 at 02:10:43PM +0200, Jan Kara wrote:
> Hello,
>
> this is the second revision of the patches to fix bugs in XFS's SEEK_HOLE
> implementation and cleanup the code a bit.
>
> Changes since v1:
> * Fixed some more buggy cases
> * Simplified code a bit as suggested by Darrick
> * Fixed range check as spotted by Brian
I applied this patchset on top of 4.12-rc1 kernel to test your v4 test
case, your new test passed all my tests, but I found generic/285
regressed with sub-page block size XFS, 285.full showed that failure was
from subtest 7
07. Test file with unwritten extents, only have dirty pages
07.01 SEEK_HOLE expected 0 or 11264, got 0. succ
07.02 SEEK_HOLE expected 1 or 11264, got 1. succ
07.03 SEEK_DATA expected 10240 or 10240, got -1. FAIL
07.04 SEEK_DATA expected 10240 or 10240, got -1. FAIL
And manual test showed subtest 8 failed too
# ./src/seek_sanity_test -s 8 -e 8 /mnt/xfs/testfile
File system magic#: 0x58465342
Allocation size: 4096
08. Test file with unwritten extents, only have unwritten pages
08.01 SEEK_HOLE expected 0 or 5632, got 0. succ
08.02 SEEK_HOLE expected 1 or 5632, got 1. succ
08.03 SEEK_DATA expected 5120 or 5120, got -1. FAIL
08.04 SEEK_DATA expected 5120 or 5120, got -1. FAIL
Other subtests all passed with sub-page block size XFS.
Thanks,
Eryu
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3 v2] xfs: Fix SEEK_HOLE implementation
2017-05-17 12:31 ` [PATCH 0/3 v2] xfs: Fix SEEK_HOLE implementation Eryu Guan
@ 2017-05-17 14:57 ` Jan Kara
2017-05-18 9:03 ` Jan Kara
0 siblings, 1 reply; 10+ messages in thread
From: Jan Kara @ 2017-05-17 14:57 UTC (permalink / raw)
To: Eryu Guan; +Cc: Jan Kara, Darrick J . Wong, linux-xfs, Brian Foster
On Wed 17-05-17 20:31:15, Eryu Guan wrote:
> Hi Jan,
>
> On Wed, May 17, 2017 at 02:10:43PM +0200, Jan Kara wrote:
> > Hello,
> >
> > this is the second revision of the patches to fix bugs in XFS's SEEK_HOLE
> > implementation and cleanup the code a bit.
> >
> > Changes since v1:
> > * Fixed some more buggy cases
> > * Simplified code a bit as suggested by Darrick
> > * Fixed range check as spotted by Brian
>
> I applied this patchset on top of 4.12-rc1 kernel to test your v4 test
> case, your new test passed all my tests, but I found generic/285
> regressed with sub-page block size XFS, 285.full showed that failure was
> from subtest 7
>
> 07. Test file with unwritten extents, only have dirty pages
> 07.01 SEEK_HOLE expected 0 or 11264, got 0. succ
> 07.02 SEEK_HOLE expected 1 or 11264, got 1. succ
> 07.03 SEEK_DATA expected 10240 or 10240, got -1. FAIL
> 07.04 SEEK_DATA expected 10240 or 10240, got -1. FAIL
>
> And manual test showed subtest 8 failed too
>
> # ./src/seek_sanity_test -s 8 -e 8 /mnt/xfs/testfile
> File system magic#: 0x58465342
> Allocation size: 4096
>
> 08. Test file with unwritten extents, only have unwritten pages
> 08.01 SEEK_HOLE expected 0 or 5632, got 0. succ
> 08.02 SEEK_HOLE expected 1 or 5632, got 1. succ
> 08.03 SEEK_DATA expected 5120 or 5120, got -1. FAIL
> 08.04 SEEK_DATA expected 5120 or 5120, got -1. FAIL
>
> Other subtests all passed with sub-page block size XFS.
Strange. It doesn't fail for me this way even with 1k blocksize. I'll
investigate more tomorrow.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3 v2] xfs: Fix SEEK_HOLE implementation
2017-05-17 14:57 ` Jan Kara
@ 2017-05-18 9:03 ` Jan Kara
2017-05-18 9:47 ` Eryu Guan
0 siblings, 1 reply; 10+ messages in thread
From: Jan Kara @ 2017-05-18 9:03 UTC (permalink / raw)
To: Eryu Guan; +Cc: Jan Kara, Darrick J . Wong, linux-xfs, Brian Foster
On Wed 17-05-17 16:57:46, Jan Kara wrote:
> On Wed 17-05-17 20:31:15, Eryu Guan wrote:
> > Hi Jan,
> >
> > On Wed, May 17, 2017 at 02:10:43PM +0200, Jan Kara wrote:
> > > Hello,
> > >
> > > this is the second revision of the patches to fix bugs in XFS's SEEK_HOLE
> > > implementation and cleanup the code a bit.
> > >
> > > Changes since v1:
> > > * Fixed some more buggy cases
> > > * Simplified code a bit as suggested by Darrick
> > > * Fixed range check as spotted by Brian
> >
> > I applied this patchset on top of 4.12-rc1 kernel to test your v4 test
> > case, your new test passed all my tests, but I found generic/285
> > regressed with sub-page block size XFS, 285.full showed that failure was
> > from subtest 7
> >
> > 07. Test file with unwritten extents, only have dirty pages
> > 07.01 SEEK_HOLE expected 0 or 11264, got 0. succ
> > 07.02 SEEK_HOLE expected 1 or 11264, got 1. succ
> > 07.03 SEEK_DATA expected 10240 or 10240, got -1. FAIL
> > 07.04 SEEK_DATA expected 10240 or 10240, got -1. FAIL
> >
> > And manual test showed subtest 8 failed too
> >
> > # ./src/seek_sanity_test -s 8 -e 8 /mnt/xfs/testfile
> > File system magic#: 0x58465342
> > Allocation size: 4096
> >
> > 08. Test file with unwritten extents, only have unwritten pages
> > 08.01 SEEK_HOLE expected 0 or 5632, got 0. succ
> > 08.02 SEEK_HOLE expected 1 or 5632, got 1. succ
> > 08.03 SEEK_DATA expected 5120 or 5120, got -1. FAIL
> > 08.04 SEEK_DATA expected 5120 or 5120, got -1. FAIL
> >
> > Other subtests all passed with sub-page block size XFS.
>
> Strange. It doesn't fail for me this way even with 1k blocksize. I'll
> investigate more tomorrow.
So I've been trying quite hard to reproduce the failure but I failed. Since
you are apparently getting some error out of lseek can you find out which
error it is (likely ENXIO but I'd like to confirm) and where it gets
generated? I don't see how it could possibly happen that SEEK_DATA would
miss that single page generated by this test and how any of my patches
would influence this particular situation. Thanks!
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3 v2] xfs: Fix SEEK_HOLE implementation
2017-05-18 9:03 ` Jan Kara
@ 2017-05-18 9:47 ` Eryu Guan
2017-05-18 10:10 ` Jan Kara
0 siblings, 1 reply; 10+ messages in thread
From: Eryu Guan @ 2017-05-18 9:47 UTC (permalink / raw)
To: Jan Kara; +Cc: Darrick J . Wong, linux-xfs, Brian Foster
On Thu, May 18, 2017 at 11:03:46AM +0200, Jan Kara wrote:
> On Wed 17-05-17 16:57:46, Jan Kara wrote:
> > On Wed 17-05-17 20:31:15, Eryu Guan wrote:
> > > Hi Jan,
> > >
> > > On Wed, May 17, 2017 at 02:10:43PM +0200, Jan Kara wrote:
> > > > Hello,
> > > >
> > > > this is the second revision of the patches to fix bugs in XFS's SEEK_HOLE
> > > > implementation and cleanup the code a bit.
> > > >
> > > > Changes since v1:
> > > > * Fixed some more buggy cases
> > > > * Simplified code a bit as suggested by Darrick
> > > > * Fixed range check as spotted by Brian
> > >
> > > I applied this patchset on top of 4.12-rc1 kernel to test your v4 test
> > > case, your new test passed all my tests, but I found generic/285
> > > regressed with sub-page block size XFS, 285.full showed that failure was
> > > from subtest 7
> > >
> > > 07. Test file with unwritten extents, only have dirty pages
> > > 07.01 SEEK_HOLE expected 0 or 11264, got 0. succ
> > > 07.02 SEEK_HOLE expected 1 or 11264, got 1. succ
> > > 07.03 SEEK_DATA expected 10240 or 10240, got -1. FAIL
> > > 07.04 SEEK_DATA expected 10240 or 10240, got -1. FAIL
> > >
> > > And manual test showed subtest 8 failed too
> > >
> > > # ./src/seek_sanity_test -s 8 -e 8 /mnt/xfs/testfile
> > > File system magic#: 0x58465342
> > > Allocation size: 4096
> > >
> > > 08. Test file with unwritten extents, only have unwritten pages
> > > 08.01 SEEK_HOLE expected 0 or 5632, got 0. succ
> > > 08.02 SEEK_HOLE expected 1 or 5632, got 1. succ
> > > 08.03 SEEK_DATA expected 5120 or 5120, got -1. FAIL
> > > 08.04 SEEK_DATA expected 5120 or 5120, got -1. FAIL
> > >
> > > Other subtests all passed with sub-page block size XFS.
> >
> > Strange. It doesn't fail for me this way even with 1k blocksize. I'll
> > investigate more tomorrow.
>
> So I've been trying quite hard to reproduce the failure but I failed. Since
> you are apparently getting some error out of lseek can you find out which
> error it is (likely ENXIO but I'd like to confirm) and where it gets
> generated? I don't see how it could possibly happen that SEEK_DATA would
> miss that single page generated by this test and how any of my patches
> would influence this particular situation. Thanks!
Yes, it's ENXIO, strace log:
...
open("/mnt/xfs/testfile07", O_RDWR|O_CREAT|O_TRUNC, 0644) = 3
write(1, "07. Test file with unwritten ext"..., 6007. Test file with unwritten extents, only have dirty pages
) = 60
fallocate(3, 0, 0, 11264) = 0
pwrite(3, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"..., 1024, 10240) = 1024
lseek(3, 0, SEEK_HOLE) = 0
write(1, "07.01 SEEK_HOLE expected 0 or 11"..., 7107.01 SEEK_HOLE expected 0 or 11264, got 0. succ
) = 71
lseek(3, 1, SEEK_HOLE) = 1
write(1, "07.02 SEEK_HOLE expected 1 or 11"..., 7107.02 SEEK_HOLE expected 1 or 11264, got 1. succ
) = 71
lseek(3, 0, SEEK_DATA) = -1 ENXIO (No such device or address)
write(1, "07.03 SEEK_DATA expected 10240 o"..., 7107.03 SEEK_DATA expected 10240 or 10240, got -1. FAIL
) = 71
lseek(3, 1, SEEK_DATA) = -1 ENXIO (No such device or address)
write(1, "07.04 SEEK_DATA expected 10240 o"..., 7107.04 SEEK_DATA expected 10240 or 10240, got -1. FAIL
...
It's the second patch "xfs: Fix off-by-in in loop termination in
xfs_find_get_desired_pgoff()" introduced the issue for me. ENXIO was
returned from the if (nmap == 1) block in __xfs_seek_hole_data()
/*
* We only received one extent out of the two requested. This
* means we've hit EOF and didn't find what we are looking for.
*/
if (nmap == 1) {
....
/*
* If we were looking for data, it's nowhere to be found
*/
ASSERT(whence == SEEK_DATA);
error = -ENXIO;
goto out_error;
}
Seems that's because the do {} while() loop in xfs_find_get_desired_pgoff() was
broken out earlier due to patch 2.
/* Searching done if the page index is out of range. */
if (page->index >= end) {
goto out;
}
In my case, it returned earlier because page->index == end == 2.
I was testing with 1k block size xfs. xfs_info output:
meta-data=/dev/sdc2 isize=512 agcount=4, agsize=5242880 blks
= sectsz=512 attr=2, projid32bit=1
= crc=1 finobt=1 spinodes=0 rmapbt=0
= reflink=0
data = bsize=1024 blocks=20971520, imaxpct=25
= sunit=0 swidth=0 blks
naming =version 2 bsize=4096 ascii-ci=0 ftype=1
log =internal bsize=1024 blocks=10240, version=2
= sectsz=512 sunit=0 blks, lazy-count=1
realtime =none extsz=4096 blocks=0, rtextents=0
Hope this helps, if you need more info please let me know.
Thanks,
Eryu
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3 v2] xfs: Fix SEEK_HOLE implementation
2017-05-18 9:47 ` Eryu Guan
@ 2017-05-18 10:10 ` Jan Kara
0 siblings, 0 replies; 10+ messages in thread
From: Jan Kara @ 2017-05-18 10:10 UTC (permalink / raw)
To: Eryu Guan; +Cc: Jan Kara, Darrick J . Wong, linux-xfs, Brian Foster
On Thu 18-05-17 17:47:53, Eryu Guan wrote:
> On Thu, May 18, 2017 at 11:03:46AM +0200, Jan Kara wrote:
> > On Wed 17-05-17 16:57:46, Jan Kara wrote:
> > > On Wed 17-05-17 20:31:15, Eryu Guan wrote:
> > > > Hi Jan,
> > > >
> > > > On Wed, May 17, 2017 at 02:10:43PM +0200, Jan Kara wrote:
> > > > > Hello,
> > > > >
> > > > > this is the second revision of the patches to fix bugs in XFS's SEEK_HOLE
> > > > > implementation and cleanup the code a bit.
> > > > >
> > > > > Changes since v1:
> > > > > * Fixed some more buggy cases
> > > > > * Simplified code a bit as suggested by Darrick
> > > > > * Fixed range check as spotted by Brian
> > > >
> > > > I applied this patchset on top of 4.12-rc1 kernel to test your v4 test
> > > > case, your new test passed all my tests, but I found generic/285
> > > > regressed with sub-page block size XFS, 285.full showed that failure was
> > > > from subtest 7
> > > >
> > > > 07. Test file with unwritten extents, only have dirty pages
> > > > 07.01 SEEK_HOLE expected 0 or 11264, got 0. succ
> > > > 07.02 SEEK_HOLE expected 1 or 11264, got 1. succ
> > > > 07.03 SEEK_DATA expected 10240 or 10240, got -1. FAIL
> > > > 07.04 SEEK_DATA expected 10240 or 10240, got -1. FAIL
> > > >
> > > > And manual test showed subtest 8 failed too
> > > >
> > > > # ./src/seek_sanity_test -s 8 -e 8 /mnt/xfs/testfile
> > > > File system magic#: 0x58465342
> > > > Allocation size: 4096
> > > >
> > > > 08. Test file with unwritten extents, only have unwritten pages
> > > > 08.01 SEEK_HOLE expected 0 or 5632, got 0. succ
> > > > 08.02 SEEK_HOLE expected 1 or 5632, got 1. succ
> > > > 08.03 SEEK_DATA expected 5120 or 5120, got -1. FAIL
> > > > 08.04 SEEK_DATA expected 5120 or 5120, got -1. FAIL
> > > >
> > > > Other subtests all passed with sub-page block size XFS.
> > >
> > > Strange. It doesn't fail for me this way even with 1k blocksize. I'll
> > > investigate more tomorrow.
> >
> > So I've been trying quite hard to reproduce the failure but I failed. Since
> > you are apparently getting some error out of lseek can you find out which
> > error it is (likely ENXIO but I'd like to confirm) and where it gets
> > generated? I don't see how it could possibly happen that SEEK_DATA would
> > miss that single page generated by this test and how any of my patches
> > would influence this particular situation. Thanks!
<snip>
> Seems that's because the do {} while() loop in xfs_find_get_desired_pgoff() was
> broken out earlier due to patch 2.
>
> /* Searching done if the page index is out of range. */
> if (page->index >= end) {
> goto out;
> }
>
> In my case, it returned earlier because page->index == end == 2.
Ah! That's it. I'm not sure why you get so short unwritten extent but it's
certainly possible. I can now reproduce the issue with:
xfs_io -f -c "falloc 0 10k" -c "pwrite 9k 512" -c "seek -d 0" /mnt/file
wrote 512/512 bytes at offset 9216
512.000000 bytes, 1 ops; 0.0000 sec (6.975 MiB/sec and 14285.7143 ops/sec)
Whence Result
DATA EOF
on 1k blocksize filesystem. And the problem is indeed that in this case I
have screwed up the condition due to rounding. I'll fix the second patch in
both series for ext4 & xfs. Thanks for debugging this!
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-05-18 10:10 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-17 12:10 [PATCH 0/3 v2] xfs: Fix SEEK_HOLE implementation Jan Kara
2017-05-17 12:10 ` [PATCH 1/3] xfs: Fix missed holes in " Jan Kara
2017-05-17 12:10 ` Jan Kara
2017-05-17 12:10 ` [PATCH 2/3] xfs: Fix off-by-in in loop termination in xfs_find_get_desired_pgoff() Jan Kara
2017-05-17 12:10 ` [PATCH 3/3] xfs: Move handling of missing page into one place " Jan Kara
2017-05-17 12:31 ` [PATCH 0/3 v2] xfs: Fix SEEK_HOLE implementation Eryu Guan
2017-05-17 14:57 ` Jan Kara
2017-05-18 9:03 ` Jan Kara
2017-05-18 9:47 ` Eryu Guan
2017-05-18 10:10 ` Jan Kara
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.