[PATCH] blktests: Ignore errors from wait(1)

[PATCH] blktests: Ignore errors from wait(1)

[Jan Kara]

Multiple blktests use wait(1) to wait for background tasks. However in
some cases tasks can exit before wait(1) is called and in that case
wait(1) complains which breaks expected output. Make sure we ignore
output from wait(1) to avoid this breakage.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 tests/block/016 | 2 +-
 tests/block/017 | 2 +-
 tests/block/018 | 2 +-
 tests/block/024 | 2 +-
 tests/block/029 | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/block/016 b/tests/block/016
index 775069c..d399ec6 100755
--- a/tests/block/016
+++ b/tests/block/016
@@ -41,7 +41,7 @@ test() {
 	# While dd is blocked, send a signal which we know dd has a handler
 	# for.
 	kill -USR1 $!
-	wait
+	wait &>/dev/null
 
 	_exit_null_blk
 
diff --git a/tests/block/017 b/tests/block/017
index 8596888..ff68e24 100755
--- a/tests/block/017
+++ b/tests/block/017
@@ -40,7 +40,7 @@ test() {
 	sleep 0.1
 	show_inflight
 
-	wait
+	wait &>/dev/null
 	show_inflight
 
 	_exit_null_blk
diff --git a/tests/block/018 b/tests/block/018
index e7ac445..d2c97ea 100755
--- a/tests/block/018
+++ b/tests/block/018
@@ -50,7 +50,7 @@ test() {
 	dd if=/dev/nullb1 of=/dev/null bs=4096 iflag=direct count=1 status=none &
 	dd if=/dev/zero of=/dev/nullb1 bs=4096 oflag=direct count=1 status=none &
 	dd if=/dev/zero of=/dev/nullb1 bs=4096 oflag=direct count=1 status=none &
-	wait
+	wait &>/dev/null
 	show_times
 
 	_exit_null_blk
diff --git a/tests/block/024 b/tests/block/024
index 2a7c934..dd99f0c 100755
--- a/tests/block/024
+++ b/tests/block/024
@@ -57,7 +57,7 @@ test() {
 	dd if=/dev/nullb1 of=/dev/null bs=4096 iflag=direct count=1500 status=none &
 	dd if=/dev/zero of=/dev/nullb1 bs=4096 oflag=direct count=1800 status=none &
 	dd if=/dev/zero of=/dev/nullb1 bs=4096 oflag=direct count=1800 status=none &
-	wait
+	wait &>/dev/null
 	show_times
 
 	_exit_null_blk
diff --git a/tests/block/029 b/tests/block/029
index b9a897d..cb8fd03 100755
--- a/tests/block/029
+++ b/tests/block/029
@@ -41,7 +41,7 @@ test() {
 		    --runtime="${TIMEOUT}" --name=nullb1 \
 		    --output="${RESULTS_DIR}/block/fio-output-029.txt" \
 		    >>"$FULL"
-		wait
+		wait &>/dev/null
 	else
 		echo "Skipping test because $sq cannot be modified" >>"$FULL"
 	fi
-- 
2.26.2

Re: [PATCH] blktests: Ignore errors from wait(1)

[Chaitanya Kulkarni]

On 6/13/22 08:17, Jan Kara wrote:
> Multiple blktests use wait(1) to wait for background tasks. However in
> some cases tasks can exit before wait(1) is called and in that case
> wait(1) complains which breaks expected output. Make sure we ignore
> output from wait(1) to avoid this breakage.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>

Please note that Shinichiro (CC'd here) is a new blktests
maintainer and not Omar.

Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH] blktests: Ignore errors from wait(1)

[Shinichiro Kawasaki]

On Jun 13, 2022 / 17:17, Jan Kara wrote:
> Multiple blktests use wait(1) to wait for background tasks. However in
> some cases tasks can exit before wait(1) is called and in that case
> wait(1) complains which breaks expected output. Make sure we ignore
> output from wait(1) to avoid this breakage.

Hi Jan, thanks for the patch.

May I know how to create the wait(1) complaint message? I added sleep command
before the waits to ensure the background tasks completed, but was not able to
see message by wait on my test system. I suspect it may depend on bash version.

-- 
Shin'ichiro Kawasaki

Re: [PATCH] blktests: Ignore errors from wait(1)

[Jan Kara]

On Mon 13-06-22 22:48:20, Chaitanya Kulkarni wrote:
> On 6/13/22 08:17, Jan Kara wrote:
> > Multiple blktests use wait(1) to wait for background tasks. However in
> > some cases tasks can exit before wait(1) is called and in that case
> > wait(1) complains which breaks expected output. Make sure we ignore
> > output from wait(1) to avoid this breakage.
> > 
> > Signed-off-by: Jan Kara <jack@suse.cz>
> 
> Please note that Shinichiro (CC'd here) is a new blktests
> maintainer and not Omar.

Thanks for review and notifying me. Maybe CONTRIBUTING.md should be updated
to speak about Shinichiro and not Omar?

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] blktests: Ignore errors from wait(1)

[Jan Kara]

On Tue 14-06-22 07:04:54, Shinichiro Kawasaki wrote:
> On Jun 13, 2022 / 17:17, Jan Kara wrote:
> > Multiple blktests use wait(1) to wait for background tasks. However in
> > some cases tasks can exit before wait(1) is called and in that case
> > wait(1) complains which breaks expected output. Make sure we ignore
> > output from wait(1) to avoid this breakage.
> 
> Hi Jan, thanks for the patch.
> 
> May I know how to create the wait(1) complaint message? I added sleep
> command before the waits to ensure the background tasks completed, but
> was not able to see message by wait on my test system. I suspect it may
> depend on bash version.

Yes, I suspect it depends on the shell as well (because otherwise I expect
people would hit this much earlier than me :). The bash I have is
"4.4.23(1)-release" - the one in openSUSE 15.3 and it shows the error
pretty reliably...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] blktests: Ignore errors from wait(1)

[Shinichiro Kawasaki]

On Jun 14, 2022 / 15:15, Jan Kara wrote:
> On Mon 13-06-22 22:48:20, Chaitanya Kulkarni wrote:
> > On 6/13/22 08:17, Jan Kara wrote:
> > > Multiple blktests use wait(1) to wait for background tasks. However in
> > > some cases tasks can exit before wait(1) is called and in that case
> > > wait(1) complains which breaks expected output. Make sure we ignore
> > > output from wait(1) to avoid this breakage.
> > > 
> > > Signed-off-by: Jan Kara <jack@suse.cz>
> > 
> > Please note that Shinichiro (CC'd here) is a new blktests
> > maintainer and not Omar.
> 
> Thanks for review and notifying me. Maybe CONTRIBUTING.md should be updated
> to speak about Shinichiro and not Omar?

Yes, it was done, just a few weeks ago :)

https://lore.kernel.org/linux-block/20220525020424.14131-1-shinichiro.kawasaki@wdc.com/

-- 
Shin'ichiro Kawasaki

Re: [PATCH] blktests: Ignore errors from wait(1)

[Shinichiro Kawasaki]

On Jun 14, 2022 / 15:18, Jan Kara wrote:
> On Tue 14-06-22 07:04:54, Shinichiro Kawasaki wrote:

[...]

> Yes, I suspect it depends on the shell as well (because otherwise I expect
> people would hit this much earlier than me :). The bash I have is
> "4.4.23(1)-release" - the one in openSUSE 15.3 and it shows the error
> pretty reliably...

I guess the problematic wait command output is job exit status report. To
confirm my guess, could your share example of the wait command output?

Based on my guess, I checked difference between bash 4.4 and my bash 5.1, but I
did not find notable bash code change about job exit status report. Hmm.

Your patch covers several waits in the block group. I suspect other waits in
other groups may have same risk. Instead of your patch, could you try the patch
below on your system? If it works, all waits in all groups can be addressed.

diff --git a/check b/check
index 7037d88..ac24afa 100755
--- a/check
+++ b/check
@@ -440,6 +440,10 @@ _run_test() {
 	RUN_FOR_ZONED=0
 	FALLBACK_DEVICE=0
 
+	# Ensure job control monitor mode is off in this sub-shell to suppress
+	# job status output.
+	set +m
+
 	# shellcheck disable=SC1090
 	. "tests/${TEST_NAME}"
 


-- 
Shin'ichiro Kawasaki

Re: [PATCH] blktests: Ignore errors from wait(1)

[Jan Kara]

On Wed 15-06-22 11:50:14, Shinichiro Kawasaki wrote:
> On Jun 14, 2022 / 15:18, Jan Kara wrote:
> > On Tue 14-06-22 07:04:54, Shinichiro Kawasaki wrote:
> 
> [...]
> 
> > Yes, I suspect it depends on the shell as well (because otherwise I expect
> > people would hit this much earlier than me :). The bash I have is
> > "4.4.23(1)-release" - the one in openSUSE 15.3 and it shows the error
> > pretty reliably...
> 
> I guess the problematic wait command output is job exit status report. To
> confirm my guess, could your share example of the wait command output?
> 
> Based on my guess, I checked difference between bash 4.4 and my bash 5.1, but I
> did not find notable bash code change about job exit status report. Hmm.
> 
> Your patch covers several waits in the block group. I suspect other waits in
> other groups may have same risk. Instead of your patch, could you try the patch
> below on your system? If it works, all waits in all groups can be addressed.

Ah, indeed. This patch fixes the problem for me as well. Thanks for looking
into this!

								Honza

> 
> diff --git a/check b/check
> index 7037d88..ac24afa 100755
> --- a/check
> +++ b/check
> @@ -440,6 +440,10 @@ _run_test() {
>  	RUN_FOR_ZONED=0
>  	FALLBACK_DEVICE=0
>  
> +	# Ensure job control monitor mode is off in this sub-shell to suppress
> +	# job status output.
> +	set +m
> +
>  	# shellcheck disable=SC1090
>  	. "tests/${TEST_NAME}"
>  
> 
> 
> -- 
> Shin'ichiro Kawasaki
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] blktests: Ignore errors from wait(1)

[Shinichiro Kawasaki]

On Jun 15, 2022 / 18:56, Jan Kara wrote:
> On Wed 15-06-22 11:50:14, Shinichiro Kawasaki wrote:

[...]

> > Your patch covers several waits in the block group. I suspect other waits in
> > other groups may have same risk. Instead of your patch, could you try the patch
> > below on your system? If it works, all waits in all groups can be addressed.
> 
> Ah, indeed. This patch fixes the problem for me as well. Thanks for looking
> into this!

Okay, thanks! I've posted the official patch to go through the review process.

-- 
Shin'ichiro Kawasaki