[LTP] [PATCH 1/3] lapi/stat.h: Add STATX_DIOALIGN related definition

[LTP] [PATCH 1/3] lapi/stat.h: Add STATX_DIOALIGN related definition

[Yang Xu]

Also add missing stx_mnt_id.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 include/lapi/stat.h | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/lapi/stat.h b/include/lapi/stat.h
index ce1f2b678..c2db8a589 100644
--- a/include/lapi/stat.h
+++ b/include/lapi/stat.h
@@ -97,7 +97,11 @@ struct statx {
 	uint32_t	stx_dev_major;
 	uint32_t	stx_dev_minor;
 	/* 0x90 */
-	uint64_t	__spare2[14];
+	uint64_t	stx_mnt_id;
+	uint32_t	stx_dio_mem_align;
+	uint32_t	stx_dio_offset_align;
+	/* 0xa0 */
+	uint64_t	__spare1[12];
 	/* 0x100 */
 };
 #endif
@@ -180,6 +184,10 @@ static inline int statx(int dirfd, const char *pathname, unsigned int flags,
 # define STATX_MNT_ID		0x00001000U
 #endif
 
+#ifndef STATX_DIOALIGN
+# define STATX_DIOALIGN		0x00002000U
+#endif
+
 #ifndef STATX_ALL
 # define STATX_ALL		0x00000fffU
 #endif
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/3] syscalls/statx10: Add basic test for STATX_DIOALIGN

[Yang Xu]

STATX_DIOALIGN is used to get stx_dio_mem_align and stx_dio_offset_align
for files on fs that support direct io. We just check whether these
value are nonzero because we don't know the underflying device details(ie
whether dio faillback to bufferio or whether use loop device).

For struct statx member check, we only check stx_dio_mem_align because
these two member is introduced toger in separate commit in kernel, so it is
safe.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 configure.ac                               |  2 +-
 runtest/syscalls                           |  1 +
 testcases/kernel/syscalls/statx/.gitignore |  1 +
 testcases/kernel/syscalls/statx/statx10.c  | 81 ++++++++++++++++++++++
 4 files changed, 84 insertions(+), 1 deletion(-)
 create mode 100644 testcases/kernel/syscalls/statx/statx10.c

