Patchwork [3/3] 285: Test offsets over 4GB

login
register
mail settings
Submitter Jan Kara
Date May 30, 2013, 12:45 p.m.
Message ID <1369917939-22660-3-git-send-email-jack@suse.cz>
Download mbox | patch
Permalink /patch/247564/
State Not Applicable
Headers show

Comments

Jan Kara - May 30, 2013, 12:45 p.m.
Test whether SEEK_HOLE and SEEK_DATA works correctly with offsets over
4GB.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 src/seek_sanity_test.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)
Eric Sandeen - May 30, 2013, 1:48 p.m.
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 <jack@suse.cz>
> ---
>  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)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Kara - May 30, 2013, 8:01 p.m.
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 <jack@suse.cz>
> > ---
> >  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)
> > 
>
Eric Sandeen - May 30, 2013, 8:05 p.m.
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 <jack@suse.cz>
>>> ---
>>>  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)
>>>
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Kara - May 30, 2013, 8:49 p.m.
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 <jack@suse.cz>
> >>> ---
> >>>  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)
> >>>
> >>
>
Dave Chinner - May 30, 2013, 10:34 p.m.
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.
Jan Kara - May 31, 2013, 8:22 a.m.
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

Patch

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)