2013-05-30 12:45:48

by Jan Kara

[permalink] [raw]
Subject: [PATCH 1/3] 285: Fix test for ext4 in some configurations

In some configurations (e.g. 1 KB block size), ext4 can decide it is
better to zero out several blocks rather than splitting unwritten
extent. This changes results SEEK_HOLE / SEEK_DATA returns and thus the
test fails. Fix the problem by disabling the feature for this test.

Signed-off-by: Jan Kara <[email protected]>
---
tests/generic/285 | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/tests/generic/285 b/tests/generic/285
index b700a15..8078b1c 100755
--- a/tests/generic/285
+++ b/tests/generic/285
@@ -46,6 +46,12 @@ BASE_TEST_FILE=$TEST_DIR/seek_sanity_testfile

[ -x $here/src/seek_sanity_test ] || _notrun "seek_sanitfy_tester not built"

+# Disable extent zeroing for ext4 as that change where holes are created
+if [ "$FSTYP" = "ext4" ]; then
+ DEV=`basename $TEST_DEV`
+ echo 0 >/sys/fs/ext4/$DEV/extent_max_zeroout_kb
+fi
+
_cleanup()
{
eval "rm -f $BASE_TEST_FILE.*"
--
1.8.1.4



2013-05-30 12:45:48

by Jan Kara

[permalink] [raw]
Subject: [PATCH 2/3] 285: Fix file syncing

The intention of tests 08 and 09 in test generic/285 is to sync the
whole file before checking for data and holes. However the helper is
called with nbytes argument set to 0 which results in not syncing
anything. Set nbytes properly.

Signed-off-by: Jan Kara <[email protected]>
---
src/seek_sanity_test.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
index eec6903..7d5868b 100644
--- a/src/seek_sanity_test.c
+++ b/src/seek_sanity_test.c
@@ -229,7 +229,7 @@ static int test09(int fd, int testnum)
* Sync out dirty pages from bufsz * 100, this will convert
* the dirty page to writeback.
*/
- ret = do_sync_dirty_pages(fd, bufsz * 100, 0);
+ ret = do_sync_dirty_pages(fd, bufsz * 100, filsz);
if (ret)
goto out;

@@ -269,7 +269,7 @@ static int test08(int fd, int testnum)
goto out;

/* Sync out all file */
- ret = do_sync_dirty_pages(fd, 0, 0);
+ ret = do_sync_dirty_pages(fd, 0, filsz);
if (ret)
goto out;

--
1.8.1.4


2013-05-30 12:45:48

by Jan Kara

[permalink] [raw]
Subject: [PATCH 3/3] 285: Test offsets over 4GB

Test whether SEEK_HOLE and SEEK_DATA works correctly with offsets over
4GB.

Signed-off-by: Jan Kara <[email protected]>
---
src/seek_sanity_test.c | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)

diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
index 7d5868b..55e7ed6 100644
--- a/src/seek_sanity_test.c
+++ b/src/seek_sanity_test.c
@@ -18,6 +18,7 @@
*/

#define _XOPEN_SOURCE 500
+#define _FILE_OFFSET_BITS 64
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/vfs.h>
@@ -191,6 +192,42 @@ static int do_lseek(int testnum, int subtest, int fd, int filsz, int origin,
return ret;
}

