diff mbox series

[v2,2/2] aio_tio: determine alignment based on target filesystem

Message ID 20190111085326.171826-2-maennich@google.com
State Superseded
Headers show
Series [v2,1/2] aio_tio: fix error diagnosis for byte transfers | expand

Commit Message

Matthias Maennich Jan. 11, 2019, 8:53 a.m. UTC
The alignment for O_DIRECT operations has to match the blocksize of the
underlying filesystem. Determine the alignment from the target file's
file system.

aio_tio test cases 1 and 2 failed (nondeterministic) on aarch64 when run
on a 4096 byte blocksize filesystem and with an alignment of 512 bytes.

Determining the blocksize from the file system and using it for the
alignment resolves the test issue.

Signed-off-by: Matthias Maennich <maennich@google.com>
Reviewed-by: Alessio Balsini <balsini@google.com>
---
 testcases/kernel/io/aio/aio02/aio_tio.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Steve Muckle Jan. 29, 2019, 6:19 p.m. UTC | #1
Reviewed-by: Steve Muckle <smuckle@google.com>

On 01/11/2019 12:53 AM, Matthias Maennich wrote:
> The alignment for O_DIRECT operations has to match the blocksize of the
> underlying filesystem. Determine the alignment from the target file's
> file system.
> 
> aio_tio test cases 1 and 2 failed (nondeterministic) on aarch64 when run
> on a 4096 byte blocksize filesystem and with an alignment of 512 bytes.
> 
> Determining the blocksize from the file system and using it for the
> alignment resolves the test issue.
> 
> Signed-off-by: Matthias Maennich <maennich@google.com>
> Reviewed-by: Alessio Balsini <balsini@google.com>
> ---
>   testcases/kernel/io/aio/aio02/aio_tio.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/testcases/kernel/io/aio/aio02/aio_tio.c b/testcases/kernel/io/aio/aio02/aio_tio.c
> index 623c81a5e..db6fa2688 100644
> --- a/testcases/kernel/io/aio/aio02/aio_tio.c
> +++ b/testcases/kernel/io/aio/aio02/aio_tio.c
> @@ -34,6 +34,7 @@
>   #include "config.h"
>   #include "common.h"
>   #include "test.h"
> +#include "safe_macros.h"
>   #include <string.h>
>   #include <errno.h>
>   
> @@ -42,7 +43,6 @@
>   #define AIO_MAXIO 32
>   #define AIO_BLKSIZE (64*1024)
>   
> -static int alignment = 512;
>   static int wait_count = 0;
>   
>   /*
> @@ -94,6 +94,8 @@ int io_tio(char *pathname, int flag, int n, int operation)
>   	void *bufptr = NULL;
>   	off_t offset = 0;
>   	struct timespec timeout;
> +	struct stat fi_stat;
> +	size_t alignment;
>   
>   	io_context_t myctx;
>   	struct iocb iocb_array[AIO_MAXIO];
> @@ -105,6 +107,10 @@ int io_tio(char *pathname, int flag, int n, int operation)
>   		return -1;
>   	}
>   
> +	/* determine the alignment from the blksize of the underlying fs */
> +	SAFE_FSTAT(NULL, fd, &fi_stat);
> +	alignment = fi_stat.st_blksize;
> +
>   	res = io_queue_init(n, &myctx);
>   	//printf (" res = %d \n", res);
>   
>
Cyril Hrubis Feb. 22, 2019, 3:33 p.m. UTC | #2
Hi!
> The alignment for O_DIRECT operations has to match the blocksize of the
> underlying filesystem. Determine the alignment from the target file's
> file system.
> 
> aio_tio test cases 1 and 2 failed (nondeterministic) on aarch64 when run
> on a 4096 byte blocksize filesystem and with an alignment of 512 bytes.

Is this really about the filesystem? Citing man open:

       Under Linux 2.4, transfer sizes, and the alignment of the  user  buffer
       and  the file offset must all be multiples of the logical block size of
       the filesystem.  Since Linux 2.6.0, alignment to the logical block size
       of  the underlying storage (typically 512 bytes) suffices.  The logical
       block size can be determined using the ioctl(2) BLKSSZGET operation  or
       from the shell using the command:

       blockdev --getss

So unless you are running ancient 2.4 kernel the aligment restrictions should
be result of the blocksize of the device rather than of the filesystem.

What does the blockdev command return on the particular system?
Matthias Maennich Feb. 26, 2019, 3:37 p.m. UTC | #3
Hi!

On 22/02/2019 15:33, Cyril Hrubis wrote:
>> The alignment for O_DIRECT operations has to match the blocksize of the
>> underlying filesystem. Determine the alignment from the target file's
>> file system.
>>
>> aio_tio test cases 1 and 2 failed (nondeterministic) on aarch64 when run
>> on a 4096 byte blocksize filesystem and with an alignment of 512 bytes.
> 
> Is this really about the filesystem? Citing man open:
> 
>         Under Linux 2.4, transfer sizes, and the alignment of the  user  buffer
>         and  the file offset must all be multiples of the logical block size of
>         the filesystem.  Since Linux 2.6.0, alignment to the logical block size
>         of  the underlying storage (typically 512 bytes) suffices.  The logical
>         block size can be determined using the ioctl(2) BLKSSZGET operation  or
>         from the shell using the command:
> 
>         blockdev --getss
> 
> So unless you are running ancient 2.4 kernel the aligment restrictions should
> be result of the blocksize of the device rather than of the filesystem.

Yes. You are right. The blocksize of the device is what I was referring 
to. Sorry for that.
In particular the test hits the alignment check in fs/direct-io.c 
(do_blockdev_direct_IO) [1], which makes the direct io operation fail in 
my case with EINVAL.

> What does the blockdev command return on the particular system?
4096

I will update the patch to correct comment and commit message.

Thanks,
Matthias

[1] https://elixir.bootlin.com/linux/v4.20/source/fs/direct-io.c#L1199
diff mbox series

Patch

diff --git a/testcases/kernel/io/aio/aio02/aio_tio.c b/testcases/kernel/io/aio/aio02/aio_tio.c
index 623c81a5e..db6fa2688 100644
--- a/testcases/kernel/io/aio/aio02/aio_tio.c
+++ b/testcases/kernel/io/aio/aio02/aio_tio.c
@@ -34,6 +34,7 @@ 
 #include "config.h"
 #include "common.h"
 #include "test.h"
+#include "safe_macros.h"
 #include <string.h>
 #include <errno.h>
 
@@ -42,7 +43,6 @@ 
 #define AIO_MAXIO 32
 #define AIO_BLKSIZE (64*1024)
 
-static int alignment = 512;
 static int wait_count = 0;
 
 /*
@@ -94,6 +94,8 @@  int io_tio(char *pathname, int flag, int n, int operation)
 	void *bufptr = NULL;
 	off_t offset = 0;
 	struct timespec timeout;
+	struct stat fi_stat;
+	size_t alignment;
 
 	io_context_t myctx;
 	struct iocb iocb_array[AIO_MAXIO];
@@ -105,6 +107,10 @@  int io_tio(char *pathname, int flag, int n, int operation)
 		return -1;
 	}
 
+	/* determine the alignment from the blksize of the underlying fs */
+	SAFE_FSTAT(NULL, fd, &fi_stat);
+	alignment = fi_stat.st_blksize;
+
 	res = io_queue_init(n, &myctx);
 	//printf (" res = %d \n", res);