Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 23 Feb 2022 08:54:26 -0800
From:   "Darrick J. Wong" <djwong@kernel.org>
To:     Anna Schumaker <anna@kernel.org>
Cc:     fstests@vger.kernel.org, linux-nfs@vger.kernel.org
Subject: Re: [PATCH 2/4] generic/531: Move test from 'quick' group to 'stress'
Message-ID: <20220223165426.GG8288@magnolia>
References: <20220208215232.491780-1-anna@kernel.org>
 <20220208215232.491780-3-anna@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220208215232.491780-3-anna@kernel.org>
Precedence: bulk

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.

> ---
>  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
>