diff --git a/configure.ac b/configure.ac
index 4c8763376..548288310 100644
--- a/configure.ac
+++ b/configure.ac
@@ -158,7 +158,7 @@ AC_CHECK_FUNCS(mkdtemp,[],AC_MSG_ERROR(mkdtemp() not found!))
 AC_CHECK_MEMBERS([struct fanotify_event_info_fid.fsid.__val],,,[#include <sys/fanotify.h>])
 AC_CHECK_MEMBERS([struct perf_event_mmap_page.aux_head],,,[#include <linux/perf_event.h>])
 AC_CHECK_MEMBERS([struct sigaction.sa_sigaction],[],[],[#include <signal.h>])
-AC_CHECK_MEMBERS([struct statx.stx_mnt_id],,,[
+AC_CHECK_MEMBERS([struct statx.stx_mnt_id, struct statx.stx_dio_mem_align],,,[
 #define _GNU_SOURCE
 #include <sys/stat.h>
 ])
diff --git a/runtest/syscalls b/runtest/syscalls
index 8b002e989..92123772c 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1769,6 +1769,7 @@ statx06 statx06
 statx07 statx07
 statx08 statx08
 statx09 statx09
+statx10 statx10
 
 membarrier01 membarrier01
 
diff --git a/testcases/kernel/syscalls/statx/.gitignore b/testcases/kernel/syscalls/statx/.gitignore
index 1cea43c0d..67341ff2d 100644
--- a/testcases/kernel/syscalls/statx/.gitignore
+++ b/testcases/kernel/syscalls/statx/.gitignore
@@ -7,3 +7,4 @@
 /statx07
 /statx08
 /statx09
+/statx10
diff --git a/testcases/kernel/syscalls/statx/statx10.c b/testcases/kernel/syscalls/statx/statx10.c
new file mode 100644
index 000000000..7a2c92ad2
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx10.c
@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * It is a basic test for STATX_DIOALIGN mask.
+ *
+ * - STATX_DIOALIGN   Want stx_dio_mem_align and stx_dio_offset_align value
+ *
+ * Minimum Linux version required is v6.1.
+ */
+
+#define _GNU_SOURCE
+#include <sys/types.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include "tst_test.h"
+#include "lapi/stat.h"
+
+#ifdef HAVE_STRUCT_STATX_STX_DIO_MEM_ALIGN
+#define MNTPOINT "mnt_point"
+#define TESTFILE "testfile"
+
+static int fd = -1;
+
+static void verify_statx(void)
+{
+	struct statx buf;
+
+	memset(&buf, 0, sizeof(buf));
+	TST_EXP_PASS(statx(AT_FDCWD, TESTFILE, 0, STATX_DIOALIGN, &buf),
+		"statx(AT_FDCWD, %s, 0, STATX_DIOALIGN, &buf)", TESTFILE);
+
+	if (!(buf.stx_mask & STATX_DIOALIGN)) {
+		tst_res(TCONF, "STATX_DIOALIGN is not supported until linux 6.1");
+		return;
+	}
+
+	if (buf.stx_dio_mem_align != 0)
+		tst_res(TPASS, "stx_dio_mem_align:%u", buf.stx_dio_mem_align);
+	else
+		tst_res(TFAIL, "don't get stx_dio_mem_align on supported dio fs");
+
+	if (buf.stx_dio_offset_align != 0)
+		tst_res(TPASS, "stx_dio_offset_align:%u", buf.stx_dio_offset_align);
+	else
+		tst_res(TFAIL, "don't get stx_dio_offset_align on supported dio fs");
+}
+
+static void setup(void)
+{
+	SAFE_FILE_PRINTF(TESTFILE, "AAAA");
+	fd = open(TESTFILE, O_RDWR | O_DIRECT);
+	if (fd == -1 && errno == EINVAL)
+		tst_brk(TCONF, "The regular file is not on a filesystem that support DIO");
+}
+
+static void cleanup(void)
+{
+	if (fd > -1)
+		SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.test_all = verify_statx,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.mntpoint = MNTPOINT,
+	.mount_device = 1,
+	.all_filesystems = 1,
+};
+#else
+TST_TEST_TCONF("test requires struct statx to have the stx_dio_mem_align fields");
+#endif
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 3/3] lapi/stat.h: Remove deprecated STATX_ALL macro

[Yang Xu]

Since kernel 5.10-rc1 commit 581701b7efd6 ("uapi: deprecate STATX_ALL"),
this flag has been mark as deprecated.

Kernel should keep this macro for compatibility, but ltp doesn't think
about it. So remove it.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 include/lapi/stat.h                       | 4 ----
 testcases/kernel/syscalls/statx/statx06.c | 4 ++--
 testcases/kernel/syscalls/statx/statx07.c | 6 +++---
 3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/include/lapi/stat.h b/include/lapi/stat.h
index c2db8a589..7c9a7a00c 100644
--- a/include/lapi/stat.h
+++ b/include/lapi/stat.h
@@ -188,10 +188,6 @@ static inline int statx(int dirfd, const char *pathname, unsigned int flags,
 # define STATX_DIOALIGN		0x00002000U
 #endif
 
-#ifndef STATX_ALL
-# define STATX_ALL		0x00000fffU
-#endif
-
 #ifndef STATX__RESERVED
 # define STATX__RESERVED	0x80000000U
 #endif
diff --git a/testcases/kernel/syscalls/statx/statx06.c b/testcases/kernel/syscalls/statx/statx06.c
index ce82b905b..60e736c5a 100644
--- a/testcases/kernel/syscalls/statx/statx06.c
+++ b/testcases/kernel/syscalls/statx/statx06.c
@@ -111,10 +111,10 @@ static void test_statx(unsigned int test_nr)
 	clock_wait_tick();
 	SAFE_CLOCK_GETTIME(CLOCK_REALTIME_COARSE, &after_time);
 
-	TEST(statx(AT_FDCWD, TEST_FILE, 0, STATX_ALL, &buff));
+	TEST(statx(AT_FDCWD, TEST_FILE, 0, STATX_BASIC_STATS | STATX_BTIME, &buff));
 	if (TST_RET != 0) {
 		tst_brk(TFAIL | TTERRNO,
-			"statx(AT_FDCWD, %s, 0, STATX_ALL, &buff)",
+			"statx(AT_FDCWD, %s, 0, STATX_BASIC_STATS | STATX_BTIME, &buff)",
 			TEST_FILE);
 	}
 
diff --git a/testcases/kernel/syscalls/statx/statx07.c b/testcases/kernel/syscalls/statx/statx07.c
index b13c11f72..c798c7a10 100644
--- a/testcases/kernel/syscalls/statx/statx07.c
+++ b/testcases/kernel/syscalls/statx/statx07.c
@@ -62,15 +62,15 @@ static int get_mode(char *file_name, int flag_type, char *flag_name)
 {
 	struct statx buf;
 
-	TEST(statx(AT_FDCWD, file_name, flag_type, STATX_ALL, &buf));
+	TEST(statx(AT_FDCWD, file_name, flag_type, STATX_BASIC_STATS | STATX_BTIME, &buf));
 
 	if (TST_RET == -1) {
 		tst_brk(TFAIL | TST_ERR,
-			"statx(AT_FDCWD, %s, %s, STATX_ALL, &buf)",
+			"statx(AT_FDCWD, %s, %s, STATX_BASIC_STATS | STATX_BTIME, &buf)",
 			file_name, flag_name);
 	}
 
-	tst_res(TINFO, "statx(AT_FDCWD, %s, %s, STATX_ALL, &buf) = %o",
+	tst_res(TINFO, "statx(AT_FDCWD, %s, %s, STATX_BASIC_STATS | STATX_BTIME, &buf) = %o",
 		file_name, flag_name, buf.stx_mode);
 
 	return buf.stx_mode;
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] syscalls/statx10: Add basic test for STATX_DIOALIGN

[Eric Biggers]

Hi Yang,

On Thu, Mar 30, 2023 at 04:22:48PM +0800, Yang Xu wrote:
> diff --git a/testcases/kernel/syscalls/statx/statx10.c b/testcases/kernel/syscalls/statx/statx10.c
> new file mode 100644
> index 000000000..7a2c92ad2
> --- /dev/null
> +++ b/testcases/kernel/syscalls/statx/statx10.c
> @@ -0,0 +1,81 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2023 FUJITSU LIMITED. All rights reserved.
> + * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * It is a basic test for STATX_DIOALIGN mask.
> + *
> + * - STATX_DIOALIGN   Want stx_dio_mem_align and stx_dio_offset_align value
> + *
> + * Minimum Linux version required is v6.1.
> + */
> +
> +#define _GNU_SOURCE
> +#include <sys/types.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <stdlib.h>
> +#include <stdbool.h>
> +#include "tst_test.h"
> +#include "lapi/stat.h"
> +
> +#ifdef HAVE_STRUCT_STATX_STX_DIO_MEM_ALIGN
> +#define MNTPOINT "mnt_point"
> +#define TESTFILE "testfile"
> +
> +static int fd = -1;
> +
> +static void verify_statx(void)
> +{
> +	struct statx buf;
> +
> +	memset(&buf, 0, sizeof(buf));
> +	TST_EXP_PASS(statx(AT_FDCWD, TESTFILE, 0, STATX_DIOALIGN, &buf),
> +		"statx(AT_FDCWD, %s, 0, STATX_DIOALIGN, &buf)", TESTFILE);
> +
> +	if (!(buf.stx_mask & STATX_DIOALIGN)) {
> +		tst_res(TCONF, "STATX_DIOALIGN is not supported until linux 6.1");
> +		return;
> +	}
> +
> +	if (buf.stx_dio_mem_align != 0)
> +		tst_res(TPASS, "stx_dio_mem_align:%u", buf.stx_dio_mem_align);
> +	else
> +		tst_res(TFAIL, "don't get stx_dio_mem_align on supported dio fs");
> +
> +	if (buf.stx_dio_offset_align != 0)
> +		tst_res(TPASS, "stx_dio_offset_align:%u", buf.stx_dio_offset_align);
> +	else
> +		tst_res(TFAIL, "don't get stx_dio_offset_align on supported dio fs");
> +}

Thanks for writing a test for STATX_DIOALIGN!

However, the above code isn't actually a valid test, since stx_dio_mem_align and
stx_dio_offset_align will be 0 if the file doesn't support DIO.  This is
documented in the statx(2) manual page.  Filesystems aren't guaranteed to
support DIO, if they do, they aren't guaranteed to support it on all files.

I think you might be assuming that STATX_DIOALIGN won't be set in stx_mask if
DIO is unsupported on the file.  That's not quite correct.  If STATX_DIOALIGN is
not set, that just means the filesystem doesn't support STATX_DIOALIGN.  In that
case, DIO might or might not be supported on the file.  The filesystem just
isn't making a statement one way or the other.

I gave some thoughts on possible tests for STATX_DIOALIGN here:
https://lore.kernel.org/fstests/Y7fU4pRA+LHhsMKj@sol.localdomain/
Can you take a look at that?

Thanks,

- Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] syscalls/statx10: Add basic test for STATX_DIOALIGN

[xuyang2018.jy]

Hi Eric

> Hi Yang,
> 
> On Thu, Mar 30, 2023 at 04:22:48PM +0800, Yang Xu wrote:
>> diff --git a/testcases/kernel/syscalls/statx/statx10.c b/testcases/kernel/syscalls/statx/statx10.c
>> new file mode 100644
>> index 000000000..7a2c92ad2
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/statx/statx10.c
>> @@ -0,0 +1,81 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2023 FUJITSU LIMITED. All rights reserved.
>> + * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
>> + */
>> +
>> +/*\
>> + * [Description]
>> + *
>> + * It is a basic test for STATX_DIOALIGN mask.
>> + *
>> + * - STATX_DIOALIGN   Want stx_dio_mem_align and stx_dio_offset_align value
>> + *
>> + * Minimum Linux version required is v6.1.
>> + */
>> +
>> +#define _GNU_SOURCE
>> +#include <sys/types.h>
>> +#include <unistd.h>
>> +#include <fcntl.h>
>> +#include <stdlib.h>
>> +#include <stdbool.h>
>> +#include "tst_test.h"
>> +#include "lapi/stat.h"
>> +
>> +#ifdef HAVE_STRUCT_STATX_STX_DIO_MEM_ALIGN
>> +#define MNTPOINT "mnt_point"
>> +#define TESTFILE "testfile"
>> +
>> +static int fd = -1;
>> +
>> +static void verify_statx(void)
>> +{
>> +	struct statx buf;
>> +
>> +	memset(&buf, 0, sizeof(buf));
>> +	TST_EXP_PASS(statx(AT_FDCWD, TESTFILE, 0, STATX_DIOALIGN, &buf),
>> +		"statx(AT_FDCWD, %s, 0, STATX_DIOALIGN, &buf)", TESTFILE);
>> +
>> +	if (!(buf.stx_mask & STATX_DIOALIGN)) {
>> +		tst_res(TCONF, "STATX_DIOALIGN is not supported until linux 6.1");
>> +		return;
>> +	}
>> +
>> +	if (buf.stx_dio_mem_align != 0)
>> +		tst_res(TPASS, "stx_dio_mem_align:%u", buf.stx_dio_mem_align);
>> +	else
>> +		tst_res(TFAIL, "don't get stx_dio_mem_align on supported dio fs");
>> +
>> +	if (buf.stx_dio_offset_align != 0)
>> +		tst_res(TPASS, "stx_dio_offset_align:%u", buf.stx_dio_offset_align);
>> +	else
>> +		tst_res(TFAIL, "don't get stx_dio_offset_align on supported dio fs");
>> +}
> 
> Thanks for writing a test for STATX_DIOALIGN!
> 
> However, the above code isn't actually a valid test, since stx_dio_mem_align and
> stx_dio_offset_align will be 0 if the file doesn't support DIO.  This is
> documented in the statx(2) manual page. 

  I have reported TCONF in setup when fail to open a file with O_DIRECT.

> Filesystems aren't guaranteed to
> support DIO, if they do, they aren't guaranteed to support it on all files.

In this case, I only test a regular file.

> 
> I think you might be assuming that STATX_DIOALIGN won't be set in stx_mask if
> DIO is unsupported on the file.  That's not quite correct.  If STATX_DIOALIGN is
> not set, that just means the filesystem doesn't support STATX_DIOALIGN.  In that
> case, DIO might or might not be supported on the file.  The filesystem just
> isn't making a statement one way or the other.

I just assume that for a filesystem that supported DIO. if I use statx 
with STATX_DIOALIGN mask, then I can get this mask for regular file  and 
also get dio related info on new kernel. For old kernel, it should not 
be get.

> 
> I gave some thoughts on possible tests for STATX_DIOALIGN here:
> https://lore.kernel.org/fstests/Y7fU4pRA+LHhsMKj@sol.localdomain/
> Can you take a look at that?

When I wrote this case, I have seen this email(I am also active in 
xfstests community). I got this idea that test nonzero fields for 
stx_dio_mem_align from your email with Boyang Xue.

Best Regards
Yang Xu
> 
> Thanks,
> 
> - Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] syscalls/statx10: Add basic test for STATX_DIOALIGN

[Eric Biggers]

On Fri, Mar 31, 2023 at 12:56:47PM +0000, xuyang2018.jy@fujitsu.com wrote:
> > Thanks for writing a test for STATX_DIOALIGN!
> > 
> > However, the above code isn't actually a valid test, since stx_dio_mem_align and
> > stx_dio_offset_align will be 0 if the file doesn't support DIO.  This is
> > documented in the statx(2) manual page. 
> 
>   I have reported TCONF in setup when fail to open a file with O_DIRECT.

Unfortunately that does not work either, as the behavior for when direct I/O is
unsupported is not standardized.  Some filesystems do indeed return -EINVAL for
open(O_DIRECT).  However, others just treat O_DIRECT as a hint and fall back to
buffered I/O.  And some filesystems even implement the former behavior for some
files and the latter behavior for other files...

Note that this was actually one of the motivations for STATX_DIOALIGN.
STATX_DIOALIGN tells you whether direct I/O is "really" supported, as opposed to
being "supported" with a buffered I/O fallback.

> > Filesystems aren't guaranteed to
> > support DIO, if they do, they aren't guaranteed to support it on all files.
> 
> In this case, I only test a regular file.

It is possible that on a single filesystem, direct I/O is supported on some
regular files but not others.  It is also possible that files on the same
filesystem have different direct I/O alignment restrictions.

Likewise, this was part of the motivation for STATX_DIOALIGN.

- Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] syscalls/statx10: Add basic test for STATX_DIOALIGN

[xuyang2018.jy]

Hi Eric

on 2023/04/01 3:29, Eric Biggers wrote:
> On Fri, Mar 31, 2023 at 12:56:47PM +0000, xuyang2018.jy@fujitsu.com wrote:
>>> Thanks for writing a test for STATX_DIOALIGN!
>>>
>>> However, the above code isn't actually a valid test, since stx_dio_mem_align and
>>> stx_dio_offset_align will be 0 if the file doesn't support DIO.  This is
>>> documented in the statx(2) manual page.
>>
>>    I have reported TCONF in setup when fail to open a file with O_DIRECT.
> 
> Unfortunately that does not work either, as the behavior for when direct I/O is
> unsupported is not standardized.  Some filesystems do indeed return -EINVAL for
> open(O_DIRECT).  However, others just treat O_DIRECT as a hint and fall back to
> buffered I/O.  And some filesystems even implement the former behavior for some
> files and the latter behavior for other files...
> 
> Note that this was actually one of the motivations for STATX_DIOALIGN.
> STATX_DIOALIGN tells you whether direct I/O is "really" supported, as opposed to
> being "supported" with a buffered I/O fallback.

Thanks for your explaination. IMO, ext4 and xfs should supported DIO.

I think I can test STATX_DIOALIGN only on ext4 and xfs (xfs can use 
XFS_IOC_DIOINFO ioctl to verify dio alignment information ) instead of 
all filesystem.

What do you think about it?

> 
>>> Filesystems aren't guaranteed to
>>> support DIO, if they do, they aren't guaranteed to support it on all files.
>>
>> In this case, I only test a regular file.
> 
> It is possible that on a single filesystem, direct I/O is supported on some
> regular files but not others.  It is also possible that files on the same
> filesystem have different direct I/O alignment restrictions.
> 
> Likewise, this was part of the motivation for STATX_DIOALIGN.

Understand, If only test ext4 and xfs,  except for regular file, Which 
file type does I should test?  device file, link file...


Best Regards
Yang Xu
> 
> - Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/3] syscalls/statx10: Add basic test for STATX_DIOALIGN

[Eric Biggers]

On Mon, Apr 03, 2023 at 01:24:07AM +0000, xuyang2018.jy@fujitsu.com wrote:
> 
> Thanks for your explaination. IMO, ext4 and xfs should supported DIO.
> 
> I think I can test STATX_DIOALIGN only on ext4 and xfs (xfs can use 
> XFS_IOC_DIOINFO ioctl to verify dio alignment information ) instead of 
> all filesystem.
> 
> What do you think about it?

ext4 supports direct I/O only on files that don't use any of the following
filesystem features: data journalling, encryption, and verity.  Also potentially
future filesystem features that have yet to even be envisioned...

But, I suppose that if you create an ext4 filesystem with the default options,
mount it with the default options, and don't call any ioctls to enable features,
it's fairly safe to assume that direct I/O is supported.

> 
> > 
> >>> Filesystems aren't guaranteed to
> >>> support DIO, if they do, they aren't guaranteed to support it on all files.
> >>
> >> In this case, I only test a regular file.
> > 
> > It is possible that on a single filesystem, direct I/O is supported on some
> > regular files but not others.  It is also possible that files on the same
> > filesystem have different direct I/O alignment restrictions.
> > 
> > Likewise, this was part of the motivation for STATX_DIOALIGN.
> 
> Understand, If only test ext4 and xfs,  except for regular file, Which 
> file type does I should test?  device file, link file...

Just regular files.

You could also test block devices, which are unrelated to filesystems.

- Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 1/4] lapi/stat.h: Add STATX_DIOALIGN related definition

[Yang Xu]

Also add missing stx_mnt_id.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 include/lapi/stat.h | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/lapi/stat.h b/include/lapi/stat.h
index ce1f2b678..c2db8a589 100644
--- a/include/lapi/stat.h
+++ b/include/lapi/stat.h
@@ -97,7 +97,11 @@ struct statx {
 	uint32_t	stx_dev_major;
 	uint32_t	stx_dev_minor;
 	/* 0x90 */
-	uint64_t	__spare2[14];
+	uint64_t	stx_mnt_id;
+	uint32_t	stx_dio_mem_align;
+	uint32_t	stx_dio_offset_align;
+	/* 0xa0 */
+	uint64_t	__spare1[12];
 	/* 0x100 */
 };
 #endif
@@ -180,6 +184,10 @@ static inline int statx(int dirfd, const char *pathname, unsigned int flags,
 # define STATX_MNT_ID		0x00001000U
 #endif
 
+#ifndef STATX_DIOALIGN
+# define STATX_DIOALIGN		0x00002000U
+#endif
+
 #ifndef STATX_ALL
 # define STATX_ALL		0x00000fffU
 #endif
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 2/4] syscalls/statx10: Add basic test for STATX_DIOALIGN on regular file

[Yang Xu]

STATX_DIOALIGN is used to get stx_dio_mem_align and stx_dio_offset_align
for files on fs that support direct io. We just check whether these
value are nonzero on ext4 and xfs.

For struct statx member check, we only check stx_dio_mem_align because
these two member is introduced toger in separate commit in kernel, so it is
safe.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 configure.ac                               |  2 +-
 runtest/syscalls                           |  1 +
 testcases/kernel/syscalls/statx/.gitignore |  1 +
 testcases/kernel/syscalls/statx/statx10.c  | 84 ++++++++++++++++++++++
 4 files changed, 87 insertions(+), 1 deletion(-)
 create mode 100644 testcases/kernel/syscalls/statx/statx10.c

diff --git a/configure.ac b/configure.ac
index 4c8763376..548288310 100644
--- a/configure.ac
+++ b/configure.ac
@@ -158,7 +158,7 @@ AC_CHECK_FUNCS(mkdtemp,[],AC_MSG_ERROR(mkdtemp() not found!))
 AC_CHECK_MEMBERS([struct fanotify_event_info_fid.fsid.__val],,,[#include <sys/fanotify.h>])
 AC_CHECK_MEMBERS([struct perf_event_mmap_page.aux_head],,,[#include <linux/perf_event.h>])
 AC_CHECK_MEMBERS([struct sigaction.sa_sigaction],[],[],[#include <signal.h>])
-AC_CHECK_MEMBERS([struct statx.stx_mnt_id],,,[
+AC_CHECK_MEMBERS([struct statx.stx_mnt_id, struct statx.stx_dio_mem_align],,,[
 #define _GNU_SOURCE
 #include <sys/stat.h>
 ])
diff --git a/runtest/syscalls b/runtest/syscalls
index 8b002e989..92123772c 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1769,6 +1769,7 @@ statx06 statx06
 statx07 statx07
 statx08 statx08
 statx09 statx09
+statx10 statx10
 
 membarrier01 membarrier01
 
diff --git a/testcases/kernel/syscalls/statx/.gitignore b/testcases/kernel/syscalls/statx/.gitignore
index 1cea43c0d..67341ff2d 100644
--- a/testcases/kernel/syscalls/statx/.gitignore
+++ b/testcases/kernel/syscalls/statx/.gitignore
@@ -7,3 +7,4 @@
 /statx07
 /statx08
 /statx09
+/statx10
diff --git a/testcases/kernel/syscalls/statx/statx10.c b/testcases/kernel/syscalls/statx/statx10.c
new file mode 100644
index 000000000..4307ada16
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx10.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * It is a basic test for STATX_DIOALIGN mask on ext4 and xfs filesystem.
+ *
+ * - STATX_DIOALIGN   Want stx_dio_mem_align and stx_dio_offset_align value
+ *
+ * Minimum Linux version required is v6.1.
+ */
+
+#define _GNU_SOURCE
+#include <sys/types.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include "tst_test.h"
+#include "lapi/stat.h"
+
+#ifdef HAVE_STRUCT_STATX_STX_DIO_MEM_ALIGN
+#define MNTPOINT "mnt_point"
+#define TESTFILE "testfile"
+
+static int fd = -1;
+
+static void verify_statx(void)
+{
+	struct statx buf;
+
+	memset(&buf, 0, sizeof(buf));
+	TST_EXP_PASS(statx(AT_FDCWD, TESTFILE, 0, STATX_DIOALIGN, &buf),
+		"statx(AT_FDCWD, %s, 0, STATX_DIOALIGN, &buf)", TESTFILE);
+
+	if (!(buf.stx_mask & STATX_DIOALIGN)) {
+		tst_res(TCONF, "STATX_DIOALIGN is not supported until linux 6.1");
+		return;
+	}
+
+	if (buf.stx_dio_mem_align != 0)
+		tst_res(TPASS, "stx_dio_mem_align:%u", buf.stx_dio_mem_align);
+	else
+		tst_res(TFAIL, "don't get stx_dio_mem_align on supported dio fs");
+
+	if (buf.stx_dio_offset_align != 0)
+		tst_res(TPASS, "stx_dio_offset_align:%u", buf.stx_dio_offset_align);
+	else
+		tst_res(TFAIL, "don't get stx_dio_offset_align on supported dio fs");
+}
+
+static void setup(void)
+{
+	if (strcmp(tst_device->fs_type, "xfs") && strcmp(tst_device->fs_type, "ext4"))
+		tst_brk(TCONF, "This test only supports ext4 and xfs");
+
+	SAFE_FILE_PRINTF(TESTFILE, "AAAA");
+	fd = open(TESTFILE, O_RDWR | O_DIRECT);
+	if (fd == -1 && errno == EINVAL)
+		tst_brk(TCONF, "The regular file is not on a filesystem that support DIO");
+}
+
+static void cleanup(void)
+{
+	if (fd > -1)
+		SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.test_all = verify_statx,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.mntpoint = MNTPOINT,
+	.mount_device = 1,
+	.all_filesystems = 1,
+};
+#else
+TST_TEST_TCONF("test requires struct statx to have the stx_dio_mem_align fields");
+#endif
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 3/4] syscalls/statx11: Add basic test for STATX_DIOALIGN on blockdev

[Yang Xu]

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 runtest/syscalls                           |  1 +
 testcases/kernel/syscalls/statx/.gitignore |  1 +
 testcases/kernel/syscalls/statx/statx11.c  | 95 ++++++++++++++++++++++
 3 files changed, 97 insertions(+)
 create mode 100644 testcases/kernel/syscalls/statx/statx11.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 92123772c..de5f0be35 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1770,6 +1770,7 @@ statx07 statx07
 statx08 statx08
 statx09 statx09
 statx10 statx10
+statx11 statx11
 
 membarrier01 membarrier01
 
diff --git a/testcases/kernel/syscalls/statx/.gitignore b/testcases/kernel/syscalls/statx/.gitignore
index 67341ff2d..48ac4078b 100644
--- a/testcases/kernel/syscalls/statx/.gitignore
+++ b/testcases/kernel/syscalls/statx/.gitignore
@@ -8,3 +8,4 @@
 /statx08
 /statx09
 /statx10
+/statx11
diff --git a/testcases/kernel/syscalls/statx/statx11.c b/testcases/kernel/syscalls/statx/statx11.c
new file mode 100644
index 000000000..327f74ef6
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx11.c
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * It is a basic test for STATX_DIOALIGN mask on blockdev.
+ *
+ * - STATX_DIOALIGN   Want stx_dio_mem_align and stx_dio_offset_align value
+ *
+ * Minimum Linux version required is v6.1.
+ */
+
+#define _GNU_SOURCE
+#include <sys/types.h>
+#include <sys/uio.h>
+#include <sys/ioctl.h>
+#include <sys/mount.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <libgen.h>
+#include "tst_test.h"
+#include "lapi/stat.h"
+
+#ifdef HAVE_STRUCT_STATX_STX_DIO_MEM_ALIGN
+#define MNTPOINT "mnt_point"
+#define TESTFILE "testfile"
+
+static int fd = -1, logical_sector_size;
+static char sys_bdev_dma_path[1024], sys_bdev_lgs_path[1024];
+
+static void verify_statx(void)
+{
+	struct statx buf;
+
+	memset(&buf, 0, sizeof(buf));
+	TST_EXP_PASS(statx(AT_FDCWD, tst_device->dev, 0, STATX_DIOALIGN, &buf),
+		"statx(AT_FDCWD, %s, 0, STATX_DIOALIGN, &buf)", tst_device->dev);
+
+	if (!(buf.stx_mask & STATX_DIOALIGN)) {
+		tst_res(TCONF, "STATX_DIOALIGN is not supported until linux 6.1");
+		return;
+	}
+
+	TST_ASSERT_ULONG(sys_bdev_dma_path, buf.stx_dio_mem_align - 1);
+	TST_ASSERT_ULONG(sys_bdev_lgs_path, buf.stx_dio_offset_align);
+	TST_EXP_EQ_LU(buf.stx_dio_offset_align, logical_sector_size);
+}
+
+static void setup(void)
+{
+	char *dev_name;
+	int dev_fd;
+
+	dev_fd = SAFE_OPEN(tst_device->dev, O_RDWR);
+	SAFE_IOCTL(dev_fd, BLKSSZGET, &logical_sector_size);
+	SAFE_CLOSE(dev_fd);
+
+	if (logical_sector_size <= 0)
+		tst_brk(TBROK, "BLKSSZGET returned invalid block size %i", logical_sector_size);
+
+	dev_name = basename((char *)tst_device->dev);
+	sprintf(sys_bdev_lgs_path, "/sys/block/%s/queue/logical_block_size", dev_name);
+	while (access(sys_bdev_lgs_path, F_OK) != 0) {
+		dev_name[strlen(dev_name)-1] = '\0';
+		sprintf(sys_bdev_lgs_path, "/sys/block/%s/queue/logical_block_size", dev_name);
+	}
+
+	sprintf(sys_bdev_dma_path, "/sys/block/%s/queue/dma_alignment", dev_name);
+	if (access(sys_bdev_dma_path, F_OK) != 0)
+		tst_brk(TCONF, "dma_alignment syfsfile doesn't exist");
+}
+
+static void cleanup(void)
+{
+	if (fd > -1)
+		SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.test_all = verify_statx,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.needs_device = 1,
+};
+#else
+TST_TEST_TCONF("test requires struct statx to have the stx_dio_mem_align fields");
+#endif
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 4/4] lapi/stat.h: Remove deprecated STATX_ALL macro

[Yang Xu]

Since kernel 5.10-rc1 commit 581701b7efd6 ("uapi: deprecate STATX_ALL"),
this flag has been mark as deprecated.

Kernel should keep this macro for compatibility, but ltp doesn't think
about it. So remove it.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 include/lapi/stat.h                       | 4 ----
 testcases/kernel/syscalls/statx/statx06.c | 4 ++--
 testcases/kernel/syscalls/statx/statx07.c | 6 +++---
 3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/include/lapi/stat.h b/include/lapi/stat.h
index c2db8a589..7c9a7a00c 100644
--- a/include/lapi/stat.h
+++ b/include/lapi/stat.h
@@ -188,10 +188,6 @@ static inline int statx(int dirfd, const char *pathname, unsigned int flags,
 # define STATX_DIOALIGN		0x00002000U
 #endif
 
-#ifndef STATX_ALL
-# define STATX_ALL		0x00000fffU
-#endif
-
 #ifndef STATX__RESERVED
 # define STATX__RESERVED	0x80000000U
 #endif
diff --git a/testcases/kernel/syscalls/statx/statx06.c b/testcases/kernel/syscalls/statx/statx06.c
index ce82b905b..60e736c5a 100644
--- a/testcases/kernel/syscalls/statx/statx06.c
+++ b/testcases/kernel/syscalls/statx/statx06.c
@@ -111,10 +111,10 @@ static void test_statx(unsigned int test_nr)
 	clock_wait_tick();
 	SAFE_CLOCK_GETTIME(CLOCK_REALTIME_COARSE, &after_time);
 
-	TEST(statx(AT_FDCWD, TEST_FILE, 0, STATX_ALL, &buff));
+	TEST(statx(AT_FDCWD, TEST_FILE, 0, STATX_BASIC_STATS | STATX_BTIME, &buff));
 	if (TST_RET != 0) {
 		tst_brk(TFAIL | TTERRNO,
-			"statx(AT_FDCWD, %s, 0, STATX_ALL, &buff)",
+			"statx(AT_FDCWD, %s, 0, STATX_BASIC_STATS | STATX_BTIME, &buff)",
 			TEST_FILE);
 	}
 
diff --git a/testcases/kernel/syscalls/statx/statx07.c b/testcases/kernel/syscalls/statx/statx07.c
index b13c11f72..c798c7a10 100644
--- a/testcases/kernel/syscalls/statx/statx07.c
+++ b/testcases/kernel/syscalls/statx/statx07.c
@@ -62,15 +62,15 @@ static int get_mode(char *file_name, int flag_type, char *flag_name)
 {
 	struct statx buf;
 
-	TEST(statx(AT_FDCWD, file_name, flag_type, STATX_ALL, &buf));
+	TEST(statx(AT_FDCWD, file_name, flag_type, STATX_BASIC_STATS | STATX_BTIME, &buf));
 
 	if (TST_RET == -1) {
 		tst_brk(TFAIL | TST_ERR,
-			"statx(AT_FDCWD, %s, %s, STATX_ALL, &buf)",
+			"statx(AT_FDCWD, %s, %s, STATX_BASIC_STATS | STATX_BTIME, &buf)",
 			file_name, flag_name);
 	}
 
-	tst_res(TINFO, "statx(AT_FDCWD, %s, %s, STATX_ALL, &buf) = %o",
+	tst_res(TINFO, "statx(AT_FDCWD, %s, %s, STATX_BASIC_STATS | STATX_BTIME, &buf) = %o",
 		file_name, flag_name, buf.stx_mode);
 
 	return buf.stx_mode;
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 2/4] syscalls/statx10: Add basic test for STATX_DIOALIGN on regular file

[Eric Biggers]

On Mon, Apr 03, 2023 at 06:44:34PM +0800, Yang Xu wrote:
> +static void verify_statx(void)
> +{
> +	struct statx buf;
> +
> +	memset(&buf, 0, sizeof(buf));
> +	TST_EXP_PASS(statx(AT_FDCWD, TESTFILE, 0, STATX_DIOALIGN, &buf),
> +		"statx(AT_FDCWD, %s, 0, STATX_DIOALIGN, &buf)", TESTFILE);
> +
> +	if (!(buf.stx_mask & STATX_DIOALIGN)) {
> +		tst_res(TCONF, "STATX_DIOALIGN is not supported until linux 6.1");
> +		return;
> +	}
> +
> +	if (buf.stx_dio_mem_align != 0)
> +		tst_res(TPASS, "stx_dio_mem_align:%u", buf.stx_dio_mem_align);
> +	else
> +		tst_res(TFAIL, "don't get stx_dio_mem_align on supported dio fs");
> +
> +	if (buf.stx_dio_offset_align != 0)
> +		tst_res(TPASS, "stx_dio_offset_align:%u", buf.stx_dio_offset_align);
> +	else
> +		tst_res(TFAIL, "don't get stx_dio_offset_align on supported dio fs");
> +}
> +
> +static void setup(void)
> +{
> +	if (strcmp(tst_device->fs_type, "xfs") && strcmp(tst_device->fs_type, "ext4"))
> +		tst_brk(TCONF, "This test only supports ext4 and xfs");
> +
> +	SAFE_FILE_PRINTF(TESTFILE, "AAAA");
> +	fd = open(TESTFILE, O_RDWR | O_DIRECT);
> +	if (fd == -1 && errno == EINVAL)
> +		tst_brk(TCONF, "The regular file is not on a filesystem that support DIO");
> +}

On ext4, files that use certain filesystem features (data journalling,
encryption, and verity) fall back to buffered I/O.  This test will fail when
passed such a file, as it assumes that DIO doesn't fall back to buffered I/O.

How is it guaranteed that such a file is not passed to this test?

- Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 3/4] syscalls/statx11: Add basic test for STATX_DIOALIGN on blockdev

[Eric Biggers]

On Mon, Apr 03, 2023 at 06:44:35PM +0800, Yang Xu wrote:
> +	TST_ASSERT_ULONG(sys_bdev_dma_path, buf.stx_dio_mem_align - 1);
> +	TST_ASSERT_ULONG(sys_bdev_lgs_path, buf.stx_dio_offset_align);

This test is tightly coupled to the kernel's current DIO restrictions on block
devices.  These changed in v6.0, and they are subject to further change in the
future.

I guess that is fine for now because STATX_DIOALIGN is only in v6.1 and later
anyway.  But, please leave a super clear comment that documents the assumptions
this test is making.

- Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 2/4] syscalls/statx10: Add basic test for STATX_DIOALIGN on regular file

[xuyang2018.jy]

on 2023/04/04 1:01, Eric Biggers wrote:
> On Mon, Apr 03, 2023 at 06:44:34PM +0800, Yang Xu wrote:
>> +static void verify_statx(void)
>> +{
>> +	struct statx buf;
>> +
>> +	memset(&buf, 0, sizeof(buf));
>> +	TST_EXP_PASS(statx(AT_FDCWD, TESTFILE, 0, STATX_DIOALIGN, &buf),
>> +		"statx(AT_FDCWD, %s, 0, STATX_DIOALIGN, &buf)", TESTFILE);
>> +
>> +	if (!(buf.stx_mask & STATX_DIOALIGN)) {
>> +		tst_res(TCONF, "STATX_DIOALIGN is not supported until linux 6.1");
>> +		return;
>> +	}
>> +
>> +	if (buf.stx_dio_mem_align != 0)
>> +		tst_res(TPASS, "stx_dio_mem_align:%u", buf.stx_dio_mem_align);
>> +	else
>> +		tst_res(TFAIL, "don't get stx_dio_mem_align on supported dio fs");
>> +
>> +	if (buf.stx_dio_offset_align != 0)
>> +		tst_res(TPASS, "stx_dio_offset_align:%u", buf.stx_dio_offset_align);
>> +	else
>> +		tst_res(TFAIL, "don't get stx_dio_offset_align on supported dio fs");
>> +}
>> +
>> +static void setup(void)
>> +{
>> +	if (strcmp(tst_device->fs_type, "xfs") && strcmp(tst_device->fs_type, "ext4"))
>> +		tst_brk(TCONF, "This test only supports ext4 and xfs");
>> +
>> +	SAFE_FILE_PRINTF(TESTFILE, "AAAA");
>> +	fd = open(TESTFILE, O_RDWR | O_DIRECT);
>> +	if (fd == -1 && errno == EINVAL)
>> +		tst_brk(TCONF, "The regular file is not on a filesystem that support DIO");
>> +}
> 
> On ext4, files that use certain filesystem features (data journalling,
> encryption, and verity) fall back to buffered I/O.  This test will fail when
> passed such a file, as it assumes that DIO doesn't fall back to buffered I/O.

Yes, I also reproduce it when I  mount a partion with data=journal on 
/tmp directory.

  mount -o data=journal /dev/vdb /tmp
[root@localhost statx]# ./statx10
......
tst_test.c:1634: TINFO: === Testing on ext2 ===
tst_test.c:1093: TINFO: Formatting /dev/loop0 with ext2 opts='' extra 
opts=''
mke2fs 1.46.5 (30-Dec-2021)
statx10.c:59: TCONF: This test only supports ext4 and xfs
tst_test.c:1634: TINFO: === Testing on ext3 ===
tst_test.c:1093: TINFO: Formatting /dev/loop0 with ext3 opts='' extra 
opts=''
mke2fs 1.46.5 (30-Dec-2021)
statx10.c:59: TCONF: This test only supports ext4 and xfs
tst_test.c:1634: TINFO: === Testing on ext4 ===
tst_test.c:1093: TINFO: Formatting /dev/loop0 with ext4 opts='' extra 
opts=''
mke2fs 1.46.5 (30-Dec-2021)
statx10.c:37: TPASS: statx(AT_FDCWD, testfile, 0, STATX_DIOALIGN, &buf) 
passed
statx10.c:48: TFAIL: don't get stx_dio_mem_align on supported dio fs
statx10.c:53: TFAIL: don't get stx_dio_offset_align on supported dio fs
tst_test.c:1634: TINFO: === Testing on xfs ===
tst_test.c:1093: TINFO: Formatting /dev/loop0 with xfs opts='' extra opts=''
statx10.c:37: TPASS: statx(AT_FDCWD, testfile, 0, STATX_DIOALIGN, &buf) 
passed
statx10.c:48: TFAIL: don't get stx_dio_mem_align on supported dio fs
statx10.c:53: TFAIL: don't get stx_dio_offset_align on supported dio fs
tst_test.c:1634: TINFO: === Testing on tmpfs ===
tst_test.c:1093: TINFO: Skipping mkfs for TMPFS filesystem
tst_test.c:1074: TINFO: Limiting tmpfs size to 32MB
statx10.c:59: TCONF: This test only supports ext4 and xfs

IMO, If we use a actual block device to test instead of use a loop 
device on /tmp directory, it should be ok.

export LTP_DEV=/dev/vdb
tst_test.c:1634: TINFO: === Testing on ext2 ===
tst_test.c:1093: TINFO: Formatting /dev/vdb with ext2 opts='' extra opts=''
mke2fs 1.46.5 (30-Dec-2021)
statx10.c:59: TCONF: This test only supports ext4 and xfs
tst_test.c:1634: TINFO: === Testing on ext3 ===
tst_test.c:1093: TINFO: Formatting /dev/vdb with ext3 opts='' extra opts=''
mke2fs 1.46.5 (30-Dec-2021)
statx10.c:59: TCONF: This test only supports ext4 and xfs
tst_test.c:1634: TINFO: === Testing on ext4 ===
tst_test.c:1093: TINFO: Formatting /dev/vdb with ext4 opts='' extra opts=''
mke2fs 1.46.5 (30-Dec-2021)
statx10.c:37: TPASS: statx(AT_FDCWD, testfile, 0, STATX_DIOALIGN, &buf) 
passed
statx10.c:46: TPASS: stx_dio_mem_align:512
statx10.c:51: TPASS: stx_dio_offset_align:512
tst_test.c:1634: TINFO: === Testing on xfs ===
tst_test.c:1093: TINFO: Formatting /dev/vdb with xfs opts='' extra opts=''
statx10.c:37: TPASS: statx(AT_FDCWD, testfile, 0, STATX_DIOALIGN, &buf) 
passed
statx10.c:46: TPASS: stx_dio_mem_align:512
statx10.c:51: TPASS: stx_dio_offset_align:512
tst_test.c:1634: TINFO: === Testing on tmpfs ===
tst_test.c:1093: TINFO: Skipping mkfs for TMPFS filesystem
tst_test.c:1074: TINFO: Limiting tmpfs size to 32MB
statx10.c:59: TCONF: This test only supports ext4 and xfs

> 
> How is it guaranteed that such a file is not passed to this test?

Since /etc/mk2fs.conf doesn't enable data=journal, encrypt, verity 
feature by default and ltp use default mkfs configure , mount option.

I think we can detect dio io support before mount.


Best Regards
Yang Xu



> 
> - Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 3/4] syscalls/statx11: Add basic test for STATX_DIOALIGN on blockdev

[xuyang2018.jy]

on 2023/04/04 1:04, Eric Biggers wrote:
> This test is tightly coupled to the kernel's current DIO restrictions on block
> devices.  These changed in v6.0, and they are subject to further change in the
> future.
> 
> I guess that is fine for now because STATX_DIOALIGN is only in v6.1 and later
> anyway.  But, please leave a super clear comment that documents the assumptions
> this test is making.

OK. Will add a clear comment in v3.

Best Regards
Yang Xu

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 2/4] syscalls/statx10: Add basic test for STATX_DIOALIGN on regular file

[xuyang2018.jy]

on 2023/04/04 11:10, xuyang2018.jy@fujitsu.com wrote:
> 
> on 2023/04/04 1:01, Eric Biggers wrote:
>> On Mon, Apr 03, 2023 at 06:44:34PM +0800, Yang Xu wrote:
>>> +static void verify_statx(void)
>>> +{
>>> +	struct statx buf;
>>> +
>>> +	memset(&buf, 0, sizeof(buf));
>>> +	TST_EXP_PASS(statx(AT_FDCWD, TESTFILE, 0, STATX_DIOALIGN, &buf),
>>> +		"statx(AT_FDCWD, %s, 0, STATX_DIOALIGN, &buf)", TESTFILE);
>>> +
>>> +	if (!(buf.stx_mask & STATX_DIOALIGN)) {
>>> +		tst_res(TCONF, "STATX_DIOALIGN is not supported until linux 6.1");
>>> +		return;
>>> +	}
>>> +
>>> +	if (buf.stx_dio_mem_align != 0)
>>> +		tst_res(TPASS, "stx_dio_mem_align:%u", buf.stx_dio_mem_align);
>>> +	else
>>> +		tst_res(TFAIL, "don't get stx_dio_mem_align on supported dio fs");
>>> +
>>> +	if (buf.stx_dio_offset_align != 0)
>>> +		tst_res(TPASS, "stx_dio_offset_align:%u", buf.stx_dio_offset_align);
>>> +	else
>>> +		tst_res(TFAIL, "don't get stx_dio_offset_align on supported dio fs");
>>> +}
>>> +
>>> +static void setup(void)
>>> +{
>>> +	if (strcmp(tst_device->fs_type, "xfs") && strcmp(tst_device->fs_type, "ext4"))
>>> +		tst_brk(TCONF, "This test only supports ext4 and xfs");
>>> +
>>> +	SAFE_FILE_PRINTF(TESTFILE, "AAAA");
>>> +	fd = open(TESTFILE, O_RDWR | O_DIRECT);
>>> +	if (fd == -1 && errno == EINVAL)
>>> +		tst_brk(TCONF, "The regular file is not on a filesystem that support DIO");
>>> +}
>>
>> On ext4, files that use certain filesystem features (data journalling,
>> encryption, and verity) fall back to buffered I/O.  This test will fail when
>> passed such a file, as it assumes that DIO doesn't fall back to buffered I/O.
> 
> Yes, I also reproduce it when I  mount a partion with data=journal on
> /tmp directory.
> 
>    mount -o data=journal /dev/vdb /tmp
> [root@localhost statx]# ./statx10
> ......
> tst_test.c:1634: TINFO: === Testing on ext2 ===
> tst_test.c:1093: TINFO: Formatting /dev/loop0 with ext2 opts='' extra
> opts=''
> mke2fs 1.46.5 (30-Dec-2021)
> statx10.c:59: TCONF: This test only supports ext4 and xfs
> tst_test.c:1634: TINFO: === Testing on ext3 ===
> tst_test.c:1093: TINFO: Formatting /dev/loop0 with ext3 opts='' extra
> opts=''
> mke2fs 1.46.5 (30-Dec-2021)
> statx10.c:59: TCONF: This test only supports ext4 and xfs
> tst_test.c:1634: TINFO: === Testing on ext4 ===
> tst_test.c:1093: TINFO: Formatting /dev/loop0 with ext4 opts='' extra
> opts=''
> mke2fs 1.46.5 (30-Dec-2021)
> statx10.c:37: TPASS: statx(AT_FDCWD, testfile, 0, STATX_DIOALIGN, &buf)
> passed

Sorry, I did a mistake, I just test regular  file on /tmp  instead of on 
real ext4 or xfs filesystem because testfile does't under mntpoint.

-#define TESTFILE "testfile"
+#define TESTFILE MNTPOINT"/testfile"

Best Regards
Yang Xu

> statx10.c:48: TFAIL: don't get stx_dio_mem_align on supported dio fs
> statx10.c:53: TFAIL: don't get stx_dio_offset_align on supported dio fs
> tst_test.c:1634: TINFO: === Testing on xfs ===
> tst_test.c:1093: TINFO: Formatting /dev/loop0 with xfs opts='' extra opts=''
> statx10.c:37: TPASS: statx(AT_FDCWD, testfile, 0, STATX_DIOALIGN, &buf)
> passed
> statx10.c:48: TFAIL: don't get stx_dio_mem_align on supported dio fs
> statx10.c:53: TFAIL: don't get stx_dio_offset_align on supported dio fs
> tst_test.c:1634: TINFO: === Testing on tmpfs ===
> tst_test.c:1093: TINFO: Skipping mkfs for TMPFS filesystem
> tst_test.c:1074: TINFO: Limiting tmpfs size to 32MB
> statx10.c:59: TCONF: This test only supports ext4 and xfs
> 
> IMO, If we use a actual block device to test instead of use a loop
> device on /tmp directory, it should be ok.
> 
> export LTP_DEV=/dev/vdb
> tst_test.c:1634: TINFO: === Testing on ext2 ===
> tst_test.c:1093: TINFO: Formatting /dev/vdb with ext2 opts='' extra opts=''
> mke2fs 1.46.5 (30-Dec-2021)
> statx10.c:59: TCONF: This test only supports ext4 and xfs
> tst_test.c:1634: TINFO: === Testing on ext3 ===
> tst_test.c:1093: TINFO: Formatting /dev/vdb with ext3 opts='' extra opts=''
> mke2fs 1.46.5 (30-Dec-2021)
> statx10.c:59: TCONF: This test only supports ext4 and xfs
> tst_test.c:1634: TINFO: === Testing on ext4 ===
> tst_test.c:1093: TINFO: Formatting /dev/vdb with ext4 opts='' extra opts=''
> mke2fs 1.46.5 (30-Dec-2021)
> statx10.c:37: TPASS: statx(AT_FDCWD, testfile, 0, STATX_DIOALIGN, &buf)
> passed
> statx10.c:46: TPASS: stx_dio_mem_align:512
> statx10.c:51: TPASS: stx_dio_offset_align:512
> tst_test.c:1634: TINFO: === Testing on xfs ===
> tst_test.c:1093: TINFO: Formatting /dev/vdb with xfs opts='' extra opts=''
> statx10.c:37: TPASS: statx(AT_FDCWD, testfile, 0, STATX_DIOALIGN, &buf)
> passed
> statx10.c:46: TPASS: stx_dio_mem_align:512
> statx10.c:51: TPASS: stx_dio_offset_align:512
> tst_test.c:1634: TINFO: === Testing on tmpfs ===
> tst_test.c:1093: TINFO: Skipping mkfs for TMPFS filesystem
> tst_test.c:1074: TINFO: Limiting tmpfs size to 32MB
> statx10.c:59: TCONF: This test only supports ext4 and xfs
> 
>>
>> How is it guaranteed that such a file is not passed to this test?
> 
> Since /etc/mk2fs.conf doesn't enable data=journal, encrypt, verity
> feature by default and ltp use default mkfs configure , mount option.
> 
> I think we can detect dio io support before mount.
> 
> 
> Best Regards
> Yang Xu
> 
> 
> 
>>
>> - Eric
> 

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3 1/4] lapi/stat.h: Add STATX_DIOALIGN related definition

[Yang Xu]

Also add missing stx_mnt_id.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 include/lapi/stat.h | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/lapi/stat.h b/include/lapi/stat.h
index ce1f2b678..c2db8a589 100644
--- a/include/lapi/stat.h
+++ b/include/lapi/stat.h
@@ -97,7 +97,11 @@ struct statx {
 	uint32_t	stx_dev_major;
 	uint32_t	stx_dev_minor;
 	/* 0x90 */
-	uint64_t	__spare2[14];
+	uint64_t	stx_mnt_id;
+	uint32_t	stx_dio_mem_align;
+	uint32_t	stx_dio_offset_align;
+	/* 0xa0 */
+	uint64_t	__spare1[12];
 	/* 0x100 */
 };
 #endif
@@ -180,6 +184,10 @@ static inline int statx(int dirfd, const char *pathname, unsigned int flags,
 # define STATX_MNT_ID		0x00001000U
 #endif
 
+#ifndef STATX_DIOALIGN
+# define STATX_DIOALIGN		0x00002000U
+#endif
+
 #ifndef STATX_ALL
 # define STATX_ALL		0x00000fffU
 #endif
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3 2/4] syscalls/statx10: Add basic test for STATX_DIOALIGN on regular file

[Yang Xu]

If the caller didn't include STATX_DIOALIGN in the request mask,
direct I/O alignment information isn't returned since statx() isn't
required to return unrequested information.

STATX_DIOALIGN is used to get stx_dio_mem_align and stx_dio_offset_align
for files on fs that support direct io. We just check whether these
value are nonzero on ext4 and xfs.

On ext4, files that use certain filesystem features (data journalling,
encryption, and verity) fall back to buffered I/O. But ltp doesn't use
these features by default, So I think dio should not fall back to
buffered I/O.

For struct statx member check, we only check stx_dio_mem_align because
these two member is introduced toger in separate commit in kernel, so it is
safe.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
v2->v3:
1. fix testfile path problme
2. also test stx_dio_mem_align and stx_dio_offset_align is not filled
when not request STATX_DIOALIGN
 configure.ac                               |  2 +-
 runtest/syscalls                           |  1 +
 testcases/kernel/syscalls/statx/.gitignore |  1 +
 testcases/kernel/syscalls/statx/statx10.c  | 98 ++++++++++++++++++++++
 4 files changed, 101 insertions(+), 1 deletion(-)
 create mode 100644 testcases/kernel/syscalls/statx/statx10.c

diff --git a/configure.ac b/configure.ac
index 4c8763376..548288310 100644
--- a/configure.ac
+++ b/configure.ac
@@ -158,7 +158,7 @@ AC_CHECK_FUNCS(mkdtemp,[],AC_MSG_ERROR(mkdtemp() not found!))
 AC_CHECK_MEMBERS([struct fanotify_event_info_fid.fsid.__val],,,[#include <sys/fanotify.h>])
 AC_CHECK_MEMBERS([struct perf_event_mmap_page.aux_head],,,[#include <linux/perf_event.h>])
 AC_CHECK_MEMBERS([struct sigaction.sa_sigaction],[],[],[#include <signal.h>])
-AC_CHECK_MEMBERS([struct statx.stx_mnt_id],,,[
+AC_CHECK_MEMBERS([struct statx.stx_mnt_id, struct statx.stx_dio_mem_align],,,[
 #define _GNU_SOURCE
 #include <sys/stat.h>
 ])
diff --git a/runtest/syscalls b/runtest/syscalls
index 8b002e989..92123772c 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1769,6 +1769,7 @@ statx06 statx06
 statx07 statx07
 statx08 statx08
 statx09 statx09
+statx10 statx10
 
 membarrier01 membarrier01
 
diff --git a/testcases/kernel/syscalls/statx/.gitignore b/testcases/kernel/syscalls/statx/.gitignore
index 1cea43c0d..67341ff2d 100644
--- a/testcases/kernel/syscalls/statx/.gitignore
+++ b/testcases/kernel/syscalls/statx/.gitignore
@@ -7,3 +7,4 @@
 /statx07
 /statx08
 /statx09
+/statx10
diff --git a/testcases/kernel/syscalls/statx/statx10.c b/testcases/kernel/syscalls/statx/statx10.c
new file mode 100644
index 000000000..7e5d287d9
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx10.c
@@ -0,0 +1,98 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * It is a basic test for STATX_DIOALIGN mask on ext4 and xfs filesystem.
+ *
+ * - STATX_DIOALIGN   Want stx_dio_mem_align and stx_dio_offset_align value
+ *
+ * These two values are only filled when STATX_DIOALIGN in the request mask.
+ * Also check these two values are nonzero under dio situation when
+ * STATX_DIOALIGN in the request mask
+ *
+ * Minimum Linux version required is v6.1.
+ */
+
+#define _GNU_SOURCE
+#include <sys/types.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include "tst_test.h"
+#include "lapi/stat.h"
+
+#ifdef HAVE_STRUCT_STATX_STX_DIO_MEM_ALIGN
+#define MNTPOINT "mnt_point"
+#define TESTFILE MNTPOINT"/testfile"
+
+static int fd = -1;
+
+static void verify_statx(void)
+{
+	struct statx buf;
+
+	memset(&buf, 0, sizeof(buf));
+	TST_EXP_PASS(statx(AT_FDCWD, TESTFILE, 0, STATX_DIOALIGN, &buf),
+		"statx(AT_FDCWD, %s, 0, STATX_DIOALIGN, &buf)", TESTFILE);
+
+	if (!(buf.stx_mask & STATX_DIOALIGN)) {
+		tst_res(TCONF, "STATX_DIOALIGN is not supported until linux 6.1");
+		return;
+	}
+
+	if (buf.stx_dio_mem_align != 0)
+		tst_res(TPASS, "stx_dio_mem_align:%u", buf.stx_dio_mem_align);
+	else
+		tst_res(TFAIL, "don't get stx_dio_mem_align on supported dio fs");
+
+	if (buf.stx_dio_offset_align != 0)
+		tst_res(TPASS, "stx_dio_offset_align:%u", buf.stx_dio_offset_align);
+	else
+		tst_res(TFAIL, "don't get stx_dio_offset_align on supported dio fs");
+
+	TST_EXP_PASS(statx(AT_FDCWD, TESTFILE, 0, 0, &buf),
+		"statx(AT_FDCWD, %s, 0, 0, &buf)", TESTFILE);
+
+	if ((buf.stx_mask & STATX_DIOALIGN)) {
+		tst_res(TFAIL, "STATX_DIOALIGN mask return even not request");
+		return;
+	}
+	TST_EXP_EQ_LU(buf.stx_dio_mem_align, 0);
+	TST_EXP_EQ_LU(buf.stx_dio_offset_align, 0);
+}
+
+static void setup(void)
+{
+	if (strcmp(tst_device->fs_type, "xfs") && strcmp(tst_device->fs_type, "ext4"))
+		tst_brk(TCONF, "This test only supports ext4 and xfs");
+
+	SAFE_FILE_PRINTF(TESTFILE, "AAAA");
+	fd = open(TESTFILE, O_RDWR | O_DIRECT);
+	if (fd == -1 && errno == EINVAL)
+		tst_brk(TCONF, "The regular file is not on a filesystem that support DIO");
+}
+
+static void cleanup(void)
+{
+	if (fd > -1)
+		SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.test_all = verify_statx,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.mntpoint = MNTPOINT,
+	.mount_device = 1,
+	.all_filesystems = 1,
+};
+#else
+TST_TEST_TCONF("test requires struct statx to have the stx_dio_mem_align fields");
+#endif
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3 3/4] syscalls/statx11: Add basic test for STATX_DIOALIGN on block device

[Yang Xu]

Since STATX_DIOLAIGN is only supported on regular file and block device,
so this case is used to test the latter.

This test is tightly coupled to the kernel's current DIO restrictions on block
devices.  These changed in v6.0, and they are subject to further change in the
future.

It is fine for now because STATX_DIOALIGN is only in v6.1 and later
anyway.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
v2->v3:
1.remove useless TESTFILE and MNTPINT macro
2.like statx10.c, test not filled situation when not request STATX_DIOLAIGN
3.add commet that this case is tightly coupled to the kernel's current DIO restrictions on block
devices
 runtest/syscalls                           |   1 +
 testcases/kernel/syscalls/statx/.gitignore |   1 +
 testcases/kernel/syscalls/statx/statx11.c  | 107 +++++++++++++++++++++
 3 files changed, 109 insertions(+)
 create mode 100644 testcases/kernel/syscalls/statx/statx11.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 92123772c..de5f0be35 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1770,6 +1770,7 @@ statx07 statx07
 statx08 statx08
 statx09 statx09
 statx10 statx10
+statx11 statx11
 
 membarrier01 membarrier01
 
diff --git a/testcases/kernel/syscalls/statx/.gitignore b/testcases/kernel/syscalls/statx/.gitignore
index 67341ff2d..48ac4078b 100644
--- a/testcases/kernel/syscalls/statx/.gitignore
+++ b/testcases/kernel/syscalls/statx/.gitignore
@@ -8,3 +8,4 @@
 /statx08
 /statx09
 /statx10
+/statx11
diff --git a/testcases/kernel/syscalls/statx/statx11.c b/testcases/kernel/syscalls/statx/statx11.c
new file mode 100644
index 000000000..35d6fbaf3
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx11.c
@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * It is a basic test for STATX_DIOALIGN mask on block device.
+ *
+ * - STATX_DIOALIGN   Want stx_dio_mem_align and stx_dio_offset_align value
+ *
+ * These two values are only filled when STATX_DIOALIGN in the request mask.
+ * These two values are tightly coupled to the kernel's current DIO
+ * restrictions on block devices.
+ *
+ * Minimum Linux version required is v6.1.
+ */
+
+#define _GNU_SOURCE
+#include <sys/types.h>
+#include <sys/ioctl.h>
+#include <sys/mount.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include "tst_test.h"
+#include "lapi/stat.h"
+
+#ifdef HAVE_STRUCT_STATX_STX_DIO_MEM_ALIGN
+static int fd = -1, logical_sector_size;
+static char sys_bdev_dma_path[1024], sys_bdev_lgs_path[1024];
+
+static void verify_statx(void)
+{
+	struct statx buf;
+
+	memset(&buf, 0, sizeof(buf));
+	TST_EXP_PASS(statx(AT_FDCWD, tst_device->dev, 0, STATX_DIOALIGN, &buf),
+		"statx(AT_FDCWD, %s, 0, STATX_DIOALIGN, &buf)", tst_device->dev);
+
+	if (!(buf.stx_mask & STATX_DIOALIGN)) {
+		tst_res(TCONF, "STATX_DIOALIGN is not supported until linux 6.1");
+		return;
+	}
+
+	/*
+	 * This test is tightly coupled to the kernel's current DIO restrictions
+	 * on block devices. The general rule of DIO needing to be aligned to the
+	 * block device's logical block size was recently relaxed to allow user buffers
+	 * (but not file offsets) aligned to the DMA alignment instead. See v6.0
+	 * commit bf8d08532bc1 ("iomap: add support for dma aligned direct-io") and
+	 * they are subject to further change in the future.
+	 * Also can see commit 2d985f8c6b9 ("vfs: support STATX_DIOALIGN on block devices).
+	 */
+	TST_ASSERT_ULONG(sys_bdev_dma_path, buf.stx_dio_mem_align - 1);
+	TST_ASSERT_ULONG(sys_bdev_lgs_path, buf.stx_dio_offset_align);
+	TST_EXP_EQ_LU(buf.stx_dio_offset_align, logical_sector_size);
+
+	TST_EXP_PASS(statx(AT_FDCWD, tst_device->dev, 0, 0, &buf),
+		"statx(AT_FDCWD, %s, 0, STATX_DIOALIGN, &buf)", tst_device->dev);
+	TST_EXP_EQ_LU(buf.stx_dio_mem_align, 0);
+	TST_EXP_EQ_LU(buf.stx_dio_offset_align, 0);
+}
+
+static void setup(void)
+{
+	char *dev_name;
+	int dev_fd;
+
+	dev_fd = SAFE_OPEN(tst_device->dev, O_RDWR);
+	SAFE_IOCTL(dev_fd, BLKSSZGET, &logical_sector_size);
+	SAFE_CLOSE(dev_fd);
+
+	if (logical_sector_size <= 0)
+		tst_brk(TBROK, "BLKSSZGET returned invalid block size %i", logical_sector_size);
+
+	dev_name = basename((char *)tst_device->dev);
+	sprintf(sys_bdev_lgs_path, "/sys/block/%s/queue/logical_block_size", dev_name);
+	while (access(sys_bdev_lgs_path, F_OK) != 0) {
+		dev_name[strlen(dev_name)-1] = '\0';
+		sprintf(sys_bdev_lgs_path, "/sys/block/%s/queue/logical_block_size", dev_name);
+	}
+
+	sprintf(sys_bdev_dma_path, "/sys/block/%s/queue/dma_alignment", dev_name);
+	if (access(sys_bdev_dma_path, F_OK) != 0)
+		tst_brk(TCONF, "dma_alignment syfsfile doesn't exist");
+}
+
+static void cleanup(void)
+{
+	if (fd > -1)
+		SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.test_all = verify_statx,
+	.setup = setup,
+	.cleanup = cleanup,
+	.needs_root = 1,
+	.needs_device = 1,
+};
+#else
+TST_TEST_TCONF("test requires struct statx to have the stx_dio_mem_align fields");
+#endif
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3 4/4] lapi/stat.h: Remove deprecated STATX_ALL macro

[Yang Xu]

Since kernel 5.10-rc1 commit 581701b7efd6 ("uapi: deprecate STATX_ALL"),
this flag has been mark as deprecated.

Kernel should keep this macro for compatibility, but ltp doesn't think
about it. So remove it.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 include/lapi/stat.h                       | 4 ----
 testcases/kernel/syscalls/statx/statx06.c | 4 ++--
 testcases/kernel/syscalls/statx/statx07.c | 6 +++---
 3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/include/lapi/stat.h b/include/lapi/stat.h
index c2db8a589..7c9a7a00c 100644
--- a/include/lapi/stat.h
+++ b/include/lapi/stat.h
@@ -188,10 +188,6 @@ static inline int statx(int dirfd, const char *pathname, unsigned int flags,
 # define STATX_DIOALIGN		0x00002000U
 #endif
 
-#ifndef STATX_ALL
-# define STATX_ALL		0x00000fffU
-#endif
-
 #ifndef STATX__RESERVED
 # define STATX__RESERVED	0x80000000U
 #endif
diff --git a/testcases/kernel/syscalls/statx/statx06.c b/testcases/kernel/syscalls/statx/statx06.c
index ce82b905b..60e736c5a 100644
--- a/testcases/kernel/syscalls/statx/statx06.c
+++ b/testcases/kernel/syscalls/statx/statx06.c
@@ -111,10 +111,10 @@ static void test_statx(unsigned int test_nr)
 	clock_wait_tick();
 	SAFE_CLOCK_GETTIME(CLOCK_REALTIME_COARSE, &after_time);
 
-	TEST(statx(AT_FDCWD, TEST_FILE, 0, STATX_ALL, &buff));
+	TEST(statx(AT_FDCWD, TEST_FILE, 0, STATX_BASIC_STATS | STATX_BTIME, &buff));
 	if (TST_RET != 0) {
 		tst_brk(TFAIL | TTERRNO,
-			"statx(AT_FDCWD, %s, 0, STATX_ALL, &buff)",
+			"statx(AT_FDCWD, %s, 0, STATX_BASIC_STATS | STATX_BTIME, &buff)",
 			TEST_FILE);
 	}
 
diff --git a/testcases/kernel/syscalls/statx/statx07.c b/testcases/kernel/syscalls/statx/statx07.c
index b13c11f72..c798c7a10 100644
--- a/testcases/kernel/syscalls/statx/statx07.c
+++ b/testcases/kernel/syscalls/statx/statx07.c
@@ -62,15 +62,15 @@ static int get_mode(char *file_name, int flag_type, char *flag_name)
 {
 	struct statx buf;
 
-	TEST(statx(AT_FDCWD, file_name, flag_type, STATX_ALL, &buf));
+	TEST(statx(AT_FDCWD, file_name, flag_type, STATX_BASIC_STATS | STATX_BTIME, &buf));
 
 	if (TST_RET == -1) {
 		tst_brk(TFAIL | TST_ERR,
-			"statx(AT_FDCWD, %s, %s, STATX_ALL, &buf)",
+			"statx(AT_FDCWD, %s, %s, STATX_BASIC_STATS | STATX_BTIME, &buf)",
 			file_name, flag_name);
 	}
 
-	tst_res(TINFO, "statx(AT_FDCWD, %s, %s, STATX_ALL, &buf) = %o",
+	tst_res(TINFO, "statx(AT_FDCWD, %s, %s, STATX_BASIC_STATS | STATX_BTIME, &buf) = %o",
 		file_name, flag_name, buf.stx_mode);
 
 	return buf.stx_mode;
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 2/4] syscalls/statx10: Add basic test for STATX_DIOALIGN on regular file

[Eric Biggers]

On Tue, Apr 04, 2023 at 03:30:28PM +0800, Yang Xu wrote:
> On ext4, files that use certain filesystem features (data journalling,
> encryption, and verity) fall back to buffered I/O. But ltp doesn't use
> these features by default, So I think dio should not fall back to
> buffered I/O.

Please document this in the code itself.

> +static void verify_statx(void)
> +{
> +	struct statx buf;
> +
> +	memset(&buf, 0, sizeof(buf));

There is no need for this memset to 0.

> +	if (buf.stx_dio_mem_align != 0)
> +		tst_res(TPASS, "stx_dio_mem_align:%u", buf.stx_dio_mem_align);
> +	else
> +		tst_res(TFAIL, "don't get stx_dio_mem_align on supported dio fs");
> +
> +	if (buf.stx_dio_offset_align != 0)
> +		tst_res(TPASS, "stx_dio_offset_align:%u", buf.stx_dio_offset_align);
> +	else
> +		tst_res(TFAIL, "don't get stx_dio_offset_align on supported dio fs");

Please leave a space between : and %u.

> +	if ((buf.stx_mask & STATX_DIOALIGN)) {

Unnecessary parentheses

> +		tst_res(TFAIL, "STATX_DIOALIGN mask return even not request");
> +		return;
> +	}

This part is not a valid test.  Please see the statx(2) manual page:

       "It should be noted that the kernel may return fields that  weren't  re‐
       quested and may fail to return fields that were requested, depending on
       what the backing filesystem supports.  (Fields that  are  given  values
       despite  being  unrequested  can  just  be  ignored.)   In either case,
       stx_mask will not be equal mask."

> +static void setup(void)
> +{
> +	if (strcmp(tst_device->fs_type, "xfs") && strcmp(tst_device->fs_type, "ext4"))
> +		tst_brk(TCONF, "This test only supports ext4 and xfs");
> +
> +	SAFE_FILE_PRINTF(TESTFILE, "AAAA");
> +	fd = open(TESTFILE, O_RDWR | O_DIRECT);
> +	if (fd == -1 && errno == EINVAL)
> +		tst_brk(TCONF, "The regular file is not on a filesystem that support DIO");
> +}
> +
> +static void cleanup(void)
> +{
> +	if (fd > -1)
> +		SAFE_CLOSE(fd);
> +}

Shouldn't 'fd' just be a local variable in setup()?

> +#else
> +TST_TEST_TCONF("test requires struct statx to have the stx_dio_mem_align fields");
> +#endif

LTP already has its own definition of struct statx in include/lapi/stat.h.  So,
why is it necessary to skip this test if the system headers lack an up-to-date
definition of struct statx?

- Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 3/4] syscalls/statx11: Add basic test for STATX_DIOALIGN on block device

[Eric Biggers]

Hi Yang,

On Tue, Apr 04, 2023 at 03:30:29PM +0800, Yang Xu wrote:
> +	/*
> +	 * This test is tightly coupled to the kernel's current DIO restrictions
> +	 * on block devices. The general rule of DIO needing to be aligned to the
> +	 * block device's logical block size was recently relaxed to allow user buffers

Please don't use the word "recently" in code comments like this.  It is vague,
and what is "recent" now will no longer be recent in the future.

> +
> +	TST_EXP_PASS(statx(AT_FDCWD, tst_device->dev, 0, 0, &buf),
> +		"statx(AT_FDCWD, %s, 0, STATX_DIOALIGN, &buf)", tst_device->dev);
> +	TST_EXP_EQ_LU(buf.stx_dio_mem_align, 0);
> +	TST_EXP_EQ_LU(buf.stx_dio_offset_align, 0);

Like I mentioned on patch 2, this is not a valid test case because the contract
of statx() allows it to return information that wasn't explicitly requested.

> +static void setup(void)
> +{
> +	char *dev_name;
> +	int dev_fd;
> +
> +	dev_fd = SAFE_OPEN(tst_device->dev, O_RDWR);
> +	SAFE_IOCTL(dev_fd, BLKSSZGET, &logical_sector_size);
> +	SAFE_CLOSE(dev_fd);
> +
> +	if (logical_sector_size <= 0)
> +		tst_brk(TBROK, "BLKSSZGET returned invalid block size %i", logical_sector_size);
> +
> +	dev_name = basename((char *)tst_device->dev);
> +	sprintf(sys_bdev_lgs_path, "/sys/block/%s/queue/logical_block_size", dev_name);
> +	while (access(sys_bdev_lgs_path, F_OK) != 0) {
> +		dev_name[strlen(dev_name)-1] = '\0';
> +		sprintf(sys_bdev_lgs_path, "/sys/block/%s/queue/logical_block_size", dev_name);
> +	}

What does "lgs" stand for?

Why are both BLKSSZGET and /sys/block/%s/queue/logical_block_size being used?
Don't they provide exactly the same information?

> +	if (access(sys_bdev_dma_path, F_OK) != 0)
> +		tst_brk(TCONF, "dma_alignment syfsfile doesn't exist");
> +}

syfsfile => sysfs file

> +static void cleanup(void)
> +{
> +	if (fd > -1)
> +		SAFE_CLOSE(fd);
> +}

What is the purpose of the 'fd' variable?

> +static struct tst_test test = {
> +	.test_all = verify_statx,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.needs_root = 1,
> +	.needs_device = 1,
> +};

Why does this test need root?

- Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 2/4] syscalls/statx10: Add basic test for STATX_DIOALIGN on regular file

[xuyang2018.jy]

on 2023/04/05 5:52, Eric Biggers wrote:
> On Tue, Apr 04, 2023 at 03:30:28PM +0800, Yang Xu wrote:
>> On ext4, files that use certain filesystem features (data journalling,
>> encryption, and verity) fall back to buffered I/O. But ltp doesn't use
>> these features by default, So I think dio should not fall back to
>> buffered I/O.
> 
> Please document this in the code itself.

OK.

> 
>> +static void verify_statx(void)
>> +{
>> +	struct statx buf;
>> +
>> +	memset(&buf, 0, sizeof(buf));
> 
> There is no need for this memset to 0.

YES.
> 
>> +	if (buf.stx_dio_mem_align != 0)
>> +		tst_res(TPASS, "stx_dio_mem_align:%u", buf.stx_dio_mem_align);
>> +	else
>> +		tst_res(TFAIL, "don't get stx_dio_mem_align on supported dio fs");
>> +
>> +	if (buf.stx_dio_offset_align != 0)
>> +		tst_res(TPASS, "stx_dio_offset_align:%u", buf.stx_dio_offset_align);
>> +	else
>> +		tst_res(TFAIL, "don't get stx_dio_offset_align on supported dio fs");
> 
> Please leave a space between : and %u.

Good catch.
> 
>> +	if ((buf.stx_mask & STATX_DIOALIGN)) {
> 
> Unnecessary parentheses

Yes,
> 
>> +		tst_res(TFAIL, "STATX_DIOALIGN mask return even not request");
>> +		return;
>> +	}
> 
> This part is not a valid test.  Please see the statx(2) manual page:
> 
>         "It should be noted that the kernel may return fields that  weren't  re‐
>         quested and may fail to return fields that were requested, depending on
>         what the backing filesystem supports.  (Fields that  are  given  values
>         despite  being  unrequested  can  just  be  ignored.)   In either case,
>         stx_mask will not be equal mask."

OK. Will remove.
> 
>> +static void setup(void)
>> +{
>> +	if (strcmp(tst_device->fs_type, "xfs") && strcmp(tst_device->fs_type, "ext4"))
>> +		tst_brk(TCONF, "This test only supports ext4 and xfs");
>> +
>> +	SAFE_FILE_PRINTF(TESTFILE, "AAAA");
>> +	fd = open(TESTFILE, O_RDWR | O_DIRECT);
>> +	if (fd == -1 && errno == EINVAL)
>> +		tst_brk(TCONF, "The regular file is not on a filesystem that support DIO");
>> +}
>> +
>> +static void cleanup(void)
>> +{
>> +	if (fd > -1)
>> +		SAFE_CLOSE(fd);
>> +}
> 
> Shouldn't 'fd' just be a local variable in setup()?

Yes.
> 
>> +#else
>> +TST_TEST_TCONF("test requires struct statx to have the stx_dio_mem_align fields");
>> +#endif
> 
> LTP already has its own definition of struct statx in include/lapi/stat.h.  So,
> why is it necessary to skip this test if the system headers lack an up-to-date
> definition of struct statx?
In actually, ltp own statx definition as below:
#if defined(HAVE_STRUCT_STATX)
#include <sys/stat.h>
#else
struct statx {
         /* 0x00 */
         uint32_t        stx_mask;
         uint32_t        stx_blksize;
         uint64_t        stx_attributes;

So we only use ltp own statx struct when system header file sys/statx.h 
doesn't have statx struct. If people use old system header file, it 
still will report non defined stx_dio_mem_align or stx_dio_offset_align.

Best Regards
Yang Xu
> 
> - Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 3/4] syscalls/statx11: Add basic test for STATX_DIOALIGN on block device

[xuyang2018.jy]

on 2023/04/05 5:59, Eric Biggers wrote:

> Hi Yang,
> 
> On Tue, Apr 04, 2023 at 03:30:29PM +0800, Yang Xu wrote:
>> +	/*
>> +	 * This test is tightly coupled to the kernel's current DIO restrictions
>> +	 * on block devices. The general rule of DIO needing to be aligned to the
>> +	 * block device's logical block size was recently relaxed to allow user buffers
> 
> Please don't use the word "recently" in code comments like this.  It is vague,
> and what is "recent" now will no longer be recent in the future.

OK.
> 
>> +
>> +	TST_EXP_PASS(statx(AT_FDCWD, tst_device->dev, 0, 0, &buf),
>> +		"statx(AT_FDCWD, %s, 0, STATX_DIOALIGN, &buf)", tst_device->dev);
>> +	TST_EXP_EQ_LU(buf.stx_dio_mem_align, 0);
>> +	TST_EXP_EQ_LU(buf.stx_dio_offset_align, 0);
> 
> Like I mentioned on patch 2, this is not a valid test case because the contract
> of statx() allows it to return information that wasn't explicitly requested.

OK. Will remove.
> 
>> +static void setup(void)
>> +{
>> +	char *dev_name;
>> +	int dev_fd;
>> +
>> +	dev_fd = SAFE_OPEN(tst_device->dev, O_RDWR);
>> +	SAFE_IOCTL(dev_fd, BLKSSZGET, &logical_sector_size);
>> +	SAFE_CLOSE(dev_fd);
>> +
>> +	if (logical_sector_size <= 0)
>> +		tst_brk(TBROK, "BLKSSZGET returned invalid block size %i", logical_sector_size);
>> +
>> +	dev_name = basename((char *)tst_device->dev);
>> +	sprintf(sys_bdev_lgs_path, "/sys/block/%s/queue/logical_block_size", dev_name);
>> +	while (access(sys_bdev_lgs_path, F_OK) != 0) {
>> +		dev_name[strlen(dev_name)-1] = '\0';
>> +		sprintf(sys_bdev_lgs_path, "/sys/block/%s/queue/logical_block_size", dev_name);
>> +	}
> 
> What does "lgs" stand for?

lgs->logical_block_size, will use more meaningful variable name.

> 
> Why are both BLKSSZGET and /sys/block/%s/queue/logical_block_size being used?
> Don't they provide exactly the same information?

Yes, they provide same info, I will only test for sys file instead of ioctl.
> 
>> +	if (access(sys_bdev_dma_path, F_OK) != 0)
>> +		tst_brk(TCONF, "dma_alignment syfsfile doesn't exist");
>> +}
> 
> syfsfile => sysfs file

Good catch.
> 
>> +static void cleanup(void)
>> +{
>> +	if (fd > -1)
>> +		SAFE_CLOSE(fd);
>> +}
> 
> What is the purpose of the 'fd' variable?

Sorry, I copy code from statx10.c, will remove.
> 
>> +static struct tst_test test = {
>> +	.test_all = verify_statx,
>> +	.setup = setup,
>> +	.cleanup = cleanup,
>> +	.needs_root = 1,
>> +	.needs_device = 1,
>> +};
> 
> Why does this test need root?

I remember I have removed this, will remove.


Best Regards
Yang Xu
> 
> - Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3 3/4] syscalls/statx11: Add basic test for STATX_DIOALIGN on block device

[xuyang2018.jy]

Hi Eric>
> 
> on 2023/04/05 5:59, Eric Biggers wrote:
> 
>> Hi Yang,
>>
>> On Tue, Apr 04, 2023 at 03:30:29PM +0800, Yang Xu wrote:
>>> +	/*
>>> +	 * This test is tightly coupled to the kernel's current DIO restrictions
>>> +	 * on block devices. The general rule of DIO needing to be aligned to the
>>> +	 * block device's logical block size was recently relaxed to allow user buffers
>>
>> Please don't use the word "recently" in code comments like this.  It is vague,
>> and what is "recent" now will no longer be recent in the future.
> 
> OK.
>>
>>> +
>>> +	TST_EXP_PASS(statx(AT_FDCWD, tst_device->dev, 0, 0, &buf),
>>> +		"statx(AT_FDCWD, %s, 0, STATX_DIOALIGN, &buf)", tst_device->dev);
>>> +	TST_EXP_EQ_LU(buf.stx_dio_mem_align, 0);
>>> +	TST_EXP_EQ_LU(buf.stx_dio_offset_align, 0);
>>
>> Like I mentioned on patch 2, this is not a valid test case because the contract
>> of statx() allows it to return information that wasn't explicitly requested.
> 
> OK. Will remove.
>>
>>> +static void setup(void)
>>> +{
>>> +	char *dev_name;
>>> +	int dev_fd;
>>> +
>>> +	dev_fd = SAFE_OPEN(tst_device->dev, O_RDWR);
>>> +	SAFE_IOCTL(dev_fd, BLKSSZGET, &logical_sector_size);
>>> +	SAFE_CLOSE(dev_fd);
>>> +
>>> +	if (logical_sector_size <= 0)
>>> +		tst_brk(TBROK, "BLKSSZGET returned invalid block size %i", logical_sector_size);
>>> +
>>> +	dev_name = basename((char *)tst_device->dev);
>>> +	sprintf(sys_bdev_lgs_path, "/sys/block/%s/queue/logical_block_size", dev_name);
>>> +	while (access(sys_bdev_lgs_path, F_OK) != 0) {
>>> +		dev_name[strlen(dev_name)-1] = '\0';
>>> +		sprintf(sys_bdev_lgs_path, "/sys/block/%s/queue/logical_block_size", dev_name);
>>> +	}
>>
>> What does "lgs" stand for?
> 
> lgs->logical_block_size, will use more meaningful variable name.
> 
>>
>> Why are both BLKSSZGET and /sys/block/%s/queue/logical_block_size being used?
>> Don't they provide exactly the same information?
> 
> Yes, they provide same info, I will only test for sys file instead of ioctl.
>>
>>> +	if (access(sys_bdev_dma_path, F_OK) != 0)
>>> +		tst_brk(TCONF, "dma_alignment syfsfile doesn't exist");
>>> +}
>>
>> syfsfile => sysfs file
> 
> Good catch.
>>
>>> +static void cleanup(void)
>>> +{
>>> +	if (fd > -1)
>>> +		SAFE_CLOSE(fd);
>>> +}
>>
>> What is the purpose of the 'fd' variable?
> 
> Sorry, I copy code from statx10.c, will remove.
>>
>>> +static struct tst_test test = {
>>> +	.test_all = verify_statx,
>>> +	.setup = setup,
>>> +	.cleanup = cleanup,
>>> +	.needs_root = 1,
>>> +	.needs_device = 1,
>>> +};
>>
>> Why does this test need root?
> 
> I remember I have removed this, will remove.

I almost forgot that /dev/loop-control needs root, otherwise will meet 
EACCES error

tst_device.c:108: TINFO: Not allowed to open /dev/loop-control. Are you 
root?: EACCES (13)
tst_device.c:147: TINFO: No free devices found
tst_device.c:354: TBROK: Failed to acquire device

Best Regards
Yang Xu
> 
> 
> Best Regards
> Yang Xu
>>
>> - Eric
> 

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v4 1/4] lapi/stat.h: Add STATX_DIOALIGN related definition

[Yang Xu]

Also add missing stx_mnt_id.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 include/lapi/stat.h | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/lapi/stat.h b/include/lapi/stat.h
index ce1f2b678..c2db8a589 100644
--- a/include/lapi/stat.h
+++ b/include/lapi/stat.h
@@ -97,7 +97,11 @@ struct statx {
 	uint32_t	stx_dev_major;
 	uint32_t	stx_dev_minor;
 	/* 0x90 */
-	uint64_t	__spare2[14];
+	uint64_t	stx_mnt_id;
+	uint32_t	stx_dio_mem_align;
+	uint32_t	stx_dio_offset_align;
+	/* 0xa0 */
+	uint64_t	__spare1[12];
 	/* 0x100 */
 };
 #endif
@@ -180,6 +184,10 @@ static inline int statx(int dirfd, const char *pathname, unsigned int flags,
 # define STATX_MNT_ID		0x00001000U
 #endif
 
+#ifndef STATX_DIOALIGN
+# define STATX_DIOALIGN		0x00002000U
+#endif
+
 #ifndef STATX_ALL
 # define STATX_ALL		0x00000fffU
 #endif
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v4 2/4] syscalls/statx10: Add basic test for STATX_DIOALIGN on regular file

[Yang Xu]

STATX_DIOALIGN is used to get stx_dio_mem_align and stx_dio_offset_align
for files on fs that support direct io. We just check whether these
value are nonzero on ext4 and xfs.

On ext4, files that use certain filesystem features (data journalling,
encryption, and verity) fall back to buffered I/O. But ltp doesn't use
these features by default, So I think dio should not fall back to
buffered I/O.

For struct statx member check, we only check stx_dio_mem_align because
these two member is introduced toger in separate commit in kernel, so it is
safe.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 configure.ac                               |  2 +-
 runtest/syscalls                           |  1 +
 testcases/kernel/syscalls/statx/.gitignore |  1 +
 testcases/kernel/syscalls/statx/statx10.c  | 84 ++++++++++++++++++++++
 4 files changed, 87 insertions(+), 1 deletion(-)
 create mode 100644 testcases/kernel/syscalls/statx/statx10.c

diff --git a/configure.ac b/configure.ac
index 4c8763376..548288310 100644
--- a/configure.ac
+++ b/configure.ac
@@ -158,7 +158,7 @@ AC_CHECK_FUNCS(mkdtemp,[],AC_MSG_ERROR(mkdtemp() not found!))
 AC_CHECK_MEMBERS([struct fanotify_event_info_fid.fsid.__val],,,[#include <sys/fanotify.h>])
 AC_CHECK_MEMBERS([struct perf_event_mmap_page.aux_head],,,[#include <linux/perf_event.h>])
 AC_CHECK_MEMBERS([struct sigaction.sa_sigaction],[],[],[#include <signal.h>])
-AC_CHECK_MEMBERS([struct statx.stx_mnt_id],,,[
+AC_CHECK_MEMBERS([struct statx.stx_mnt_id, struct statx.stx_dio_mem_align],,,[
 #define _GNU_SOURCE
 #include <sys/stat.h>
 ])
diff --git a/runtest/syscalls b/runtest/syscalls
index 8b002e989..92123772c 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1769,6 +1769,7 @@ statx06 statx06
 statx07 statx07
 statx08 statx08
 statx09 statx09
+statx10 statx10
 
 membarrier01 membarrier01
 
diff --git a/testcases/kernel/syscalls/statx/.gitignore b/testcases/kernel/syscalls/statx/.gitignore
index 1cea43c0d..67341ff2d 100644
--- a/testcases/kernel/syscalls/statx/.gitignore
+++ b/testcases/kernel/syscalls/statx/.gitignore
@@ -7,3 +7,4 @@
 /statx07
 /statx08
 /statx09
+/statx10
diff --git a/testcases/kernel/syscalls/statx/statx10.c b/testcases/kernel/syscalls/statx/statx10.c
new file mode 100644
index 000000000..59e0c06e4
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx10.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * It is a basic test for STATX_DIOALIGN mask on ext4 and xfs filesystem.
+ *
+ * - STATX_DIOALIGN   Want stx_dio_mem_align and stx_dio_offset_align value
+ *
+ * Check these two values are nonzero under dio situation when STATX_DIOALIGN
+ * in the request mask.
+ *
+ * On ext4, files that use certain filesystem features (data journaling,
+ * encryption, and verity) fall back to buffered I/O. But ltp doesn't use these
+ * features by default, So I think dio should not fall back to buffered I/O.
+ *
+ * Minimum Linux version required is v6.1.
+ */
+
+#define _GNU_SOURCE
+#include <sys/types.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include "tst_test.h"
+#include "lapi/stat.h"
+
+#define MNTPOINT "mnt_point"
+#define TESTFILE MNTPOINT"/testfile"
+
+static void verify_statx(void)
+{
+	struct statx buf;
+
+	TST_EXP_PASS_SILENT(statx(AT_FDCWD, TESTFILE, 0, STATX_DIOALIGN, &buf),
+		"statx(AT_FDCWD, %s, 0, STATX_DIOALIGN, &buf)", TESTFILE);
+
+	if (!(buf.stx_mask & STATX_DIOALIGN)) {
+		tst_res(TCONF, "STATX_DIOALIGN is not supported until linux 6.1");
+		return;
+	}
+
+#ifdef HAVE_STRUCT_STATX_STX_DIO_MEM_ALIGN
+	if (buf.stx_dio_mem_align != 0)
+		tst_res(TPASS, "stx_dio_mem_align: %u", buf.stx_dio_mem_align);
+	else
+		tst_res(TFAIL, "don't get stx_dio_mem_align on supported dio fs");
+
+	if (buf.stx_dio_offset_align != 0)
+		tst_res(TPASS, "stx_dio_offset_align: %u", buf.stx_dio_offset_align);
+	else
+		tst_res(TFAIL, "don't get stx_dio_offset_align on supported dio fs");
+#endif
+}
+
+static void setup(void)
+{
+	int fd = -1;
+
+	if (strcmp(tst_device->fs_type, "xfs") && strcmp(tst_device->fs_type, "ext4"))
+		tst_brk(TCONF, "This test only supports ext4 and xfs");
+
+	SAFE_FILE_PRINTF(TESTFILE, "AAAA");
+	fd = open(TESTFILE, O_RDWR | O_DIRECT);
+	if (fd == -1 && errno == EINVAL) {
+		SAFE_CLOSE(fd);
+		tst_brk(TCONF, "The regular file is not on a filesystem that support DIO");
+	}
+	SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.test_all = verify_statx,
+	.setup = setup,
+	.needs_root = 1,
+	.mntpoint = MNTPOINT,
+	.mount_device = 1,
+	.all_filesystems = 1,
+};
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v4 3/4] syscalls/statx11: Add basic test for STATX_DIOALIGN on block device

[Yang Xu]

Since STATX_DIOLAIGN is only supported on regular file and block device,
so this case is used to test the latter.

This test is tightly coupled to the kernel's current DIO restrictions on block
devices.  These changed in v6.0, and they are subject to further change in the
future.

It is fine for now because STATX_DIOALIGN is only in v6.1 and later
anyway.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 runtest/syscalls                           |  1 +
 testcases/kernel/syscalls/statx/.gitignore |  1 +
 testcases/kernel/syscalls/statx/statx11.c  | 81 ++++++++++++++++++++++
 3 files changed, 83 insertions(+)
 create mode 100644 testcases/kernel/syscalls/statx/statx11.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 92123772c..de5f0be35 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1770,6 +1770,7 @@ statx07 statx07
 statx08 statx08
 statx09 statx09
 statx10 statx10
+statx11 statx11
 
 membarrier01 membarrier01
 
diff --git a/testcases/kernel/syscalls/statx/.gitignore b/testcases/kernel/syscalls/statx/.gitignore
index 67341ff2d..48ac4078b 100644
--- a/testcases/kernel/syscalls/statx/.gitignore
+++ b/testcases/kernel/syscalls/statx/.gitignore
@@ -8,3 +8,4 @@
 /statx08
 /statx09
 /statx10
+/statx11
diff --git a/testcases/kernel/syscalls/statx/statx11.c b/testcases/kernel/syscalls/statx/statx11.c
new file mode 100644
index 000000000..02449888a
--- /dev/null
+++ b/testcases/kernel/syscalls/statx/statx11.c
@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@fujitsu.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * It is a basic test for STATX_DIOALIGN mask on block device.
+ *
+ * - STATX_DIOALIGN   Want stx_dio_mem_align and stx_dio_offset_align value
+ *
+ * These two values are tightly coupled to the kernel's current DIO
+ * restrictions on block devices.
+ *
+ * Minimum Linux version required is v6.1.
+ */
+
+#define _GNU_SOURCE
+#include <sys/types.h>
+#include <sys/mount.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include "tst_test.h"
+#include "lapi/stat.h"
+
+static char sys_bdev_dma_path[1024], sys_bdev_logical_path[1024];
+
+static void verify_statx(void)
+{
+	struct statx buf;
+
+	memset(&buf, 0, sizeof(buf));
+	TST_EXP_PASS_SILENT(statx(AT_FDCWD, tst_device->dev, 0, STATX_DIOALIGN, &buf),
+		"statx(AT_FDCWD, %s, 0, STATX_DIOALIGN, &buf)", tst_device->dev);
+
+	if (!(buf.stx_mask & STATX_DIOALIGN)) {
+		tst_res(TCONF, "STATX_DIOALIGN is not supported until linux 6.1");
+		return;
+	}
+
+#ifdef HAVE_STRUCT_STATX_STX_DIO_MEM_ALIGN
+	/*
+	 * This test is tightly coupled to the kernel's current DIO restrictions
+	 * on block devices. The general rule of DIO needing to be aligned to the
+	 * block device's logical block size was relaxed to allow user buffers
+	 * (but not file offsets) aligned to the DMA alignment instead. See v6.0
+	 * commit bf8d08532bc1 ("iomap: add support for dma aligned direct-io") and
+	 * they are subject to further change in the future.
+	 * Also can see commit 2d985f8c6b9 ("vfs: support STATX_DIOALIGN on block devices).
+	 */
+	TST_ASSERT_ULONG(sys_bdev_dma_path, buf.stx_dio_mem_align - 1);
+	TST_ASSERT_ULONG(sys_bdev_logical_path, buf.stx_dio_offset_align);
+#endif
+}
+
+static void setup(void)
+{
+	char *dev_name;
+
+	dev_name = basename((char *)tst_device->dev);
+	sprintf(sys_bdev_logical_path, "/sys/block/%s/queue/logical_block_size", dev_name);
+	while (access(sys_bdev_logical_path, F_OK) != 0) {
+		dev_name[strlen(dev_name)-1] = '\0';
+		sprintf(sys_bdev_logical_path, "/sys/block/%s/queue/logical_block_size", dev_name);
+	}
+
+	sprintf(sys_bdev_dma_path, "/sys/block/%s/queue/dma_alignment", dev_name);
+	if (access(sys_bdev_dma_path, F_OK) != 0)
+		tst_brk(TCONF, "dma_alignment sysfs file doesn't exist");
+}
+
+static struct tst_test test = {
+	.test_all = verify_statx,
+	.setup = setup,
+	.needs_device = 1,
+	.needs_root = 1,
+};
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v4 4/4] lapi/stat.h: Remove deprecated STATX_ALL macro

[Yang Xu]

Since kernel 5.10-rc1 commit 581701b7efd6 ("uapi: deprecate STATX_ALL"),
this flag has been mark as deprecated.

Kernel should keep this macro for compatibility, but ltp doesn't think
about it. So remove it.

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 include/lapi/stat.h                       | 4 ----
 testcases/kernel/syscalls/statx/statx06.c | 4 ++--
 testcases/kernel/syscalls/statx/statx07.c | 6 +++---
 3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/include/lapi/stat.h b/include/lapi/stat.h
index c2db8a589..7c9a7a00c 100644
--- a/include/lapi/stat.h
+++ b/include/lapi/stat.h
@@ -188,10 +188,6 @@ static inline int statx(int dirfd, const char *pathname, unsigned int flags,
 # define STATX_DIOALIGN		0x00002000U
 #endif
 
-#ifndef STATX_ALL
-# define STATX_ALL		0x00000fffU
-#endif
-
 #ifndef STATX__RESERVED
 # define STATX__RESERVED	0x80000000U
 #endif
diff --git a/testcases/kernel/syscalls/statx/statx06.c b/testcases/kernel/syscalls/statx/statx06.c
index ce82b905b..60e736c5a 100644
--- a/testcases/kernel/syscalls/statx/statx06.c
+++ b/testcases/kernel/syscalls/statx/statx06.c
@@ -111,10 +111,10 @@ static void test_statx(unsigned int test_nr)
 	clock_wait_tick();
 	SAFE_CLOCK_GETTIME(CLOCK_REALTIME_COARSE, &after_time);
 
-	TEST(statx(AT_FDCWD, TEST_FILE, 0, STATX_ALL, &buff));
+	TEST(statx(AT_FDCWD, TEST_FILE, 0, STATX_BASIC_STATS | STATX_BTIME, &buff));
 	if (TST_RET != 0) {
 		tst_brk(TFAIL | TTERRNO,
-			"statx(AT_FDCWD, %s, 0, STATX_ALL, &buff)",
+			"statx(AT_FDCWD, %s, 0, STATX_BASIC_STATS | STATX_BTIME, &buff)",
 			TEST_FILE);
 	}
 
diff --git a/testcases/kernel/syscalls/statx/statx07.c b/testcases/kernel/syscalls/statx/statx07.c
index b13c11f72..c798c7a10 100644
--- a/testcases/kernel/syscalls/statx/statx07.c
+++ b/testcases/kernel/syscalls/statx/statx07.c
@@ -62,15 +62,15 @@ static int get_mode(char *file_name, int flag_type, char *flag_name)
 {
 	struct statx buf;
 
-	TEST(statx(AT_FDCWD, file_name, flag_type, STATX_ALL, &buf));
+	TEST(statx(AT_FDCWD, file_name, flag_type, STATX_BASIC_STATS | STATX_BTIME, &buf));
 
 	if (TST_RET == -1) {
 		tst_brk(TFAIL | TST_ERR,
-			"statx(AT_FDCWD, %s, %s, STATX_ALL, &buf)",
+			"statx(AT_FDCWD, %s, %s, STATX_BASIC_STATS | STATX_BTIME, &buf)",
 			file_name, flag_name);
 	}
 
-	tst_res(TINFO, "statx(AT_FDCWD, %s, %s, STATX_ALL, &buf) = %o",
+	tst_res(TINFO, "statx(AT_FDCWD, %s, %s, STATX_BASIC_STATS | STATX_BTIME, &buf) = %o",
 		file_name, flag_name, buf.stx_mode);
 
 	return buf.stx_mode;
-- 
2.39.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 1/4] lapi/stat.h: Add STATX_DIOALIGN related definition

[Yang Xu (Fujitsu)]

Hi

Ping!

Best Regards
Yang Xu> Also add missing stx_mnt_id.
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>   include/lapi/stat.h | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/include/lapi/stat.h b/include/lapi/stat.h
> index ce1f2b678..c2db8a589 100644
> --- a/include/lapi/stat.h
> +++ b/include/lapi/stat.h
> @@ -97,7 +97,11 @@ struct statx {
>   	uint32_t	stx_dev_major;
>   	uint32_t	stx_dev_minor;
>   	/* 0x90 */
> -	uint64_t	__spare2[14];
> +	uint64_t	stx_mnt_id;
> +	uint32_t	stx_dio_mem_align;
> +	uint32_t	stx_dio_offset_align;
> +	/* 0xa0 */
> +	uint64_t	__spare1[12];
>   	/* 0x100 */
>   };
>   #endif
> @@ -180,6 +184,10 @@ static inline int statx(int dirfd, const char *pathname, unsigned int flags,
>   # define STATX_MNT_ID		0x00001000U
>   #endif
>   
> +#ifndef STATX_DIOALIGN
> +# define STATX_DIOALIGN		0x00002000U
> +#endif
> +
>   #ifndef STATX_ALL
>   # define STATX_ALL		0x00000fffU
>   #endif

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 1/4] lapi/stat.h: Add STATX_DIOALIGN related definition

[Eric Biggers]

On Thu, Apr 06, 2023 at 01:40:19PM +0800, Yang Xu wrote:
> Also add missing stx_mnt_id.
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
> ---
>  include/lapi/stat.h | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/include/lapi/stat.h b/include/lapi/stat.h
> index ce1f2b678..c2db8a589 100644
> --- a/include/lapi/stat.h
> +++ b/include/lapi/stat.h
> @@ -97,7 +97,11 @@ struct statx {
>  	uint32_t	stx_dev_major;
>  	uint32_t	stx_dev_minor;
>  	/* 0x90 */
> -	uint64_t	__spare2[14];
> +	uint64_t	stx_mnt_id;
> +	uint32_t	stx_dio_mem_align;
> +	uint32_t	stx_dio_offset_align;
> +	/* 0xa0 */
> +	uint64_t	__spare1[12];
>  	/* 0x100 */
>  };

Not like it matters, but the kernel header has __spare3, not __spare1.

- Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 4/4] lapi/stat.h: Remove deprecated STATX_ALL macro

[Eric Biggers]

On Thu, Apr 06, 2023 at 01:40:22PM +0800, Yang Xu wrote:
> diff --git a/testcases/kernel/syscalls/statx/statx07.c b/testcases/kernel/syscalls/statx/statx07.c
> index b13c11f72..c798c7a10 100644
> --- a/testcases/kernel/syscalls/statx/statx07.c
> +++ b/testcases/kernel/syscalls/statx/statx07.c
> @@ -62,15 +62,15 @@ static int get_mode(char *file_name, int flag_type, char *flag_name)
>  {
>  	struct statx buf;
>  
> -	TEST(statx(AT_FDCWD, file_name, flag_type, STATX_ALL, &buf));
> +	TEST(statx(AT_FDCWD, file_name, flag_type, STATX_BASIC_STATS | STATX_BTIME, &buf));
>  
>  	if (TST_RET == -1) {
>  		tst_brk(TFAIL | TST_ERR,
> -			"statx(AT_FDCWD, %s, %s, STATX_ALL, &buf)",
> +			"statx(AT_FDCWD, %s, %s, STATX_BASIC_STATS | STATX_BTIME, &buf)",
>  			file_name, flag_name);
>  	}
>  
> -	tst_res(TINFO, "statx(AT_FDCWD, %s, %s, STATX_ALL, &buf) = %o",
> +	tst_res(TINFO, "statx(AT_FDCWD, %s, %s, STATX_BASIC_STATS | STATX_BTIME, &buf) = %o",
>  		file_name, flag_name, buf.stx_mode);
>  
>  	return buf.stx_mode;

Looks like this place just wants STATX_BASIC_STATS.

- Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 2/4] syscalls/statx10: Add basic test for STATX_DIOALIGN on regular file

[Eric Biggers]

On Thu, Apr 06, 2023 at 01:40:20PM +0800, Yang Xu wrote:
> + * On ext4, files that use certain filesystem features (data journaling,
> + * encryption, and verity) fall back to buffered I/O. But ltp doesn't use these
> + * features by default, So I think dio should not fall back to buffered I/O.

Does LTP create and mount the filesystem itself?

If not, then it wouldn't have control over this.

> +	if (!(buf.stx_mask & STATX_DIOALIGN)) {
> +		tst_res(TCONF, "STATX_DIOALIGN is not supported until linux 6.1");
> +		return;
> +	}

"Filesystem does not support STATX_DIOALIGN"

> +
> +#ifdef HAVE_STRUCT_STATX_STX_DIO_MEM_ALIGN

This looks wrong.  If the system headers are missing this field, then the
definition in the LTP source tree should be used instead.

> +	if (buf.stx_dio_mem_align != 0)
> +		tst_res(TPASS, "stx_dio_mem_align: %u", buf.stx_dio_mem_align);
> +	else
> +		tst_res(TFAIL, "don't get stx_dio_mem_align on supported dio fs");

For the failure case: "stx_dio_mem_align was 0, but DIO should be supported"

> +
> +	if (buf.stx_dio_offset_align != 0)
> +		tst_res(TPASS, "stx_dio_offset_align: %u", buf.stx_dio_offset_align);
> +	else
> +		tst_res(TFAIL, "don't get stx_dio_offset_align on supported dio fs");
> +#endif

For the failure case: "stx_dio_offset_align was 0, but DIO should be supported"

> +	SAFE_FILE_PRINTF(TESTFILE, "AAAA");
> +	fd = open(TESTFILE, O_RDWR | O_DIRECT);
> +	if (fd == -1 && errno == EINVAL) {
> +		SAFE_CLOSE(fd);
> +		tst_brk(TCONF, "The regular file is not on a filesystem that support DIO");
> +	}
> +	SAFE_CLOSE(fd);

The open() is not checked for error in all cases.

Also, this is closing the file descriptor even when it is -1.

> +static struct tst_test test = {
> +	.test_all = verify_statx,
> +	.setup = setup,
> +	.needs_root = 1,

Why does this test need root?

- Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 3/4] syscalls/statx11: Add basic test for STATX_DIOALIGN on block device

[Eric Biggers]

On Thu, Apr 06, 2023 at 01:40:21PM +0800, Yang Xu wrote:
> +static void verify_statx(void)
> +{
> +	struct statx buf;
> +
> +	memset(&buf, 0, sizeof(buf));

It is not necessary to memset struct statx to 0 before calling statx().

> +#ifdef HAVE_STRUCT_STATX_STX_DIO_MEM_ALIGN

Again, this looks wrong.  If the system headers are outdated, then LTP should
use its in-tree header instead.

> +static void setup(void)
> +{
> +	char *dev_name;
> +
> +	dev_name = basename((char *)tst_device->dev);

This is modifying a const string, which seems problematic.

> +	sprintf(sys_bdev_logical_path, "/sys/block/%s/queue/logical_block_size", dev_name);
> +	while (access(sys_bdev_logical_path, F_OK) != 0) {
> +		dev_name[strlen(dev_name)-1] = '\0';
> +		sprintf(sys_bdev_logical_path, "/sys/block/%s/queue/logical_block_size", dev_name);
> +	}

What is this code doing?  Is it trying to strip off the partition number of the
block device name?  If so, it is incorrect because it assumes the partition
number is only 1 digit long, which is not guaranteed.

How about just using /sys/class/block/%s/queue, which works for partitions?

- Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 1/4] lapi/stat.h: Add STATX_DIOALIGN related definition

[Yang Xu (Fujitsu)]

on 2023/04/27 5:56, Eric Biggers wrote:
> On Thu, Apr 06, 2023 at 01:40:19PM +0800, Yang Xu wrote:
>> Also add missing stx_mnt_id.
>>
>> Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
>> ---
>>   include/lapi/stat.h | 10 +++++++++-
>>   1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/lapi/stat.h b/include/lapi/stat.h
>> index ce1f2b678..c2db8a589 100644
>> --- a/include/lapi/stat.h
>> +++ b/include/lapi/stat.h
>> @@ -97,7 +97,11 @@ struct statx {
>>   	uint32_t	stx_dev_major;
>>   	uint32_t	stx_dev_minor;
>>   	/* 0x90 */
>> -	uint64_t	__spare2[14];
>> +	uint64_t	stx_mnt_id;
>> +	uint32_t	stx_dio_mem_align;
>> +	uint32_t	stx_dio_offset_align;
>> +	/* 0xa0 */
>> +	uint64_t	__spare1[12];
>>   	/* 0x100 */
>>   };
> 
> Not like it matters, but the kernel header has __spare3, not __spare1.

Yes, I know this.

Sorry, I don't explain this reason for using _spare1[12] in commit message.

Looks the history of this struct in the kernel header.

Since kernel commit a528d35e ("statx: Add a system call to make enhanced 
file info available")[1], it introduced 
__spare0[1],__spare1[1],__spare2[14].


Then in kernel commit 3209f68 ("statx: Include a mask for stx_attributes 
in struct statx")[2], it uses stx_attributes_mask to replace __spare1[1],
so it leaves a gap.


After kernel commit fa2fcf4f1 ("statx: add mount ID")[3], it uses 
stx_mnit_id and _spare2 , _spare3[12] to replace _spare2[14].

Finally, in kernel commit 825cf206 ("statx: add direct I/O alignment 
information")[4], use  stx_dio_mem_align and stx_dio_offset_align to 
replace _spare2. It also leaves a gap.

That is why I use __spare1[12] instead of _spare3[12].


[1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/uapi/linux/stat.h?id=a528d35e8b
[2]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/uapi/linux/stat.h?id=3209f68b
[3]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/uapi/linux/stat.h?id=fa2fcf4f
[4]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/include/uapi/linux/stat.h?id=825cf206

Best Regards
Yang Xu

> 
> - Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 4/4] lapi/stat.h: Remove deprecated STATX_ALL macro

[Yang Xu (Fujitsu)]

on 2023/04/27 5:56, Eric Biggers wrote:
> On Thu, Apr 06, 2023 at 01:40:22PM +0800, Yang Xu wrote:
>> diff --git a/testcases/kernel/syscalls/statx/statx07.c b/testcases/kernel/syscalls/statx/statx07.c
>> index b13c11f72..c798c7a10 100644
>> --- a/testcases/kernel/syscalls/statx/statx07.c
>> +++ b/testcases/kernel/syscalls/statx/statx07.c
>> @@ -62,15 +62,15 @@ static int get_mode(char *file_name, int flag_type, char *flag_name)
>>   {
>>   	struct statx buf;
>>   
>> -	TEST(statx(AT_FDCWD, file_name, flag_type, STATX_ALL, &buf));
>> +	TEST(statx(AT_FDCWD, file_name, flag_type, STATX_BASIC_STATS | STATX_BTIME, &buf));
>>   
>>   	if (TST_RET == -1) {
>>   		tst_brk(TFAIL | TST_ERR,
>> -			"statx(AT_FDCWD, %s, %s, STATX_ALL, &buf)",
>> +			"statx(AT_FDCWD, %s, %s, STATX_BASIC_STATS | STATX_BTIME, &buf)",
>>   			file_name, flag_name);
>>   	}
>>   
>> -	tst_res(TINFO, "statx(AT_FDCWD, %s, %s, STATX_ALL, &buf) = %o",
>> +	tst_res(TINFO, "statx(AT_FDCWD, %s, %s, STATX_BASIC_STATS | STATX_BTIME, &buf) = %o",
>>   		file_name, flag_name, buf.stx_mode);
>>   
>>   	return buf.stx_mode;
> 
> Looks like this place just wants STATX_BASIC_STATS.

Yes, I will only use STATX_BASIC_STATS for statx07.c

Best Regards
Yang Xu
> 
> - Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 2/4] syscalls/statx10: Add basic test for STATX_DIOALIGN on regular file

[Yang Xu (Fujitsu)]

on 2023/04/27 6:06, Eric Biggers wrote:
> On Thu, Apr 06, 2023 at 01:40:20PM +0800, Yang Xu wrote:
>> + * On ext4, files that use certain filesystem features (data journaling,
>> + * encryption, and verity) fall back to buffered I/O. But ltp doesn't use these
>> + * features by default, So I think dio should not fall back to buffered I/O.
> 
> Does LTP create and mount the filesystem itself?

Yes, I have enabled mount_device in tst_test struct, mount_device usage 
you can see the following url.
https://github.com/linux-test-project/ltp/wiki/C-Test-API#115-testing-with-a-block-device

If we set block device to LTP_DEV environment, we use this block device 
to mount. Otherwise, use loop device to simuate it.


> 
> If not, then it wouldn't have control over this.
> 
>> +	if (!(buf.stx_mask & STATX_DIOALIGN)) {
>> +		tst_res(TCONF, "STATX_DIOALIGN is not supported until linux 6.1");
>> +		return;
>> +	}
> 
> "Filesystem does not support STATX_DIOALIGN"

OK.
> 
>> +
>> +#ifdef HAVE_STRUCT_STATX_STX_DIO_MEM_ALIGN
> 
> This looks wrong.  If the system headers are missing this field, then the
> definition in the LTP source tree should be used instead.

Yes, usually, if system headers miss this field, we should use ltp 
definition ie some macro.  But here it has a little difference, it is a 
member in a struct.

see include/lapi/stat.h

#if defined(HAVE_STRUCT_STATX)
#include <sys/stat.h>
#else
struct statx {
         /* 0x00 */
         uint32_t        stx_mask;
         uint32_t        stx_blksize;
         uint64_t        stx_attributes;
         /* 0x10 */
         uint32_t        stx_nlink;
         uint32_t        stx_uid;
         uint32_t        stx_gid;
         uint16_t        stx_mode;
         uint16_t        __spare0[1];
         /* 0x20 */
         uint64_t        stx_ino;
         uint64_t        stx_size;
         uint64_t        stx_blocks;
         uint64_t        stx_attributes_mask;
         /* 0x40 */
         const struct statx_timestamp    stx_atime;
         const struct statx_timestamp    stx_btime;
         const struct statx_timestamp    stx_ctime;
         const struct statx_timestamp    stx_mtime;
         /* 0x80 */
         uint32_t        stx_rdev_major;
         uint32_t        stx_rdev_minor;
         uint32_t        stx_dev_major;
         uint32_t        stx_dev_minor;
         /* 0x90 */
         uint64_t        __spare2[14];
         /* 0x100 */
};
#endif

the ltp definition only can be used when <sys/stat.h> miss statx struct 
instead of statx struct member.  It seems we don't have a better idea. 
Or do you have some idea?

It seems we think this question more complex, if system header miss, 
then use ltp definition, then we can not figure out whether fail or we 
just on old kernel.  Except we add a mininl kernel check in  the beginning.

> 
>> +	if (buf.stx_dio_mem_align != 0)
>> +		tst_res(TPASS, "stx_dio_mem_align: %u", buf.stx_dio_mem_align);
>> +	else
>> +		tst_res(TFAIL, "don't get stx_dio_mem_align on supported dio fs");
> 
> For the failure case: "stx_dio_mem_align was 0, but DIO should be supported"

OK.
> 
>> +
>> +	if (buf.stx_dio_offset_align != 0)
>> +		tst_res(TPASS, "stx_dio_offset_align: %u", buf.stx_dio_offset_align);
>> +	else
>> +		tst_res(TFAIL, "don't get stx_dio_offset_align on supported dio fs");
>> +#endif
> 
> For the failure case: "stx_dio_offset_align was 0, but DIO should be supported"

OK.
> 
>> +	SAFE_FILE_PRINTF(TESTFILE, "AAAA");
>> +	fd = open(TESTFILE, O_RDWR | O_DIRECT);
>> +	if (fd == -1 && errno == EINVAL) {
>> +		SAFE_CLOSE(fd);
>> +		tst_brk(TCONF, "The regular file is not on a filesystem that support DIO");
>> +	}
>> +	SAFE_CLOSE(fd);
> 
> The open() is not checked for error in all cases.

how about the following code:


fd = open(TESTFILE, O_RDWR | O_DIRECT);
if (fd == -1) {
	if (errno == EINVAL)
		 tst_brk(TCONF, "The regular file is not on a filesystem that support 
DIO");
	else
		tst_brk(TBROK | TERRNO, "The regular file was open with O_RDWR | 
O_DIRECT failed");
}
SAFE_CLOSE(fd);
> 
> Also, this is closing the file descriptor even when it is -1.

Oh, my mistake, sorry.
> 
>> +static struct tst_test test = {
>> +	.test_all = verify_statx,
>> +	.setup = setup,
>> +	.needs_root = 1,
> 
> Why does this test need root?

When using /dev/loop-control to search free loop device, we needs root.
see below:
tst_device.c:108: TINFO: Not allowed to open /dev/loop-control. Are you 
root?: EACCES (13)
tst_device.c:147: TINFO: No free devices found
tst_device.c:354: TBROK: Failed to acquire device

Best Regards
Yang Xu
> 
> - Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 3/4] syscalls/statx11: Add basic test for STATX_DIOALIGN on block device

[Yang Xu (Fujitsu)]

  2023/04/27 6:12, Eric Biggers 写道:
> On Thu, Apr 06, 2023 at 01:40:21PM +0800, Yang Xu wrote:
>> +static void verify_statx(void)
>> +{
>> +	struct statx buf;
>> +
>> +	memset(&buf, 0, sizeof(buf));
> 
> It is not necessary to memset struct statx to 0 before calling statx().

Will remove.
> 
>> +#ifdef HAVE_STRUCT_STATX_STX_DIO_MEM_ALIGN
> 
> Again, this looks wrong.  If the system headers are outdated, then LTP should
> use its in-tree header instead.

Have mention this in the previous email. We can discussion this in that 
email.

> 
>> +static void setup(void)
>> +{
>> +	char *dev_name;
>> +
>> +	dev_name = basename((char *)tst_device->dev);
> 
> This is modifying a const string, which seems problematic.

Yes, I plan to  modify code as below:
         char full_name[256];

         strcpy(full_name, tst_device->dev);
         dev_name = SAFE_BASENAME(full_name);

> 
>> +	sprintf(sys_bdev_logical_path, "/sys/block/%s/queue/logical_block_size", dev_name);
>> +	while (access(sys_bdev_logical_path, F_OK) != 0) {
>> +		dev_name[strlen(dev_name)-1] = '\0';
>> +		sprintf(sys_bdev_logical_path, "/sys/block/%s/queue/logical_block_size", dev_name);
>> +	}
> 
> What is this code doing?  Is it trying to strip off the partition number of the
> block device name?  

Yes.

>If so, it is incorrect because it assumes the partition
> number is only 1 digit long, which is not guaranteed.

I don't assume the partition number is only 1 digit long, this code has 
a while circulate. Also, I try the /dev/vdb11 and it also works.


> 
> How about just using /sys/class/block/%s/queue, which works for partitions?

In fact, /sys/block or /sys/class/block, these files are all link files 
to /sys/device/pci...... see below:
#cd /sys/class/block
[root@localhost block]# ls -l
total 0
lrwxrwxrwx. 1 root root 0 Apr 20 00:07 loop0 -> 
../../devices/virtual/block/loop0
lrwxrwxrwx. 1 root root 0 Apr 20 00:07 loop1 -> 
../../devices/virtual/block/loop1
lrwxrwxrwx. 1 root root 0 Apr 27 08:25 sda -> 
../../devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda
lrwxrwxrwx. 1 root root 0 Apr 27 08:25 sda1 -> 
../../devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda1
lrwxrwxrwx. 1 root root 0 Apr 27 08:25 sda2 -> 
../../devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda2
lrwxrwxrwx. 1 root root 0 Apr 27 08:25 sda3 -> 
../../devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda3
lrwxrwxrwx. 1 root root 0 Apr 27 08:25 sda4 -> 
../../devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda4
lrwxrwxrwx. 1 root root 0 Apr 27 08:25 sda5 -> 
../../devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda5
lrwxrwxrwx. 1 root root 0 Apr 27 08:25 sda6 -> 
../../devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda6
lrwxrwxrwx. 1 root root 0 Apr 27 08:25 zram0 -> 
../../devices/virtual/block/zram0
[root@localhost block]# cd /sys/block/
[root@localhost block]# ls -l
total 0
lrwxrwxrwx. 1 root root 0 Apr 20 00:03 loop0 -> 
../devices/virtual/block/loop0
lrwxrwxrwx. 1 root root 0 Apr 20 00:03 loop1 -> 
../devices/virtual/block/loop1
lrwxrwxrwx. 1 root root 0 Apr 20 00:03 sda -> 
../devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda
lrwxrwxrwx. 1 root root 0 Apr 20 00:03 zram0 -> 
../devices/virtual/block/zram0
[root@localhost block]# pwd

Best Regards
Yang Xu

> 
> - Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 3/4] syscalls/statx11: Add basic test for STATX_DIOALIGN on block device

[Yang Xu (Fujitsu)]

on 2023/04/27 11:37, Yang Xu (Fujitsu) wrote:
> 
> 
>    2023/04/27 6:12, Eric Biggers 写道:
>> On Thu, Apr 06, 2023 at 01:40:21PM +0800, Yang Xu wrote:
>>> +static void verify_statx(void)
>>> +{
>>> +	struct statx buf;
>>> +
>>> +	memset(&buf, 0, sizeof(buf));
>>
>> It is not necessary to memset struct statx to 0 before calling statx().
> 
> Will remove.
>>
>>> +#ifdef HAVE_STRUCT_STATX_STX_DIO_MEM_ALIGN
>>
>> Again, this looks wrong.  If the system headers are outdated, then LTP should
>> use its in-tree header instead.
> 
> Have mention this in the previous email. We can discussion this in that
> email.
> 
>>
>>> +static void setup(void)
>>> +{
>>> +	char *dev_name;
>>> +
>>> +	dev_name = basename((char *)tst_device->dev);
>>
>> This is modifying a const string, which seems problematic.
> 
> Yes, I plan to  modify code as below:
>           char full_name[256];
> 
>           strcpy(full_name, tst_device->dev);
>           dev_name = SAFE_BASENAME(full_name);
> 
>>
>>> +	sprintf(sys_bdev_logical_path, "/sys/block/%s/queue/logical_block_size", dev_name);
>>> +	while (access(sys_bdev_logical_path, F_OK) != 0) {
>>> +		dev_name[strlen(dev_name)-1] = '\0';
>>> +		sprintf(sys_bdev_logical_path, "/sys/block/%s/queue/logical_block_size", dev_name);
>>> +	}
>>
>> What is this code doing?  Is it trying to strip off the partition number of the
>> block device name?
> 
> Yes.
> 
>> If so, it is incorrect because it assumes the partition
>> number is only 1 digit long, which is not guaranteed.
> 
> I don't assume the partition number is only 1 digit long, this code has
> a while circulate. Also, I try the /dev/vdb11 and it also works.
> 
> 
>>
>> How about just using /sys/class/block/%s/queue, which works for partitions?

  /sys/class/block/%s/queue for partitions does't exist.

[root@localhost sda5]# pwd
/sys/class/block/sda5
[root@localhost sda5]# ls
alignment_offset  dev  discard_alignment  holders  inflight  partition 
power  ro  size  start  stat  subsystem  trace  uevent
[root@localhost sda5]#

Best Regards
Yang Xu

> 
> In fact, /sys/block or /sys/class/block, these files are all link files
> to /sys/device/pci...... see below:
> #cd /sys/class/block
> [root@localhost block]# ls -l
> total 0
> lrwxrwxrwx. 1 root root 0 Apr 20 00:07 loop0 ->
> ../../devices/virtual/block/loop0
> lrwxrwxrwx. 1 root root 0 Apr 20 00:07 loop1 ->
> ../../devices/virtual/block/loop1
> lrwxrwxrwx. 1 root root 0 Apr 27 08:25 sda ->
> ../../devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda
> lrwxrwxrwx. 1 root root 0 Apr 27 08:25 sda1 ->
> ../../devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda1
> lrwxrwxrwx. 1 root root 0 Apr 27 08:25 sda2 ->
> ../../devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda2
> lrwxrwxrwx. 1 root root 0 Apr 27 08:25 sda3 ->
> ../../devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda3
> lrwxrwxrwx. 1 root root 0 Apr 27 08:25 sda4 ->
> ../../devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda4
> lrwxrwxrwx. 1 root root 0 Apr 27 08:25 sda5 ->
> ../../devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda5
> lrwxrwxrwx. 1 root root 0 Apr 27 08:25 sda6 ->
> ../../devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda/sda6
> lrwxrwxrwx. 1 root root 0 Apr 27 08:25 zram0 ->
> ../../devices/virtual/block/zram0
> [root@localhost block]# cd /sys/block/
> [root@localhost block]# ls -l
> total 0
> lrwxrwxrwx. 1 root root 0 Apr 20 00:03 loop0 ->
> ../devices/virtual/block/loop0
> lrwxrwxrwx. 1 root root 0 Apr 20 00:03 loop1 ->
> ../devices/virtual/block/loop1
> lrwxrwxrwx. 1 root root 0 Apr 20 00:03 sda ->
> ../devices/pci0000:00/0000:00:1f.2/ata1/host0/target0:0:0/0:0:0:0/block/sda
> lrwxrwxrwx. 1 root root 0 Apr 20 00:03 zram0 ->
> ../devices/virtual/block/zram0
> [root@localhost block]# pwd
> 
> Best Regards
> Yang Xu
> 
>>
>> - Eric
> 

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 2/4] syscalls/statx10: Add basic test for STATX_DIOALIGN on regular file

[Eric Biggers]

On Thu, Apr 27, 2023 at 03:03:23AM +0000, Yang Xu (Fujitsu) wrote:
> on 2023/04/27 6:06, Eric Biggers wrote:
> > On Thu, Apr 06, 2023 at 01:40:20PM +0800, Yang Xu wrote:
> >> + * On ext4, files that use certain filesystem features (data journaling,
> >> + * encryption, and verity) fall back to buffered I/O. But ltp doesn't use these
> >> + * features by default, So I think dio should not fall back to buffered I/O.
> > 
> > Does LTP create and mount the filesystem itself?
> 
> Yes, I have enabled mount_device in tst_test struct, mount_device usage 
> you can see the following url.
> https://github.com/linux-test-project/ltp/wiki/C-Test-API#115-testing-with-a-block-device
> 
> If we set block device to LTP_DEV environment, we use this block device 
> to mount. Otherwise, use loop device to simuate it.

Great, can you update the comment to make it clear that this test creates its
own filesystem?

> > 
> > If not, then it wouldn't have control over this.
> > 
> >> +	if (!(buf.stx_mask & STATX_DIOALIGN)) {
> >> +		tst_res(TCONF, "STATX_DIOALIGN is not supported until linux 6.1");
> >> +		return;
> >> +	}
> > 
> > "Filesystem does not support STATX_DIOALIGN"
> 
> OK.
> > 
> >> +
> >> +#ifdef HAVE_STRUCT_STATX_STX_DIO_MEM_ALIGN
> > 
> > This looks wrong.  If the system headers are missing this field, then the
> > definition in the LTP source tree should be used instead.
> 
> Yes, usually, if system headers miss this field, we should use ltp 
> definition ie some macro.  But here it has a little difference, it is a 
> member in a struct.
> 
> see include/lapi/stat.h
> 
> #if defined(HAVE_STRUCT_STATX)
> #include <sys/stat.h>
> #else
> struct statx {
>          /* 0x00 */
>          uint32_t        stx_mask;
>          uint32_t        stx_blksize;
>          uint64_t        stx_attributes;
>          /* 0x10 */
>          uint32_t        stx_nlink;
>          uint32_t        stx_uid;
>          uint32_t        stx_gid;
>          uint16_t        stx_mode;
>          uint16_t        __spare0[1];
>          /* 0x20 */
>          uint64_t        stx_ino;
>          uint64_t        stx_size;
>          uint64_t        stx_blocks;
>          uint64_t        stx_attributes_mask;
>          /* 0x40 */
>          const struct statx_timestamp    stx_atime;
>          const struct statx_timestamp    stx_btime;
>          const struct statx_timestamp    stx_ctime;
>          const struct statx_timestamp    stx_mtime;
>          /* 0x80 */
>          uint32_t        stx_rdev_major;
>          uint32_t        stx_rdev_minor;
>          uint32_t        stx_dev_major;
>          uint32_t        stx_dev_minor;
>          /* 0x90 */
>          uint64_t        __spare2[14];
>          /* 0x100 */
> };
> #endif
> 
> the ltp definition only can be used when <sys/stat.h> miss statx struct 
> instead of statx struct member.  It seems we don't have a better idea. 
> Or do you have some idea?
> 
> It seems we think this question more complex, if system header miss, 
> then use ltp definition, then we can not figure out whether fail or we 
> just on old kernel.  Except we add a mininl kernel check in  the beginning.
> 

As I said, if the system headers are missing the needed fields, then LTP should
use its in-tree definition.  I.e., the in-tree definition should only be used if
HAVE_STRUCT_STATX && HAVE_STRUCT_STATX_STX_MNT_ID && [all other tested fields].

> >> +	SAFE_FILE_PRINTF(TESTFILE, "AAAA");
> >> +	fd = open(TESTFILE, O_RDWR | O_DIRECT);
> >> +	if (fd == -1 && errno == EINVAL) {
> >> +		SAFE_CLOSE(fd);
> >> +		tst_brk(TCONF, "The regular file is not on a filesystem that support DIO");
> >> +	}
> >> +	SAFE_CLOSE(fd);
> > 
> > The open() is not checked for error in all cases.
> 
> how about the following code:
> 
> 
> fd = open(TESTFILE, O_RDWR | O_DIRECT);
> if (fd == -1) {
> 	if (errno == EINVAL)
> 		 tst_brk(TCONF, "The regular file is not on a filesystem that support 
> DIO");
> 	else
> 		tst_brk(TBROK | TERRNO, "The regular file was open with O_RDWR | 
> O_DIRECT failed");
> }
> SAFE_CLOSE(fd);

I think that's okay.

- Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 2/4] syscalls/statx10: Add basic test for STATX_DIOALIGN on regular file

[Eric Biggers]

On Mon, May 01, 2023 at 10:44:42AM -0700, Eric Biggers wrote:
> As I said, if the system headers are missing the needed fields, then LTP should
> use its in-tree definition.  I.e., the in-tree definition should only be used if
> HAVE_STRUCT_STATX && HAVE_STRUCT_STATX_STX_MNT_ID && [all other tested fields].

"the in-tree definition should only be used" =>
"the system definition should only be used"

- Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 3/4] syscalls/statx11: Add basic test for STATX_DIOALIGN on block device

[Eric Biggers]

On Thu, Apr 27, 2023 at 03:50:25AM +0000, Yang Xu (Fujitsu) wrote:
> >>> +	sprintf(sys_bdev_logical_path, "/sys/block/%s/queue/logical_block_size", dev_name);
> >>> +	while (access(sys_bdev_logical_path, F_OK) != 0) {
> >>> +		dev_name[strlen(dev_name)-1] = '\0';
> >>> +		sprintf(sys_bdev_logical_path, "/sys/block/%s/queue/logical_block_size", dev_name);
> >>> +	}
> >>
> >> What is this code doing?  Is it trying to strip off the partition number of the
> >> block device name?
> > 
> > Yes.
> > 
> >> If so, it is incorrect because it assumes the partition
> >> number is only 1 digit long, which is not guaranteed.
> > 
> > I don't assume the partition number is only 1 digit long, this code has
> > a while circulate. Also, I try the /dev/vdb11 and it also works.
> > 
> > 
> >>
> >> How about just using /sys/class/block/%s/queue, which works for partitions?
> 
>   /sys/class/block/%s/queue for partitions does't exist.

Okay, sorry, I forgot that /sys/class/block/%s/queue doesn't exist for
partitions.

Please at least add a comment that explains what this code is doing, as it's
hard to understand.

- Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 2/4] syscalls/statx10: Add basic test for STATX_DIOALIGN on regular file

[Yang Xu (Fujitsu)]

on  2023/05/02 1:44, Eric Biggers wrote:
> On Thu, Apr 27, 2023 at 03:03:23AM +0000, Yang Xu (Fujitsu) wrote:
>> on 2023/04/27 6:06, Eric Biggers wrote:
>>> On Thu, Apr 06, 2023 at 01:40:20PM +0800, Yang Xu wrote:
>>>> + * On ext4, files that use certain filesystem features (data journaling,
>>>> + * encryption, and verity) fall back to buffered I/O. But ltp doesn't use these
>>>> + * features by default, So I think dio should not fall back to buffered I/O.
>>>
>>> Does LTP create and mount the filesystem itself?
>>
>> Yes, I have enabled mount_device in tst_test struct, mount_device usage
>> you can see the following url.
>> https://github.com/linux-test-project/ltp/wiki/C-Test-API#115-testing-with-a-block-device
>>
>> If we set block device to LTP_DEV environment, we use this block device
>> to mount. Otherwise, use loop device to simuate it.
> 
> Great, can you update the comment to make it clear that this test creates its
> own filesystem?

Of course.
> 
>>>
>>> If not, then it wouldn't have control over this.
>>>
>>>> +	if (!(buf.stx_mask & STATX_DIOALIGN)) {
>>>> +		tst_res(TCONF, "STATX_DIOALIGN is not supported until linux 6.1");
>>>> +		return;
>>>> +	}
>>>
>>> "Filesystem does not support STATX_DIOALIGN"
>>
>> OK.
>>>
>>>> +
>>>> +#ifdef HAVE_STRUCT_STATX_STX_DIO_MEM_ALIGN
>>>
>>> This looks wrong.  If the system headers are missing this field, then the
>>> definition in the LTP source tree should be used instead.
>>
>> Yes, usually, if system headers miss this field, we should use ltp
>> definition ie some macro.  But here it has a little difference, it is a
>> member in a struct.
>>
>> see include/lapi/stat.h
>>
>> #if defined(HAVE_STRUCT_STATX)
>> #include <sys/stat.h>
>> #else
>> struct statx {
>>           /* 0x00 */
>>           uint32_t        stx_mask;
>>           uint32_t        stx_blksize;
>>           uint64_t        stx_attributes;
>>           /* 0x10 */
>>           uint32_t        stx_nlink;
>>           uint32_t        stx_uid;
>>           uint32_t        stx_gid;
>>           uint16_t        stx_mode;
>>           uint16_t        __spare0[1];
>>           /* 0x20 */
>>           uint64_t        stx_ino;
>>           uint64_t        stx_size;
>>           uint64_t        stx_blocks;
>>           uint64_t        stx_attributes_mask;
>>           /* 0x40 */
>>           const struct statx_timestamp    stx_atime;
>>           const struct statx_timestamp    stx_btime;
>>           const struct statx_timestamp    stx_ctime;
>>           const struct statx_timestamp    stx_mtime;
>>           /* 0x80 */
>>           uint32_t        stx_rdev_major;
>>           uint32_t        stx_rdev_minor;
>>           uint32_t        stx_dev_major;
>>           uint32_t        stx_dev_minor;
>>           /* 0x90 */
>>           uint64_t        __spare2[14];
>>           /* 0x100 */
>> };
>> #endif
>>
>> the ltp definition only can be used when <sys/stat.h> miss statx struct
>> instead of statx struct member.  It seems we don't have a better idea.
>> Or do you have some idea?
>>
>> It seems we think this question more complex, if system header miss,
>> then use ltp definition, then we can not figure out whether fail or we
>> just on old kernel.  Except we add a mininl kernel check in  the beginning.
>>
> 
> As I said, if the system headers are missing the needed fields, then LTP should
> use its in-tree definition.  I.e., the in-tree definition should only be used if
> HAVE_STRUCT_STATX && HAVE_STRUCT_STATX_STX_MNT_ID && [all other tested fields].

Yes, it should work well but ltp has other owner headers(they still 
include <sys/stat.h>), so it can't work well
I try it as below:

+#if defined(HAVE_STATX) && \
+    defined(HAVE_STRUCT_STATX_TIMESTAMP) && \
+    defined(HAVE_STRUCT_STATX) && \
+    defined(HAVE_STRUCT_STATX_STX_MNT_ID)
+#include <sys/stat.h>
+#else
....
#endif


see ltp/include, ltp owner header also uses  <sys/stat.h>(use stat 
struct or stat syscall)
safe_file_ops_fn.h:21:#include <sys/stat.h>
safe_macros_fn.h:19:#include <sys/stat.h>
tst_device.h:11:#include <sys/stat.h>
tst_safe_file_at.h:10:#include <sys/stat.h>
tst_safe_macros.h:13:#include <sys/stat.h>

If I remove mnt_id  ifdef check in statx01.c, then statx01  will report 
redefine error for statx struct as below:

In file included from statx01.c:35:
../../../../include/lapi/stat.h:30:8: error: redefinition of ‘struct 
statx_timestamp’
  struct statx_timestamp {
         ^~~~~~~~~~~~~~~
In file included from /usr/include/bits/statx.h:31,
                  from /usr/include/sys/stat.h:446,
                  from ../../../../include/tst_device.h:11,
                  from ../../../../include/tst_test.h:23,
                  from statx01.c:33:
/usr/include/linux/stat.h:56:8: note: originally defined here
  struct statx_timestamp {
         ^~~~~~~~~~~~~~~
In file included from statx01.c:35:
../../../../include/lapi/stat.h:72:8: error: redefinition of ‘struct statx’
  struct statx {
         ^~~~~
In file included from /usr/include/bits/statx.h:31,
                  from /usr/include/sys/stat.h:446,
                  from ../../../../include/tst_device.h:11,
                  from ../../../../include/tst_test.h:23,
                  from statx01.c:33:
/usr/include/linux/stat.h:99:8: note: originally defined here
  struct statx {
         ^~~~~
In file included from statx01.c:35:
../../../../include/lapi/stat.h:113:19: error: conflicting types for ‘statx’
  static inline int statx(int dirfd, const char *pathname, unsigned int 
flags,
                    ^~~~~
In file included from /usr/include/bits/statx.h:39,
                  from /usr/include/sys/stat.h:446,
                  from ../../../../include/tst_device.h:11,
                  from ../../../../include/tst_test.h:23,
                  from statx01.c:33:
/usr/include/bits/statx-generic.h:56:5: note: previous declaration of 
‘statx’ was here
  int statx (int __dirfd, const char *__restrict __path, int __flags,
      ^~~~~
statx01.c:96:2: error: #endif without #if
  #endif
   ^~~~~

IMO, to change ltp owner header to avoid use <sys/mount.h> seems 
difficulty.


Best Regards
Yang Xu

> 
>>>> +	SAFE_FILE_PRINTF(TESTFILE, "AAAA");
>>>> +	fd = open(TESTFILE, O_RDWR | O_DIRECT);
>>>> +	if (fd == -1 && errno == EINVAL) {
>>>> +		SAFE_CLOSE(fd);
>>>> +		tst_brk(TCONF, "The regular file is not on a filesystem that support DIO");
>>>> +	}
>>>> +	SAFE_CLOSE(fd);
>>>
>>> The open() is not checked for error in all cases.
>>
>> how about the following code:
>>
>>
>> fd = open(TESTFILE, O_RDWR | O_DIRECT);
>> if (fd == -1) {
>> 	if (errno == EINVAL)
>> 		 tst_brk(TCONF, "The regular file is not on a filesystem that support
>> DIO");
>> 	else
>> 		tst_brk(TBROK | TERRNO, "The regular file was open with O_RDWR |
>> O_DIRECT failed");
>> }
>> SAFE_CLOSE(fd);
> 
> I think that's okay.
> 
> - Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 3/4] syscalls/statx11: Add basic test for STATX_DIOALIGN on block device

[Yang Xu (Fujitsu)]

on 2023/05/02 1:49, Eric Biggers wrote:
> On Thu, Apr 27, 2023 at 03:50:25AM +0000, Yang Xu (Fujitsu) wrote:
>>>>> +	sprintf(sys_bdev_logical_path, "/sys/block/%s/queue/logical_block_size", dev_name);
>>>>> +	while (access(sys_bdev_logical_path, F_OK) != 0) {
>>>>> +		dev_name[strlen(dev_name)-1] = '\0';
>>>>> +		sprintf(sys_bdev_logical_path, "/sys/block/%s/queue/logical_block_size", dev_name);
>>>>> +	}
>>>>
>>>> What is this code doing?  Is it trying to strip off the partition number of the
>>>> block device name?
>>>
>>> Yes.
>>>
>>>> If so, it is incorrect because it assumes the partition
>>>> number is only 1 digit long, which is not guaranteed.
>>>
>>> I don't assume the partition number is only 1 digit long, this code has
>>> a while circulate. Also, I try the /dev/vdb11 and it also works.
>>>
>>>
>>>>
>>>> How about just using /sys/class/block/%s/queue, which works for partitions?
>>
>>    /sys/class/block/%s/queue for partitions does't exist.
> 
> Okay, sorry, I forgot that /sys/class/block/%s/queue doesn't exist for
> partitions.
> 
> Please at least add a comment that explains what this code is doing, as it's
> hard to understand.

Yes, will add a comment for this

Best Regards
Yang Xu
> 
> - Eric

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 2/4] syscalls/statx10: Add basic test for STATX_DIOALIGN on regular file

[Yang Xu (Fujitsu)]

on 2023/05/08 16:25, Yang Xu (Fujitsu) wrote:
> IMO, to change ltp owner header to avoid use <sys/mount.h> seems
> difficulty.

<sys/mount.h> => <sys/stat.h>

Best Regards
Yang Xu

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp