Re: [LTP] [PATCH 1/1] nfs/nfs08.sh: Add test for NFS cache invalidation

Cyril Hrubis
2023-04-25T13:14:12Z

Hi!
> v4 [1] of not yet upstreamed patch accidentally broke cache invalidation
> for directories by clearing NFS_INO_INVALID_DATA inappropriately.
> Although it was fixed in v5 [2] thus kernel was not actually broken,
> it's better to prevent this in the future.
>
> [1] https://lore.kernel.org/linux-nfs/167649314509.15170.15885497881041431304@noble.neil.brown.name/
> [2] https://lore.kernel.org/linux-nfs/167943762461.8008.3152357340238024342@noble.neil.brown.name/
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> NOTE: although Neil suggested to test on ext2 to make the problems more
> obvious (ext2 has low-resolution time stamps) and to use sleep, with
> nfs_lib.sh LTP code it works without sleep and on all filesystems.
>
> It'd require more changes in nfs_lib.sh to force ext2 only for nfs08.sh
> (other tests are using the library). Not only it's needed, but I also
> plan to switch nfs_lib.sh to use all available filesystems via
> TST_ALL_FILESYSTEMS=1, therefore ext2 will be among tested filesystems
> anyway.
>
> Test fails also on all available NFS versions, thus using the same
> approach as the other tests - test all versions, although not sure if
> it's really needed. But test runs very quickly (unlike other nfs tests
> it's not a stress test), thus I'd keep it.
>
> NOTE: anybody trying to test this patch with TMPDIR (to test different
> filesystems) test with this fix [1].
>
> Kind regards,
> Petr
>
> [1] https://lore.kernel.org/ltp/20230412073953.1983857-1-pvorel@suse.cz/
>
> runtest/net.nfs | 11 +++++++++++
> testcases/network/nfs/nfs_stress/Makefile | 9 +--------
> testcases/network/nfs/nfs_stress/nfs08.sh | 20 ++++++++++++++++++++
> 3 files changed, 32 insertions(+), 8 deletions(-)
> create mode 100755 testcases/network/nfs/nfs_stress/nfs08.sh
>
> diff --git a/runtest/net.nfs b/runtest/net.nfs
> index 3249c35cb..72cf4b307 100644
> --- a/runtest/net.nfs
> +++ b/runtest/net.nfs
> @@ -72,6 +72,17 @@ nfs4_ipv6_07 nfs07.sh -6 -v 4 -t tcp
> nfs41_ipv6_07 nfs07.sh -6 -v 4.1 -t tcp
> nfs42_ipv6_07 nfs07.sh -6 -v 4.2 -t tcp
>
> +nfs3_08 nfs08.sh -v 3 -t udp
> +nfs3t_08 nfs08.sh -v 3 -t tcp
> +nfs4_08 nfs08.sh -v 4 -t tcp
> +nfs41_08 nfs08.sh -v 4.1 -t tcp
> +nfs42_08 nfs08.sh -v 4.2 -t tcp
> +nfs3_ipv6_08 nfs08.sh -6 -v 3 -t udp
> +nfs3t_ipv6_08 nfs08.sh -6 -v 3 -t tcp
> +nfs4_ipv6_08 nfs08.sh -6 -v 4 -t tcp
> +nfs41_ipv6_08 nfs08.sh -6 -v 4.1 -t tcp
> +nfs42_ipv6_08 nfs08.sh -6 -v 4.2 -t tcp
> +
> nfslock3_01 nfslock01.sh -v 3 -t udp
> nfslock3t_01 nfslock01.sh -v 3 -t tcp
> nfslock4_01 nfslock01.sh -v 4 -t tcp
> diff --git a/testcases/network/nfs/nfs_stress/Makefile b/testcases/network/nfs/nfs_stress/Makefile
> index 8cd095867..5b396dede 100644
> --- a/testcases/network/nfs/nfs_stress/Makefile
> +++ b/testcases/network/nfs/nfs_stress/Makefile
> @@ -9,13 +9,6 @@ include $(top_srcdir)/include/mk/testcases.mk
> nfs04_create_file: CPPFLAGS += -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
> nfs05_make_tree: LDLIBS += -lpthread
>
> -INSTALL_TARGETS := nfs_lib.sh \
> - nfs01.sh \
> - nfs02.sh \
> - nfs03.sh \
> - nfs04.sh \
> - nfs05.sh \
> - nfs06.sh \
> - nfs07.sh
> +INSTALL_TARGETS := *.sh
>
> include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/network/nfs/nfs_stress/nfs08.sh b/testcases/network/nfs/nfs_stress/nfs08.sh
> new file mode 100755
> index 000000000..8a0f40242
> --- /dev/null
> +++ b/testcases/network/nfs/nfs_stress/nfs08.sh
> @@ -0,0 +1,20 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2023 Petr Vorel <pvorel@suse.cz>
> +# Test reproducer for broken NFS cache invalidation for directories.
> +# Based on reproducer from Neil Brown <neilb@suse.de>
> +
> +TST_TESTFUNC="do_test"
> +
> +do_test()
> +{
> + tst_res TINFO "testing NFS cache invalidation for directories"
> +
> + touch 1
> + EXPECT_PASS 'ls | grep 1'
> +
> + touch 2
> + EXPECT_PASS 'ls | grep 2'
> +}

I do not get how this actually detects case invalidation, it probably
does, but slightly better description how this actually excercises the
case would help.

--
Cyril Hrubis
chrubis@suse.cz