+/* test a huge file to check for 4G overflows */
+static int test10(int fd, int testnum)
+{
+ char *buf = NULL;
+ int bufsz = alloc_size;
+ off_t filsz = 8ULL << 30; /* 8G */
+ off_t off = filsz - 2*bufsz;
+ int ret = -1;
+
+ buf = do_malloc(bufsz);
+ if (!buf)
+ goto out;
+ memset(buf, 'a', bufsz);
+
+ ret = do_pwrite(fd, buf, bufsz, 0);
+ if (ret)
+ goto out;
+ ret = do_pwrite(fd, buf, bufsz, off);
+ if (ret)
+ goto out;
+
+ /* offset at the beginning */
+ ret += do_lseek(testnum, 1, fd, filsz, SEEK_HOLE, 0, bufsz);
+ ret += do_lseek(testnum, 2, fd, filsz, SEEK_HOLE, 1, bufsz);
+ ret += do_lseek(testnum, 3, fd, filsz, SEEK_DATA, 0, 0);
+ ret += do_lseek(testnum, 4, fd, filsz, SEEK_DATA, 1, 1);
+
+ /* offset around eof */
+ ret += do_lseek(testnum, 5, fd, filsz, SEEK_HOLE, off, off + bufsz);
+ ret += do_lseek(testnum, 6, fd, filsz, SEEK_DATA, off, off);
+ ret += do_lseek(testnum, 7, fd, filsz, SEEK_DATA, off + 1, off + 1);
+
+out:
+ do_free(buf);
+ return ret;
+}
/*
* test file with unwritten extents, have both dirty and
* writeback pages in page cache.
@@ -577,6 +614,7 @@ struct testrec seek_tests[] = {
{ 7, test07, "Test file with unwritten extents, only have dirty pages" },
{ 8, test08, "Test file with unwritten extents, only have unwritten pages" },
{ 9, test09, "Test file with unwritten extents, have both dirty && unwritten pages" },
+ { 10, test10, "Test a huge file" },
};

static int run_test(struct testrec *tr)
--
1.8.1.4


2013-05-30 13:45:33

by Eric Sandeen

[permalink] [raw]
Subject: Re: [PATCH 1/3] 285: Fix test for ext4 in some configurations

On 5/30/13 7:45 AM, Jan Kara wrote:
> In some configurations (e.g. 1 KB block size), ext4 can decide it is
> better to zero out several blocks rather than splitting unwritten
> extent. This changes results SEEK_HOLE / SEEK_DATA returns and thus the
> test fails. Fix the problem by disabling the feature for this test.
>
> Signed-off-by: Jan Kara <[email protected]>
> ---
> tests/generic/285 | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/tests/generic/285 b/tests/generic/285
> index b700a15..8078b1c 100755
> --- a/tests/generic/285
> +++ b/tests/generic/285
> @@ -46,6 +46,12 @@ BASE_TEST_FILE=$TEST_DIR/seek_sanity_testfile
>
> [ -x $here/src/seek_sanity_test ] || _notrun "seek_sanitfy_tester not built"
>
> +# Disable extent zeroing for ext4 as that change where holes are created
> +if [ "$FSTYP" = "ext4" ]; then
> + DEV=`basename $TEST_DEV`
> + echo 0 >/sys/fs/ext4/$DEV/extent_max_zeroout_kb
> +fi
> +
> _cleanup()
> {
> eval "rm -f $BASE_TEST_FILE.*"
>

When the test dev is unmounted & remounted this will be reset, right? So:

Reviewed-by: Eric Sandeen <[email protected]>

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

2013-05-30 13:47:34

by Eric Sandeen

[permalink] [raw]
Subject: Re: [PATCH 2/3] 285: Fix file syncing

On 5/30/13 7:45 AM, Jan Kara wrote:
> The intention of tests 08 and 09 in test generic/285 is to sync the
> whole file before checking for data and holes. However the helper is
> called with nbytes argument set to 0 which results in not syncing
> anything. Set nbytes properly.

Hm, are you sure? (Is the man page wrong, or is the sync_file_range
implementation wrong?)

DESCRIPTION
sync_file_range() permits fine control when synchronizing the
open file referred to by the file descriptor fd with disk.

offset is the starting byte of the file range to be synchro-
nized. nbytes specifies the length of the range to be synchro-
nized, in bytes; if nbytes is zero, then all bytes from offset
through to the end of file are synchronized.

-Eric

> Signed-off-by: Jan Kara <[email protected]>
> ---
> src/seek_sanity_test.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
> index eec6903..7d5868b 100644
> --- a/src/seek_sanity_test.c
> +++ b/src/seek_sanity_test.c
> @@ -229,7 +229,7 @@ static int test09(int fd, int testnum)
> * Sync out dirty pages from bufsz * 100, this will convert
> * the dirty page to writeback.
> */
> - ret = do_sync_dirty_pages(fd, bufsz * 100, 0);
> + ret = do_sync_dirty_pages(fd, bufsz * 100, filsz);
> if (ret)
> goto out;
>
> @@ -269,7 +269,7 @@ static int test08(int fd, int testnum)
> goto out;
>
> /* Sync out all file */
> - ret = do_sync_dirty_pages(fd, 0, 0);
> + ret = do_sync_dirty_pages(fd, 0, filsz);
> if (ret)
> goto out;
>
>


