Re: [LTP] [PATCH] syscalls/statx13: Add basic test for STATX_WRITE_ATOMIC on regular file

[Petr Vorel <pvorel@suse.cz>]

Hi Gaurav,

nit: this is a second version of the patch, it'd help to distinguish them if you
generate it with git format-patch -v2. It's also mentioned (as "-v 2") in our
tutorial:

https://linux-test-project.readthedocs.io/en/latest/developers/test_case_tutorial.html

(Tutorial is slightly outdated both code and instructions but still more or less
valid. It should be structured and improved but I still recommend you to read it
to get various ideas not covered elsewhere).

Also this second patch failed to build in CI. Could you please enable CI in your
LTP fork and push the branch before sending? You 1) get results quicker 2) saves
our time to look on something which is broken.

> This patch adds a new test to validate these atomic write limit fields.
> The test ensures the filesystem correctly advertises the STATX_ATTR_WRITE_ATOMIC
> attribute when queried on a file opened with O_DIRECT. It also verifies that the
> reported optimized maximum is logically consistent by falling within the
> absolute minimum and maximum boundaries. Furthermore, it checks that all
> reported atomic write unit sizes are valid powers of two, adhering to the strict
> requirements of the kernel block layer.

> If the underlying storage hardware or filesystem lacks atomic write
> support, the test gracefully skips with TCONF.

> Fixes: #1224

> Signed-off-by: Gaurav Pathak <gaurav.pathak@suse.com>
> ---
>  configure.ac                               |   2 +-
>  testcases/kernel/syscalls/statx/.gitignore |   1 +
>  testcases/kernel/syscalls/statx/statx13.c  | 130 +++++++++++++++++++++
>  3 files changed, 132 insertions(+), 1 deletion(-)

Please go over agent reports (it asked for runtest/syscalls already in the first patch).
...
> index 000000000..ea42a102c
> --- /dev/null
> +++ b/testcases/kernel/syscalls/statx/statx13.c
> @@ -0,0 +1,130 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2026 SUSE LLC <gaurav.pathak@suse.com>
> + */
> +
> +/*\
> + * This test validates the STATX_WRITE_ATOMIC feature (introduced in Linux 6.13).
> + * It ensures that supported filesystems (xfs as of now) correctly report their
git grep generic_atomic_write_valid (functions mentioned at [1] which was
mentioned in the "block atomic writes" feature [2]) mentions also ext4. And you
also test both xfs and ext4, you should either update filesystem list in docs or
not mention filesystems at all.

[1] https://lore.kernel.org/linux-xfs/20240607143919.2622319-1-john.g.garry@oracle.com/T/#t
[2] https://lore.kernel.org/lkml/20240620125359.2684798-1-john.g.garry@oracle.com/

> + * atomic write limits to user space when queried via statx().
> + *
> + * The test performs the following validations:
nit: there needs to be a blank line otherwise list will not be formatted

=> we should teach agent to recognise it

> + * - Creates a test file using O_DIRECT (a prerequisite for atomic writes).
> + * - Calls statx() with the STATX_WRITE_ATOMIC mask to retrieve the limits.
> + * - Verifies that stx_atomic_write_unit_min, stx_atomic_write_unit_max, and
> + *    stx_atomic_write_unit_max_opt are logically consistent (e.g., max_opt is
And this extra space before stx_atomic_write_unit_max_opt make is formatted as
bold (see later doc build)

> + *    within the min and max bounds).
> + * - Ensures all reported atomic write unit sizes are valid powers of two.
> + */
> +
> +#define _GNU_SOURCE
> +#include <sys/param.h>
> +#include "tst_test.h"
> +
> +#define MNTPOINT "mnt_point"
> +#define TESTFILE MNTPOINT"/testfile"

nit style check complains, you'll find it with:
$ make check-statx13
...
CHECK testcases/kernel/syscalls/statx/statx13.c
statx13.c:25: CHECK: Concatenated strings should use spaces between elements


