[PATCH] seek_sanity_test: use fs block size not preferred IO size

[PATCH] seek_sanity_test: use fs block size not preferred IO size

[Eric Sandeen]

seek_sanity_test uses a 16x multiple of st_blksize for the basis
of its io sizes; for filesystems that use generic_fillattr,
that's the same as fs block size.  But on xfs, we take
"blocksize for file system I/O" a bit more generously, and
use xfs_preferred_iosize, which may be page size, stripe width,
or otherwise.

If we get a value of any more than 4k, the 16x multiplier puts
us past 64k, which is the threshold for xfs speculative
preallocation kicking in, and this starts messing with the
file layout that the test expects.

So: Switch the test to use statvfs, and get the true fs
block size.  This will still break for block sizes > 4k,
but for now at least it makes the test work on xfs with
page sizes > 4k using the default mkfs or smaller.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

Another option would be to mount xfs with options to
defeat speculative prealloc, or to decrease the 16x
multiplier, but the intent of the test seems to be a 16x
multiple of fs block size, so let's at least start there.

diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
index 18262c2..4042e9b 100644
--- a/src/seek_sanity_test.c
+++ b/src/seek_sanity_test.c
@@ -20,7 +20,7 @@
 #define _XOPEN_SOURCE 500
 #define _FILE_OFFSET_BITS 64
 #include <sys/types.h>
-#include <sys/stat.h>
+#include <sys/statvfs.h>
 #include <sys/vfs.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -51,16 +51,16 @@ static void get_file_system(int fd)
 
 static int get_io_sizes(int fd)
 {
-       struct stat buf;
+       struct statvfs buf;
        int ret;
 
-       ret = fstat(fd, &buf);
+       ret = fstatvfs(fd, &buf);
        if (ret)
                fprintf(stderr, "  ERROR %d: Failed to find io blocksize\n",
                        errno);
 
-       /* st_blksize is typically also the allocation size */
-       alloc_size = buf.st_blksize;
+       /* block size is typically also the allocation size */
+       alloc_size = buf.f_bsize;
        fprintf(stdout, "Allocation size: %ld\n", alloc_size);
 
        return ret;

Re: [PATCH] seek_sanity_test: use fs block size not preferred IO size

[Dave Chinner]

On Fri, Sep 02, 2016 at 07:04:01PM -0500, Eric Sandeen wrote:
> seek_sanity_test uses a 16x multiple of st_blksize for the basis
> of its io sizes; for filesystems that use generic_fillattr,
> that's the same as fs block size.  But on xfs, we take
> "blocksize for file system I/O" a bit more generously, and
> use xfs_preferred_iosize, which may be page size, stripe width,
> or otherwise.

We take what the stat(2) man page says literally:

	The st_blksize field gives the "preferred" blocksize for
	efficient filesystem I/O.  (Writing to a file in smaller
	chunks may cause an inefficient read-modify-rewrite.)

i.e. I/O sizes smaller than PAGE_SIZE can require RMW cycles in the
page cache, so you need to do page sized IO as a minimum for
efficient IO.

> If we get a value of any more than 4k, the 16x multiplier puts
> us past 64k, which is the threshold for xfs speculative
> preallocation kicking in, and this starts messing with the
> file layout that the test expects.
> 
> So: Switch the test to use statvfs, and get the true fs
> block size.  This will still break for block sizes > 4k,
> but for now at least it makes the test work on xfs with
> page sizes > 4k using the default mkfs or smaller.

So we're going to have to fix it again when someone runs 64k block
size on aarch64 or ppc64?

> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> Another option would be to mount xfs with options to
> defeat speculative prealloc, or to decrease the 16x
> multiplier, but the intent of the test seems to be a 16x
> multiple of fs block size, so let's at least start there.

The easiest way to handle this is to ftruncate() the file out to
it's final size before starting to write to the file. Then
speculative prealloc will never occur.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] seek_sanity_test: use fs block size not preferred IO size

[Eric Sandeen]

On 9/4/16 7:26 PM, Dave Chinner wrote:
> On Fri, Sep 02, 2016 at 07:04:01PM -0500, Eric Sandeen wrote:
>> seek_sanity_test uses a 16x multiple of st_blksize for the basis
>> of its io sizes; for filesystems that use generic_fillattr,
>> that's the same as fs block size.  But on xfs, we take
>> "blocksize for file system I/O" a bit more generously, and
>> use xfs_preferred_iosize, which may be page size, stripe width,
>> or otherwise.
> 
> We take what the stat(2) man page says literally:
> 
> 	The st_blksize field gives the "preferred" blocksize for
> 	efficient filesystem I/O.  (Writing to a file in smaller
> 	chunks may cause an inefficient read-modify-rewrite.)
> 
> i.e. I/O sizes smaller than PAGE_SIZE can require RMW cycles in the
> page cache, so you need to do page sized IO as a minimum for
> efficient IO.

sure.

>> If we get a value of any more than 4k, the 16x multiplier puts
>> us past 64k, which is the threshold for xfs speculative
>> preallocation kicking in, and this starts messing with the
>> file layout that the test expects.
>>
>> So: Switch the test to use statvfs, and get the true fs
>> block size.  This will still break for block sizes > 4k,
>> but for now at least it makes the test work on xfs with
>> page sizes > 4k using the default mkfs or smaller.
> 
> So we're going to have to fix it again when someone runs 64k block
> size on aarch64 or ppc64?

Yep! ;)

But from the comments, the test's intent is to use a multiple of
the allocation size.  It doesn't do that today so at least this
is a place to start, with the side effect of fixing the default
case.

>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> Another option would be to mount xfs with options to
>> defeat speculative prealloc, or to decrease the 16x
>> multiplier, but the intent of the test seems to be a 16x
>> multiple of fs block size, so let's at least start there.
> 
> The easiest way to handle this is to ftruncate() the file out to
> it's final size before starting to write to the file. Then
> speculative prealloc will never occur.

yeah, I thought about that - but that behavior isn't documented
anywhere as a userspace API - it'll work for now I guess, but
I kind of hate encoding that sort of internal heuristic into
a regression test.

-Eric

> Cheers,
> 
> Dave.
>