Received-SPF: softfail (google.com: domain of transitioning linux-nfs-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
MIME-Version: 1.0
References: <20220208215232.491780-1-anna@kernel.org> <20220208215232.491780-3-anna@kernel.org>
 <20220223165426.GG8288@magnolia>
In-Reply-To: <20220223165426.GG8288@magnolia>
From:   Anna Schumaker <anna@kernel.org>
Date:   Wed, 23 Feb 2022 14:38:30 -0500
Message-ID: <CAFX2Jfmfhh3NVtC3gE6pCtqdh3oy5LiHqTa2-Ggk995j-g8akA@mail.gmail.com>
Subject: Re: [PATCH 2/4] generic/531: Move test from 'quick' group to 'stress'
To:     "Darrick J. Wong" <djwong@kernel.org>
Cc:     fstests@vger.kernel.org,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Feb 23, 2022 at 11:54 AM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Tue, Feb 08, 2022 at 04:52:30PM -0500, Anna Schumaker wrote:
> > From: Anna Schumaker <Anna.Schumaker@Netapp.com>
> >
> > The comment up top says this is a stress test, so at the very least it
> > should be added to this group. As for removing it from the quick group,
> > making this test variable on the number of CPUs means this test could
> > take a very long time to finish (I'm unsure exactly how long on NFS v4.1
> > because I usually kill it after a half hour or so)
> >
> > Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
> > ---
> > I have thought of two alternatives to this patch that would work for me:
> >   1) Could we add an _unsupported_fs function which is the opposite of
> >      _supported_fs to prevent tests from running on specific filesystems?
> >   2) Would it be okay to check if $FSTYP == "nfs" when setting nr_cpus,
> >      and set it to 1 instead? Perhaps through a function in common/rc
> >      that other tests can use if they scale work based on cpu-count?
>
> How about we create a function to estimate fs threading scalability?
> There are probably (simple) filesystems out there with a Big Filesystem
> Lock that won't benefit from more CPUs pounding on it...
>
> # Estimate how many writer threads we should start to stress test this
> # type of filesystem.
> _estimate_threading_factor() {
>         case "$FSTYP" in
>         "nfs")
>                 echo 1;;
>         *)
>                 echo $((2 * $(getconf _NPROCESSORS_ONLN) ));;
>         esac
> }
>
> and later:
>
> nr_cpus=$(_estimate_threading_factor)
>
> Once something like this is landed, we can customize for each FSTYP.  I
> suspect that XFS on spinning rust might actually want "2" here, not
> nr_cpus*2, given the sporadic complaints about this test taking much
> longer for a few people.

Sure. Should I do a `git grep` for "nr_cpus" on the other tests and
update them all at the same time, or just leave it with this one file
to start?

Anna
>
> > ---
> >  tests/generic/531 | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/generic/531 b/tests/generic/531
> > index 5e84ca977b44..62e3cac92423 100755
> > --- a/tests/generic/531
> > +++ b/tests/generic/531
> > @@ -12,7 +12,7 @@
> >  # Use every CPU possible to stress the filesystem.
> >  #
> >  . ./common/preamble
> > -_begin_fstest auto quick unlink
> > +_begin_fstest auto stress unlink
>
> As for this change itself,
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
>
> --D
>
>
> >  testfile=$TEST_DIR/$seq.txt
> >
> >  # Import common functions.
> > --
> > 2.35.1
> >