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

[Cyril Hrubis <chrubis@suse.cz>]

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

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