On Tue, Aug 13, 2024 at 02:21:08PM -0400, Jeff Layton wrote:
Hi Jeff :)
Any description about this case test for?
> Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> ---
> HCH suggested I roll a fstest for this problem that I found in btrfs the
> other day. In principle, we probably could expand this to other dir
> operations and to check the parent timestamps, but having to do all that
> in C is a pain. I didn't see a good way to use xfs_io for this,
> however.
Is there a kernel commit or patch link about the bug which you found?
> ---
> src/Makefile | 2 +-
> src/unlink-ctime.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
> tests/generic/755 | 26 ++++++++++++++++++++++++++
> tests/generic/755.out | 2 ++
> 4 files changed, 79 insertions(+), 1 deletion(-)
>
> diff --git a/src/Makefile b/src/Makefile
> index 9979613711c9..c71fa41e4668 100644
> --- a/src/Makefile
> +++ b/src/Makefile
> @@ -34,7 +34,7 @@ LINUX_TARGETS = xfsctl bstat t_mtab getdevicesize preallo_rw_pattern_reader \
> attr_replace_test swapon mkswap t_attr_corruption t_open_tmpfiles \
> fscrypt-crypt-util bulkstat_null_ocount splice-test chprojid_fail \
> detached_mounts_propagation ext4_resize t_readdir_3 splice2pipe \
> - uuid_ioctl t_snapshot_deleted_subvolume fiemap-fault
> + uuid_ioctl t_snapshot_deleted_subvolume fiemap-fault unlink-ctime
The .gitignore need updating too.
>
> EXTRA_EXECS = dmerror fill2attr fill2fs fill2fs_check scaleread.sh \
> btrfs_crc32c_forged_name.py popdir.pl popattr.py \
> diff --git a/src/unlink-ctime.c b/src/unlink-ctime.c
> new file mode 100644
> index 000000000000..7661e340eaba
> --- /dev/null
> +++ b/src/unlink-ctime.c
> @@ -0,0 +1,50 @@
> +#define _GNU_SOURCE 1
> +#include <stdio.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <errno.h>
> +#include <sys/stat.h>
> +
> +int main(int argc, char **argv)
> +{
> + int fd, ret;
> + struct statx before, after;
> +
> + if (argc < 2) {
> + fprintf(stderr, "Must specify filename!\n");
> + return 1;
> + }
> +
> + fd = open(argv[1], O_RDWR|O_CREAT, 0600);
> + if (fd < 0) {
> + perror("open");
> + return 1;
> + }
> +
> + ret = statx(fd, "", AT_EMPTY_PATH, STATX_CTIME, &before);
> + if (ret) {
> + perror("statx");
> + return 1;
> + }
> +
> + sleep(1);
> +
> + ret = unlink(argv[1]);
> + if (ret) {
> + perror("unlink");
> + return 1;
> + }
> +
> + ret = statx(fd, "", AT_EMPTY_PATH, STATX_CTIME, &after);
So you need to keep the "fd" after unlink. If so, there might not be a
way through xfs_io to do that.
> + if (ret) {
> + perror("statx");
> + return 1;
> + }
> +
> + if (before.stx_ctime.tv_nsec == after.stx_ctime.tv_nsec &&
> + before.stx_ctime.tv_sec == after.stx_ctime.tv_sec) {
> + printf("No change to ctime after unlink!\n");
> + return 1;
> + }
> + return 0;
> +}
> diff --git a/tests/generic/755 b/tests/generic/755
> new file mode 100755
> index 000000000000..500c51f99630
> --- /dev/null
> +++ b/tests/generic/755
> @@ -0,0 +1,26 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2024, Jeff Layton <jlayton@xxxxxxxxxx>
> +#
> +# FS QA Test No. 755
> +#
> +# Create a file, stat it and then unlink it. Does the ctime of the
> +# target inode change?
> +#
> +. ./common/preamble
> +_begin_fstest auto quick
^^^^^^
unlink
> +
> +# Import common functions.
> +. ./common/filter
> +. ./common/dmerror
Why dmerror and filter are needed? If not necessary, remove these
3 lines.
Others looks good to me.
Thanks,
Zorro
> +
> +_require_test
> +_require_test_program unlink-ctime
> +
> +testfile="$TEST_DIR/unlink-ctime.$$"
> +
> +$here/src/unlink-ctime $testfile
> +
> +echo Silence is golden
> +status=0
> +exit
> diff --git a/tests/generic/755.out b/tests/generic/755.out
> new file mode 100644
> index 000000000000..7c9ea51cd298
> --- /dev/null
> +++ b/tests/generic/755.out
> @@ -0,0 +1,2 @@
> +QA output created by 755
> +Silence is golden
>
> ---
> base-commit: f5ada754d5838d29fd270257003d0d123a9d1cd2
> change-id: 20240813-master-e3b46de630bd
>
> Best regards,
> --
> Jeff Layton <jlayton@xxxxxxxxxx>
>
>