> +#define MODE 0644
> +
> +#define WRITE_SIZE 4096
> +#define ALIGNMENT  4096
> +
> +static int file_fd = -1;
> +
> +static void verify_statx(void)
> +{
> +	struct statx buff;
> +
> +	TST_EXP_PASS_SILENT(statx(AT_FDCWD, TESTFILE, 0, STATX_BASIC_STATS | STATX_WRITE_ATOMIC, &buff),
> +			"statx(AT_FDCWD, %s, 0, STATX_WRITE_ATOMIC, &buf)", TESTFILE);
> +
> +	if (!(buff.stx_attributes & STATX_ATTR_WRITE_ATOMIC)) {
> +		tst_res(TCONF, "Filesystem does not support STATX_WRITE_ATOMIC");
> +		return;

This is enough (without following return).
		tst_brk(TCONF, "Filesystem does not support STATX_WRITE_ATOMIC");
> +	}
> +
> +	if (buff.stx_atomic_write_unit_min > 0 &&
> +			__builtin_popcount(buff.stx_atomic_write_unit_min) == 1)
> +		tst_res(TPASS, "stx_atomic_write_unit_min(%u) is power of 2",
> +				buff.stx_atomic_write_unit_min);
> +	else
> +		tst_res(TFAIL, "stx_atomic_write_unit_min(%u) is not a power of 2",
> +				buff.stx_atomic_write_unit_min);
> +
> +	if (buff.stx_atomic_write_unit_max > 0 &&
> +			__builtin_popcount(buff.stx_atomic_write_unit_max) == 1)
> +		tst_res(TPASS, "stx_atomic_write_unit_max(%u) is power of 2",
> +				buff.stx_atomic_write_unit_max);
> +	else
> +		tst_res(TFAIL, "stx_atomic_write_unit_max(%u) is not a power of 2",
> +				buff.stx_atomic_write_unit_max);
> +
> +#ifdef HAVE_STRUCT_STATX_STX_ATOMIC_WRITE_UNIT_MAX_OPT
> +	if (buff.stx_atomic_write_unit_max_opt == 0) {
> +		tst_res(TINFO, "stx_atomic_write_unit_max_opt is 0 (no optimized max reported)");
Shouldn't this be TPASS?

> +	} else {
> +		if (buff.stx_atomic_write_unit_max_opt > buff.stx_atomic_write_unit_max)
> +			tst_res(TFAIL, "stx_atomic_write_unit_max_opt (%u) exceeds max (%u)",
> +					buff.stx_atomic_write_unit_max_opt,
> +					buff.stx_atomic_write_unit_max);
> +
> +		else if (buff.stx_atomic_write_unit_max_opt < buff.stx_atomic_write_unit_min)
> +			tst_res(TFAIL, "stx_atomic_write_unit_max_opt (%u) is less than min (%u)",
> +					buff.stx_atomic_write_unit_max_opt,
> +					buff.stx_atomic_write_unit_min);
> +		else
> +			tst_res(TPASS, "stx_atomic_write_unit_max_opt (%u) is within valid range [%u, %u]",
> +					buff.stx_atomic_write_unit_max_opt,
> +					buff.stx_atomic_write_unit_min,
> +					buff.stx_atomic_write_unit_max);
> +
> +		if (__builtin_popcount(buff.stx_atomic_write_unit_max_opt) != 1)
> +			tst_res(TFAIL, "stx_atomic_write_unit_max_opt (%u) is not a power of 2",
> +					buff.stx_atomic_write_unit_max_opt);
> +	}
> +#else
> +	tst_res(TCONF, "stx_atomic_write_unit_max_opt is not defined in struct statx");
> +#endif
> +}
> +
> +static void setup(void)
> +{
> +	char *data_buff = SAFE_MEMALIGN(ALIGNMENT, WRITE_SIZE);
> +
> +	if (strcmp(tst_device->fs_type, "xfs") && strcmp(tst_device->fs_type, "ext4"))
> +		tst_brk(TCONF, "This test only supports ext4 and xfs");
FYI .filesystems member in struct tst_test test select filesystems, you don't
need to check here. Please remove it.

=> we should teach agent to detect this.

> +
> +	umask(0);
> +	memset(data_buff, '@', WRITE_SIZE);
> +
> +	file_fd =  SAFE_OPEN(TESTFILE, O_RDWR | O_CREAT | O_DIRECT, MODE);
> +	SAFE_WRITE(SAFE_WRITE_ALL, file_fd, data_buff, WRITE_SIZE);
> +}
> +
> +static void cleanup(void)
> +{
> +	if (file_fd > -1)
> +		SAFE_CLOSE(file_fd);
> +}
> +
> +static struct tst_test test = {
> +	.test_all = verify_statx,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.min_kver = "6.13",
> +	.needs_root = 1,
> +	.needs_device = 1,
> +	.needs_tmpdir = 1,
nit: Some tags aren't needed and will be later deleted. You can see it:

$ cd metadata/; make
/home/pvorel/install/src/ltp.git/metadata/parse.sh > ltp.json
testcases/kernel/syscalls/statx/statx13.c: useless tag: needs_device
testcases/kernel/syscalls/statx/statx13.c: useless tag: needs_tmpdir

Or you could see it at doc build (but that usually requires python virtualenv
and it's slower, OTOH you can check how the doc will look like)

$ cd doc; make setup && make
=> see the docs in thml file:
doc/html/users/test_catalog.html#statx13

> +	.mntpoint = MNTPOINT,
> +	.mount_device = 1,
> +	.filesystems = (struct tst_fs[]) {
> +		{
> +			.type = "xfs",
> +			.mkfs_opts = (const char *const []){"-f", "-bsize=16K", NULL},
> +		},
> +		{
> +			.type = "ext4",
> +			.mkfs_opts = (const char *const []){"-O", "bigalloc", "-b", "4096", "-C", "65536", NULL},
BTW my 7.2.0-4.g080d79d-default still TCONF on ext4. Maybe wrong params?
> +		},
> +		{}
> +	},
> +};

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