| Message ID | 173933094538.1758477.11313063681546904819.stgit@frogsfrogsfrogs (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [01/34] generic/476: fix fsstress process management | expand |
On Tue, Feb 11, 2025 at 07:33:48PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > The next patch in this series fixes various issues with the recently > added fstests process isolation scheme by running each new process in a > separate process group session. Unfortunately, the processes in the > session are created with SIGINT ignored by default because they are not > attached to the controlling terminal. Therefore, switch the kill signal > to SIGPIPE because that is usually fatal and not masked by default. > > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org> > --- > common/fuzzy | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) I thought I reviewed this, must have missed it. > diff --git a/common/fuzzy b/common/fuzzy > index 0a2d91542b561e..e9df956e721949 100644 > --- a/common/fuzzy > +++ b/common/fuzzy > @@ -891,7 +891,7 @@ __stress_xfs_scrub_loop() { > local runningfile="$2" > local scrub_startat="$3" > shift; shift; shift > - local sigint_ret="$(( $(kill -l SIGINT) + 128 ))" > + local signal_ret="$(( $(kill -l SIGPIPE) + 128 ))" > local scrublog="$tmp.scrub" > > while __stress_scrub_running "$scrub_startat" "$runningfile"; do > @@ -902,7 +902,7 @@ __stress_xfs_scrub_loop() { > _scratch_scrub "$@" &> $scrublog > res=$? > if [ "$res" -eq "$sigint_ret" ]; then s/sigint_ret/signal_ret/ Otherwise looks fine, so with that fixed: Reviewed-by: Dave Chinner <dchinner@redhat.com> -Dave.
On Wed, Feb 12, 2025 at 03:45:50PM +1100, Dave Chinner wrote: > On Tue, Feb 11, 2025 at 07:33:48PM -0800, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@kernel.org> > > > > The next patch in this series fixes various issues with the recently > > added fstests process isolation scheme by running each new process in a > > separate process group session. Unfortunately, the processes in the > > session are created with SIGINT ignored by default because they are not > > attached to the controlling terminal. Therefore, switch the kill signal > > to SIGPIPE because that is usually fatal and not masked by default. > > > > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org> > > --- > > common/fuzzy | 13 ++++++------- > > 1 file changed, 6 insertions(+), 7 deletions(-) > > I thought I reviewed this, must have missed it. > > > diff --git a/common/fuzzy b/common/fuzzy > > index 0a2d91542b561e..e9df956e721949 100644 > > --- a/common/fuzzy > > +++ b/common/fuzzy > > @@ -891,7 +891,7 @@ __stress_xfs_scrub_loop() { > > local runningfile="$2" > > local scrub_startat="$3" > > shift; shift; shift > > - local sigint_ret="$(( $(kill -l SIGINT) + 128 ))" > > + local signal_ret="$(( $(kill -l SIGPIPE) + 128 ))" > > local scrublog="$tmp.scrub" > > > > while __stress_scrub_running "$scrub_startat" "$runningfile"; do > > @@ -902,7 +902,7 @@ __stress_xfs_scrub_loop() { > > _scratch_scrub "$@" &> $scrublog > > res=$? > > if [ "$res" -eq "$sigint_ret" ]; then > > s/sigint_ret/signal_ret/ Heh, whoops. Will fix. > Otherwise looks fine, so with that fixed: > > Reviewed-by: Dave Chinner <dchinner@redhat.com> Thanks! --D > -Dave. > > -- > Dave Chinner > david@fromorbit.com >
From: Darrick J. Wong <djwong@kernel.org> The next patch in this series fixes various issues with the recently added fstests process isolation scheme by running each new process in a separate process group session. Unfortunately, the processes in the session are created with SIGINT ignored by default because they are not attached to the controlling terminal. Therefore, switch the kill signal to SIGPIPE because that is usually fatal and not masked by default. Signed-off-by: "Darrick J. Wong" <djwong@kernel.org> Reviewed-by: Dave Chinner <dchinner@redhat.com> --- v3.1: fix bash typo --- common/fuzzy | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/common/fuzzy b/common/fuzzy index f9f2261365d2e2..dd6f04caad60ed 100644 --- a/common/fuzzy +++ b/common/fuzzy @@ -900,7 +900,7 @@ __stress_xfs_scrub_loop() { local runningfile="$2" local scrub_startat="$3" shift; shift; shift - local sigint_ret="$(( $(kill -l SIGINT) + 128 ))" + local signal_ret="$(( $(kill -l SIGPIPE) + 128 ))" local scrublog="$tmp.scrub" local now local compress @@ -921,8 +921,8 @@ __stress_xfs_scrub_loop() { _scratch_scrub "$@" &> $scrublog res=$? fi - if [ "$res" -eq "$sigint_ret" ]; then - # Ignore SIGINT because the cleanup function sends + if [ "$res" -eq "$signal_ret" ]; then + # Ignore SIGPIPE because the cleanup function sends # that to terminate xfs_scrub res=0 fi @@ -1203,13 +1203,11 @@ _scratch_xfs_stress_scrub_cleanup() { rm -f "$runningfile" echo "Cleaning up scrub stress run at $(date)" >> $seqres.full - # Send SIGINT so that bash won't print a 'Terminated' message that - # distorts the golden output. echo "Killing stressor processes at $(date)" >> $seqres.full _kill_fsstress - pkill -INT --parent $$ xfs_io >> $seqres.full 2>&1 - pkill -INT --parent $$ fsx >> $seqres.full 2>&1 - pkill -INT --parent $$ xfs_scrub >> $seqres.full 2>&1 + pkill -PIPE --parent $$ xfs_io >> $seqres.full 2>&1 + pkill -PIPE --parent $$ fsx >> $seqres.full 2>&1 + pkill -PIPE --parent $$ xfs_scrub >> $seqres.full 2>&1 # Tests are not allowed to exit with the scratch fs frozen. If we # started a fs freeze/thaw background loop, wait for that loop to exit @@ -1239,6 +1237,7 @@ _scratch_xfs_stress_scrub_cleanup() { # Wait for the remaining children to exit. echo "Waiting for children to exit at $(date)" >> $seqres.full wait + echo "Children exited as of $(date)" >> $seqres.full # Ensure the scratch fs is also writable before we exit. if [ -n "$__SCRUB_STRESS_REMOUNT_LOOP" ]; then
diff --git a/common/fuzzy b/common/fuzzy index 0a2d91542b561e..e9df956e721949 100644 --- a/common/fuzzy +++ b/common/fuzzy @@ -891,7 +891,7 @@ __stress_xfs_scrub_loop() { local runningfile="$2" local scrub_startat="$3" shift; shift; shift - local sigint_ret="$(( $(kill -l SIGINT) + 128 ))" + local signal_ret="$(( $(kill -l SIGPIPE) + 128 ))" local scrublog="$tmp.scrub" while __stress_scrub_running "$scrub_startat" "$runningfile"; do @@ -902,7 +902,7 @@ __stress_xfs_scrub_loop() { _scratch_scrub "$@" &> $scrublog res=$? if [ "$res" -eq "$sigint_ret" ]; then - # Ignore SIGINT because the cleanup function sends + # Ignore SIGPIPE because the cleanup function sends # that to terminate xfs_scrub res=0 fi @@ -1173,13 +1173,11 @@ _scratch_xfs_stress_scrub_cleanup() { rm -f "$runningfile" echo "Cleaning up scrub stress run at $(date)" >> $seqres.full - # Send SIGINT so that bash won't print a 'Terminated' message that - # distorts the golden output. echo "Killing stressor processes at $(date)" >> $seqres.full _kill_fsstress - pkill -INT --parent $$ xfs_io >> $seqres.full 2>&1 - pkill -INT --parent $$ fsx >> $seqres.full 2>&1 - pkill -INT --parent $$ xfs_scrub >> $seqres.full 2>&1 + pkill -PIPE --parent $$ xfs_io >> $seqres.full 2>&1 + pkill -PIPE --parent $$ fsx >> $seqres.full 2>&1 + pkill -PIPE --parent $$ xfs_scrub >> $seqres.full 2>&1 # Tests are not allowed to exit with the scratch fs frozen. If we # started a fs freeze/thaw background loop, wait for that loop to exit @@ -1209,6 +1207,7 @@ _scratch_xfs_stress_scrub_cleanup() { # Wait for the remaining children to exit. echo "Waiting for children to exit at $(date)" >> $seqres.full wait + echo "Children exited as of $(date)" >> $seqres.full # Ensure the scratch fs is also writable before we exit. if [ -n "$__SCRUB_STRESS_REMOUNT_LOOP" ]; then