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

[Eric Biggers <ebiggers@kernel.org>]

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