2013-05-30 13:48:24

by Eric Sandeen

[permalink] [raw]
Subject: Re: [PATCH 3/3] 285: Test offsets over 4GB

On 5/30/13 7:45 AM, Jan Kara wrote:
> Test whether SEEK_HOLE and SEEK_DATA works correctly with offsets over
> 4GB.


Hm, should we add 2T as well while we're at it?

(and does this cause any new failures?)

-Eric

> Signed-off-by: Jan Kara <[email protected]>
> ---
> src/seek_sanity_test.c | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
> index 7d5868b..55e7ed6 100644
> --- a/src/seek_sanity_test.c
> +++ b/src/seek_sanity_test.c
> @@ -18,6 +18,7 @@
> */
>
> #define _XOPEN_SOURCE 500
> +#define _FILE_OFFSET_BITS 64
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <sys/vfs.h>
> @@ -191,6 +192,42 @@ static int do_lseek(int testnum, int subtest, int fd, int filsz, int origin,
> return ret;
> }
>
> +/* test a huge file to check for 4G overflows */
> +static int test10(int fd, int testnum)
> +{
> + char *buf = NULL;
> + int bufsz = alloc_size;
> + off_t filsz = 8ULL << 30; /* 8G */
> + off_t off = filsz - 2*bufsz;
> + int ret = -1;
> +
> + buf = do_malloc(bufsz);
> + if (!buf)
> + goto out;
> + memset(buf, 'a', bufsz);
> +
> + ret = do_pwrite(fd, buf, bufsz, 0);
> + if (ret)
> + goto out;
> + ret = do_pwrite(fd, buf, bufsz, off);
> + if (ret)
> + goto out;
> +
> + /* offset at the beginning */
> + ret += do_lseek(testnum, 1, fd, filsz, SEEK_HOLE, 0, bufsz);
> + ret += do_lseek(testnum, 2, fd, filsz, SEEK_HOLE, 1, bufsz);
> + ret += do_lseek(testnum, 3, fd, filsz, SEEK_DATA, 0, 0);
> + ret += do_lseek(testnum, 4, fd, filsz, SEEK_DATA, 1, 1);
> +
> + /* offset around eof */
> + ret += do_lseek(testnum, 5, fd, filsz, SEEK_HOLE, off, off + bufsz);
> + ret += do_lseek(testnum, 6, fd, filsz, SEEK_DATA, off, off);
> + ret += do_lseek(testnum, 7, fd, filsz, SEEK_DATA, off + 1, off + 1);
> +
> +out:
> + do_free(buf);
> + return ret;
> +}
> /*
> * test file with unwritten extents, have both dirty and
> * writeback pages in page cache.
> @@ -577,6 +614,7 @@ struct testrec seek_tests[] = {
> { 7, test07, "Test file with unwritten extents, only have dirty pages" },
> { 8, test08, "Test file with unwritten extents, only have unwritten pages" },
> { 9, test09, "Test file with unwritten extents, have both dirty && unwritten pages" },
> + { 10, test10, "Test a huge file" },
> };
>
> static int run_test(struct testrec *tr)
>

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

2013-05-30 19:57:25

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH 2/3] 285: Fix file syncing

On Thu 30-05-13 08:47:30, Eric Sandeen wrote:
> On 5/30/13 7:45 AM, Jan Kara wrote:
> > The intention of tests 08 and 09 in test generic/285 is to sync the
> > whole file before checking for data and holes. However the helper is
> > called with nbytes argument set to 0 which results in not syncing
> > anything. Set nbytes properly.
>
> Hm, are you sure? (Is the man page wrong, or is the sync_file_range
> implementation wrong?)
>
> DESCRIPTION
> sync_file_range() permits fine control when synchronizing the
> open file referred to by the file descriptor fd with disk.
>
> offset is the starting byte of the file range to be synchro-
> nized. nbytes specifies the length of the range to be synchro-
> nized, in bytes; if nbytes is zero, then all bytes from offset
> through to the end of file are synchronized.
My fault that I didn't read the manpage carefully enough. Scratch this
patch.

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2013-05-30 20:01:19

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH 3/3] 285: Test offsets over 4GB

On Thu 30-05-13 08:48:24, Eric Sandeen wrote:
> On 5/30/13 7:45 AM, Jan Kara wrote:
> > Test whether SEEK_HOLE and SEEK_DATA works correctly with offsets over
> > 4GB.
>
>
> Hm, should we add 2T as well while we're at it?
>
> (and does this cause any new failures?)
Yes, ext4 is broken. I've sent fixes for it yesterday. I'm not sure what
exactly would overflow at 2T ... block counts if signed int is used and
blocksize is 1KB?

Honza

> > Signed-off-by: Jan Kara <[email protected]>
> > ---
> > src/seek_sanity_test.c | 38 ++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 38 insertions(+)
> >
> > diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
> > index 7d5868b..55e7ed6 100644
> > --- a/src/seek_sanity_test.c
> > +++ b/src/seek_sanity_test.c
> > @@ -18,6 +18,7 @@
> > */
> >
> > #define _XOPEN_SOURCE 500
> > +#define _FILE_OFFSET_BITS 64
> > #include <sys/types.h>
> > #include <sys/stat.h>
> > #include <sys/vfs.h>
> > @@ -191,6 +192,42 @@ static int do_lseek(int testnum, int subtest, int fd, int filsz, int origin,
> > return ret;
> > }
> >
> > +/* test a huge file to check for 4G overflows */
> > +static int test10(int fd, int testnum)
> > +{
> > + char *buf = NULL;
> > + int bufsz = alloc_size;
> > + off_t filsz = 8ULL << 30; /* 8G */
> > + off_t off = filsz - 2*bufsz;
> > + int ret = -1;
> > +
> > + buf = do_malloc(bufsz);
> > + if (!buf)
> > + goto out;
> > + memset(buf, 'a', bufsz);
> > +
> > + ret = do_pwrite(fd, buf, bufsz, 0);
> > + if (ret)
> > + goto out;
> > + ret = do_pwrite(fd, buf, bufsz, off);
> > + if (ret)
> > + goto out;
> > +
> > + /* offset at the beginning */
> > + ret += do_lseek(testnum, 1, fd, filsz, SEEK_HOLE, 0, bufsz);
> > + ret += do_lseek(testnum, 2, fd, filsz, SEEK_HOLE, 1, bufsz);
> > + ret += do_lseek(testnum, 3, fd, filsz, SEEK_DATA, 0, 0);
> > + ret += do_lseek(testnum, 4, fd, filsz, SEEK_DATA, 1, 1);
> > +
> > + /* offset around eof */
> > + ret += do_lseek(testnum, 5, fd, filsz, SEEK_HOLE, off, off + bufsz);
> > + ret += do_lseek(testnum, 6, fd, filsz, SEEK_DATA, off, off);
> > + ret += do_lseek(testnum, 7, fd, filsz, SEEK_DATA, off + 1, off + 1);
> > +
> > +out:
> > + do_free(buf);
> > + return ret;
> > +}
> > /*
> > * test file with unwritten extents, have both dirty and
> > * writeback pages in page cache.
> > @@ -577,6 +614,7 @@ struct testrec seek_tests[] = {
> > { 7, test07, "Test file with unwritten extents, only have dirty pages" },
> > { 8, test08, "Test file with unwritten extents, only have unwritten pages" },
> > { 9, test09, "Test file with unwritten extents, have both dirty && unwritten pages" },
> > + { 10, test10, "Test a huge file" },
> > };
> >
> > static int run_test(struct testrec *tr)
> >
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2013-05-30 20:05:02

by Eric Sandeen

[permalink] [raw]
Subject: Re: [PATCH 3/3] 285: Test offsets over 4GB

On 5/30/13 3:01 PM, Jan Kara wrote:
> On Thu 30-05-13 08:48:24, Eric Sandeen wrote:
>> On 5/30/13 7:45 AM, Jan Kara wrote:
>>> Test whether SEEK_HOLE and SEEK_DATA works correctly with offsets over
>>> 4GB.
>>
>>
>> Hm, should we add 2T as well while we're at it?
>>
>> (and does this cause any new failures?)
> Yes, ext4 is broken. I've sent fixes for it yesterday. I'm not sure what

Argh, sorry I forgot that. I just want to be careful about more rigorous
tests making it look like we have regressions in the code.

> exactly would overflow at 2T ... block counts if signed int is used and
> blocksize is 1KB?

Hum ok, where'd I come up with 2T? :) never mind that maybe, unless
there are other potential trouble points we should add (8T? 16T for
filesystems that can handle it?)

-Eric

> Honza
>
>>> Signed-off-by: Jan Kara <[email protected]>
>>> ---
>>> src/seek_sanity_test.c | 38 ++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 38 insertions(+)
>>>
>>> diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
>>> index 7d5868b..55e7ed6 100644
>>> --- a/src/seek_sanity_test.c
>>> +++ b/src/seek_sanity_test.c
>>> @@ -18,6 +18,7 @@
>>> */
>>>
>>> #define _XOPEN_SOURCE 500
>>> +#define _FILE_OFFSET_BITS 64
>>> #include <sys/types.h>
>>> #include <sys/stat.h>
>>> #include <sys/vfs.h>
>>> @@ -191,6 +192,42 @@ static int do_lseek(int testnum, int subtest, int fd, int filsz, int origin,
>>> return ret;
>>> }
>>>
>>> +/* test a huge file to check for 4G overflows */
>>> +static int test10(int fd, int testnum)
>>> +{
>>> + char *buf = NULL;
>>> + int bufsz = alloc_size;
>>> + off_t filsz = 8ULL << 30; /* 8G */
>>> + off_t off = filsz - 2*bufsz;
>>> + int ret = -1;
>>> +
>>> + buf = do_malloc(bufsz);
>>> + if (!buf)
>>> + goto out;
>>> + memset(buf, 'a', bufsz);
>>> +
>>> + ret = do_pwrite(fd, buf, bufsz, 0);
>>> + if (ret)
>>> + goto out;
>>> + ret = do_pwrite(fd, buf, bufsz, off);
>>> + if (ret)
>>> + goto out;
>>> +
>>> + /* offset at the beginning */
>>> + ret += do_lseek(testnum, 1, fd, filsz, SEEK_HOLE, 0, bufsz);
>>> + ret += do_lseek(testnum, 2, fd, filsz, SEEK_HOLE, 1, bufsz);
>>> + ret += do_lseek(testnum, 3, fd, filsz, SEEK_DATA, 0, 0);
>>> + ret += do_lseek(testnum, 4, fd, filsz, SEEK_DATA, 1, 1);
>>> +
>>> + /* offset around eof */
>>> + ret += do_lseek(testnum, 5, fd, filsz, SEEK_HOLE, off, off + bufsz);
>>> + ret += do_lseek(testnum, 6, fd, filsz, SEEK_DATA, off, off);
>>> + ret += do_lseek(testnum, 7, fd, filsz, SEEK_DATA, off + 1, off + 1);
>>> +
>>> +out:
>>> + do_free(buf);
>>> + return ret;
>>> +}
>>> /*
>>> * test file with unwritten extents, have both dirty and
>>> * writeback pages in page cache.
>>> @@ -577,6 +614,7 @@ struct testrec seek_tests[] = {
>>> { 7, test07, "Test file with unwritten extents, only have dirty pages" },
>>> { 8, test08, "Test file with unwritten extents, only have unwritten pages" },
>>> { 9, test09, "Test file with unwritten extents, have both dirty && unwritten pages" },
>>> + { 10, test10, "Test a huge file" },
>>> };
>>>
>>> static int run_test(struct testrec *tr)
>>>
>>

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

2013-05-30 20:49:23

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH 3/3] 285: Test offsets over 4GB

On Thu 30-05-13 15:05:02, Eric Sandeen wrote:
> On 5/30/13 3:01 PM, Jan Kara wrote:
> > On Thu 30-05-13 08:48:24, Eric Sandeen wrote:
> >> On 5/30/13 7:45 AM, Jan Kara wrote:
> >>> Test whether SEEK_HOLE and SEEK_DATA works correctly with offsets over
> >>> 4GB.
> >>
> >>
> >> Hm, should we add 2T as well while we're at it?
> >>
> >> (and does this cause any new failures?)
> > Yes, ext4 is broken. I've sent fixes for it yesterday. I'm not sure what
>
> Argh, sorry I forgot that. I just want to be careful about more rigorous
> tests making it look like we have regressions in the code.
>
> > exactly would overflow at 2T ... block counts if signed int is used and
> > blocksize is 1KB?
>
> Hum ok, where'd I come up with 2T? :) never mind that maybe, unless
> there are other potential trouble points we should add (8T? 16T for
> filesystems that can handle it?)
Yeah, so 8T + something might be interesting and both ext4 & xfs should
handle that fine. 16T + something might be interesting for xfs if it
supports that size. I'll update this patch with these checks.

Honza


> >>> Signed-off-by: Jan Kara <[email protected]>
> >>> ---
> >>> src/seek_sanity_test.c | 38 ++++++++++++++++++++++++++++++++++++++
> >>> 1 file changed, 38 insertions(+)
> >>>
> >>> diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
> >>> index 7d5868b..55e7ed6 100644
> >>> --- a/src/seek_sanity_test.c
> >>> +++ b/src/seek_sanity_test.c
> >>> @@ -18,6 +18,7 @@
> >>> */
> >>>
> >>> #define _XOPEN_SOURCE 500
> >>> +#define _FILE_OFFSET_BITS 64
> >>> #include <sys/types.h>
> >>> #include <sys/stat.h>
> >>> #include <sys/vfs.h>
> >>> @@ -191,6 +192,42 @@ static int do_lseek(int testnum, int subtest, int fd, int filsz, int origin,
> >>> return ret;
> >>> }
> >>>
> >>> +/* test a huge file to check for 4G overflows */
> >>> +static int test10(int fd, int testnum)
> >>> +{
> >>> + char *buf = NULL;
> >>> + int bufsz = alloc_size;
> >>> + off_t filsz = 8ULL << 30; /* 8G */
> >>> + off_t off = filsz - 2*bufsz;
> >>> + int ret = -1;
> >>> +
> >>> + buf = do_malloc(bufsz);
> >>> + if (!buf)
> >>> + goto out;
> >>> + memset(buf, 'a', bufsz);
> >>> +
> >>> + ret = do_pwrite(fd, buf, bufsz, 0);
> >>> + if (ret)
> >>> + goto out;
> >>> + ret = do_pwrite(fd, buf, bufsz, off);
> >>> + if (ret)
> >>> + goto out;
> >>> +
> >>> + /* offset at the beginning */
> >>> + ret += do_lseek(testnum, 1, fd, filsz, SEEK_HOLE, 0, bufsz);
> >>> + ret += do_lseek(testnum, 2, fd, filsz, SEEK_HOLE, 1, bufsz);
> >>> + ret += do_lseek(testnum, 3, fd, filsz, SEEK_DATA, 0, 0);
> >>> + ret += do_lseek(testnum, 4, fd, filsz, SEEK_DATA, 1, 1);
> >>> +
> >>> + /* offset around eof */
> >>> + ret += do_lseek(testnum, 5, fd, filsz, SEEK_HOLE, off, off + bufsz);
> >>> + ret += do_lseek(testnum, 6, fd, filsz, SEEK_DATA, off, off);
> >>> + ret += do_lseek(testnum, 7, fd, filsz, SEEK_DATA, off + 1, off + 1);
> >>> +
> >>> +out:
> >>> + do_free(buf);
> >>> + return ret;
> >>> +}
> >>> /*
> >>> * test file with unwritten extents, have both dirty and
> >>> * writeback pages in page cache.
> >>> @@ -577,6 +614,7 @@ struct testrec seek_tests[] = {
> >>> { 7, test07, "Test file with unwritten extents, only have dirty pages" },
> >>> { 8, test08, "Test file with unwritten extents, only have unwritten pages" },
> >>> { 9, test09, "Test file with unwritten extents, have both dirty && unwritten pages" },
> >>> + { 10, test10, "Test a huge file" },
> >>> };
> >>>
> >>> static int run_test(struct testrec *tr)
> >>>
> >>
>
--
Jan Kara <[email protected]>
SUSE Labs, CR

2013-05-30 22:30:27

by Dave Chinner

[permalink] [raw]
Subject: Re: [PATCH 1/3] 285: Fix test for ext4 in some configurations

On Thu, May 30, 2013 at 02:45:37PM +0200, Jan Kara wrote:
> In some configurations (e.g. 1 KB block size), ext4 can decide it is
> better to zero out several blocks rather than splitting unwritten
> extent. This changes results SEEK_HOLE / SEEK_DATA returns and thus the
> test fails. Fix the problem by disabling the feature for this test.
>
> Signed-off-by: Jan Kara <[email protected]>
> ---
> tests/generic/285 | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/tests/generic/285 b/tests/generic/285
> index b700a15..8078b1c 100755
> --- a/tests/generic/285
> +++ b/tests/generic/285
> @@ -46,6 +46,12 @@ BASE_TEST_FILE=$TEST_DIR/seek_sanity_testfile
>
> [ -x $here/src/seek_sanity_test ] || _notrun "seek_sanitfy_tester not built"
>
> +# Disable extent zeroing for ext4 as that change where holes are created
> +if [ "$FSTYP" = "ext4" ]; then
> + DEV=`basename $TEST_DEV`
> + echo 0 >/sys/fs/ext4/$DEV/extent_max_zeroout_kb
> +fi

would that be better going into _require_seek_data_hole so that 286
also picks up this behaviour for ext4?

Cheers,

Dave.
--
Dave Chinner
[email protected]

2013-05-30 22:34:16

by Dave Chinner

[permalink] [raw]
Subject: Re: [PATCH 3/3] 285: Test offsets over 4GB

On Thu, May 30, 2013 at 10:49:21PM +0200, Jan Kara wrote:
> On Thu 30-05-13 15:05:02, Eric Sandeen wrote:
> > On 5/30/13 3:01 PM, Jan Kara wrote:
> > > On Thu 30-05-13 08:48:24, Eric Sandeen wrote:
> > >> On 5/30/13 7:45 AM, Jan Kara wrote:
> > >>> Test whether SEEK_HOLE and SEEK_DATA works correctly with offsets over
> > >>> 4GB.
> > >>
> > >>
> > >> Hm, should we add 2T as well while we're at it?
> > >>
> > >> (and does this cause any new failures?)
> > > Yes, ext4 is broken. I've sent fixes for it yesterday. I'm not sure what
> >
> > Argh, sorry I forgot that. I just want to be careful about more rigorous
> > tests making it look like we have regressions in the code.
> >
> > > exactly would overflow at 2T ... block counts if signed int is used and
> > > blocksize is 1KB?
> >
> > Hum ok, where'd I come up with 2T? :) never mind that maybe, unless
> > there are other potential trouble points we should add (8T? 16T for
> > filesystems that can handle it?)
> Yeah, so 8T + something might be interesting and both ext4 & xfs should
> handle that fine. 16T + something might be interesting for xfs if it
> supports that size. I'll update this patch with these checks.

What boundary traversal are we trying to test at these high
offsets?

I mean, I can understand wanting to confirm they work, but there's
no 32 bit variable boundary in the seek code at 8/16TB that needs to
be specifically test is there? i.e. is it just testing the same case
as the 8GB case?

Cheers,

Dave.
--
Dave Chinner
[email protected]

2013-05-31 08:10:26

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH 1/3] 285: Fix test for ext4 in some configurations

On Fri 31-05-13 08:30:00, Dave Chinner wrote:
> On Thu, May 30, 2013 at 02:45:37PM +0200, Jan Kara wrote:
> > In some configurations (e.g. 1 KB block size), ext4 can decide it is
> > better to zero out several blocks rather than splitting unwritten
> > extent. This changes results SEEK_HOLE / SEEK_DATA returns and thus the
> > test fails. Fix the problem by disabling the feature for this test.
> >
> > Signed-off-by: Jan Kara <[email protected]>
> > ---
> > tests/generic/285 | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/tests/generic/285 b/tests/generic/285
> > index b700a15..8078b1c 100755
> > --- a/tests/generic/285
> > +++ b/tests/generic/285
> > @@ -46,6 +46,12 @@ BASE_TEST_FILE=$TEST_DIR/seek_sanity_testfile
> >
> > [ -x $here/src/seek_sanity_test ] || _notrun "seek_sanitfy_tester not built"
> >
> > +# Disable extent zeroing for ext4 as that change where holes are created
> > +if [ "$FSTYP" = "ext4" ]; then
> > + DEV=`basename $TEST_DEV`
> > + echo 0 >/sys/fs/ext4/$DEV/extent_max_zeroout_kb
> > +fi
>
> would that be better going into _require_seek_data_hole so that 286
> also picks up this behaviour for ext4?
Actually, thinking about it, test 286 doesn't need extent_max_zeroout_kb
set. It just compares file sizes / used blocks are the same and they really
should be regardless of extent_max_zeroout_kb setting. It is even desirable
to test this with the default extent_max_zeroout_kb setting...

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2013-05-31 08:22:29

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH 3/3] 285: Test offsets over 4GB

On Fri 31-05-13 08:34:14, Dave Chinner wrote:
> On Thu, May 30, 2013 at 10:49:21PM +0200, Jan Kara wrote:
> > On Thu 30-05-13 15:05:02, Eric Sandeen wrote:
> > > On 5/30/13 3:01 PM, Jan Kara wrote:
> > > > On Thu 30-05-13 08:48:24, Eric Sandeen wrote:
> > > >> On 5/30/13 7:45 AM, Jan Kara wrote:
> > > >>> Test whether SEEK_HOLE and SEEK_DATA works correctly with offsets over
> > > >>> 4GB.
> > > >>
> > > >>
> > > >> Hm, should we add 2T as well while we're at it?
> > > >>
> > > >> (and does this cause any new failures?)
> > > > Yes, ext4 is broken. I've sent fixes for it yesterday. I'm not sure what
> > >
> > > Argh, sorry I forgot that. I just want to be careful about more rigorous
> > > tests making it look like we have regressions in the code.
> > >
> > > > exactly would overflow at 2T ... block counts if signed int is used and
> > > > blocksize is 1KB?
> > >
> > > Hum ok, where'd I come up with 2T? :) never mind that maybe, unless
> > > there are other potential trouble points we should add (8T? 16T for
> > > filesystems that can handle it?)
> > Yeah, so 8T + something might be interesting and both ext4 & xfs should
> > handle that fine. 16T + something might be interesting for xfs if it
> > supports that size. I'll update this patch with these checks.
>
> What boundary traversal are we trying to test at these high
> offsets?
If fs converts passed offsets to block numbers (as ext4 does) and wrongly
used 32-bit signed instead of 32-bit unsigned type for block numbers, the
overflow would happen at 8T. In case of XFS 64-bit counters should be used
for blocks so checking somewhere beyond 16T is for that.

I'm testing at 3 different offsets because different filesystems have
different s_maxbytes settings so we cannot just test beyond 16 TB - ext4
would just return EFBIG for that.

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs