Re: [PATCH 1/2] generic: fsstress with cpu offlining

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Aug 18, 2021 at 02:32:49PM +0800, Zorro Lang wrote:
> On Wed, Aug 18, 2021 at 02:07:37PM +0800, Zorro Lang wrote:
> > On Tue, Aug 17, 2021 at 04:53:19PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > Exercise filesystem operations when we're taking CPUs online and offline
> > > throughout the test.
> > 
> > Just ask, is this test cover something (commits)?

This test started its life as a simple exerciser to try to spot problems
with the percpu data structure handling in Dave's log scalability
patchset.  Now that inode inactivation also uses percpu lists, it covers
both of those things.

However, filesystems were already supposed to keep running even with
CPUs going off and online, so I didn't list any specific commits.

> > > 
> > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > ---
> > >  tests/generic/726     |   71 +++++++++++++++++++++++++++++++++++++++++++++++++
> > >  tests/generic/726.out |    2 +
> > >  2 files changed, 73 insertions(+)
> > >  create mode 100755 tests/generic/726
> > >  create mode 100644 tests/generic/726.out
> > > 
> > > 
> > > diff --git a/tests/generic/726 b/tests/generic/726
> > > new file mode 100755
> > > index 00000000..4b072b7f
> > > --- /dev/null
> > > +++ b/tests/generic/726
> > > @@ -0,0 +1,71 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (c) 2021 Oracle, Inc.  All Rights Reserved.
> > > +#
> > > +# FS QA Test No. 726
> > > +#
> > > +# Run an all-writes fsstress run with multiple threads while exercising CPU
> > > +# hotplugging to shake out bugs in the write path.
> > > +#
> > > +. ./common/preamble
> > > +_begin_fstest auto rw
> 
> Oh, I think it can be in 'stress' group, due to it's a fsstress random test, and
> it really takes long time on my system (with 24 cpus):
> 
> generic/726      1041s
> 
> And might take more time :)

Hmm.  Ok, back to the drawing board on this one then...

> Thanks,
> Zorro
> 
> > > +
> > > +# Override the default cleanup function.
> > > +_cleanup()
> > > +{
> > > +	cd /
> > > +	rm -f $tmp.*
> > > +	$KILLALL_PROG -9 fsstress > /dev/null 2>&1
> > 
> > At least there's "exercise_cpu_hotplug &", should we wait at here? Even we removed
> > $tmp.hotplug, can't make sure the process is over.

Yes.

> > 
> > > +	for i in "$sysfs_cpu_dir/"cpu*/online; do
> > > +		echo 1 > "$i" 2>/dev/null
> > > +	done
> > > +}
> > > +
> > > +exercise_cpu_hotplug()
> > > +{
> > > +	while [ -e $sentinel_file ]; do
> > > +		local idx=$(( RANDOM % nr_hotplug_cpus ))
> > > +		local cpu="${hotplug_cpus[idx]}"
> > > +		local action=$(( RANDOM % 2 ))
> > > +
> > > +		echo "$action" > "$sysfs_cpu_dir/cpu$cpu/online" 2>/dev/null
> > > +		sleep 0.5
> > > +	done
> > > +}
> > > +
> > > +# Import common functions.
> > > +
> > > +# Modify as appropriate.
> > 
> > Two useless comments at here?

Removed.

> > > +_supported_fs generic
> > > +
> > > +sysfs_cpu_dir="/sys/devices/system/cpu"
> > > +
> > > +# Figure out which CPU(s) support hotplug.
> > > +nrcpus=$(getconf _NPROCESSORS_CONF)
> > > +hotplug_cpus=()
> > > +for ((i = 0; i < nrcpus; i++ )); do
> > > +	test -e "$sysfs_cpu_dir/cpu$i/online" && hotplug_cpus+=("$i")
> > > +done
> > > +nr_hotplug_cpus="${#hotplug_cpus[@]}"
> > > +test "$nr_hotplug_cpus" -gt 0 || _notrun "CPU hotplugging not supported"
> > 
> > Is that worth being a helper?

I would defer that to the second time someone wants to write a cpu
hotplug test.

> > > +
> > > +_require_scratch
> > > +_require_command "$KILLALL_PROG" "killall"
> > > +
> > > +echo "Silence is golden."
> > > +
> > > +_scratch_mkfs > $seqres.full 2>&1
> > > +_scratch_mount >> $seqres.full 2>&1
> > > +
> > > +sentinel_file=$tmp.hotplug
> > > +touch $sentinel_file
> > > +exercise_cpu_hotplug &
> > > +
> > > +nr_cpus=$((LOAD_FACTOR * 4))

Hm, this probably ought to be nr_cpus=$((LOAD_FACTOR * nr_hotplug_cpus))

> > > +nr_ops=$((10000 * nr_cpus * TIME_FACTOR))

And this nr_ops=$((25000 * TIME_FACTOR))

--D

> > > +$FSSTRESS_PROG $FSSTRESS_AVOID -w -d $SCRATCH_MNT -n $nr_ops -p $nr_cpus >> $seqres.full
> > > +rm -f $sentinel_file
> > > +
> > > +# success, all done
> > > +status=0
> > > +exit
> > > diff --git a/tests/generic/726.out b/tests/generic/726.out
> > > new file mode 100644
> > > index 00000000..6839f8ce
> > > --- /dev/null
> > > +++ b/tests/generic/726.out
> > > @@ -0,0 +1,2 @@
> > > +QA output created by 726
> > > +Silence is golden.
> > > 
>