2023-04-26 17:50:07

by Petr Vorel

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

Hi all,

> > 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/[email protected]/
> > > [2] https://lore.kernel.org/linux-nfs/[email protected]/

> ...
> > > --- /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 <[email protected]>
> > > +# Test reproducer for broken NFS cache invalidation for directories.
> > > +# Based on reproducer from Neil Brown <[email protected]>
> > > +
> > > +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.

> The behavior is:

> "touch 1" asks for data invalidation (new file created), therefore following ls
> (EXPECT_PASS 'ls | grep 1') fills the cache. "touch 2" should again ask for
> data invalidation, but it the unfixed v4 version of the patch it did not
> resulted to cache invalidation. Therefore second ls (EXPECT_PASS 'ls | grep 2')
> shows just 1, but not 2. i.e. in the affected kernel only second ls failed,
> but obviously both should be checked (nobody knows how another bug on cache
> invalidation will behave).

> I can add the description above to the commit message and adjust the comment in
> the file.

Merged with the updated description.

Kind regards,
Petr