| Message ID | 20250103031920.2868-2-lizhijian@fujitsu.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [blktests,v3,1/2] tests/rnbd: Add a basic RNBD test | expand |
On Jan 03, 2025 / 11:19, Li Zhijian wrote: > This test case has been in my possession for quite some time. > I am upstreaming it now because it has once again detected a regression in > a recent kernel release[0]. > > It's just stupid to connect and disconnect RNBD on localhost and expect > no dmesg exceptions, with some attempts actually succeeding. > > [0] https://lore.kernel.org/linux-rdma/20241223025700.292536-1-lizhijian@fujitsu.com/ > > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> > --- > V3: > - Always stop the rnbd regardless of the result of start Thanks for this V3. In my environment, this test case sometimes passes, but it still fails. With V2, it failed always, and the j counter had value 0 or 1 in most cases. With this V3, the j counter has values 5 or larger. When I repeat the test case, the pass ratio looks like around 50%. It was improved, but still not enough. IMO, the j > 10 check is dependent on the test environment too much and other blktests users will likely to see the failure. So I still suggest to remove the check. Instead, how about the report the j value? The change below will print it like this: rnbd/002 (Start Stop RNBD repeatly) [passed] runtime 51.674s ... 52.117s start/stop success ratio 9/100 ... 10/100 diff --git a/tests/rnbd/002 b/tests/rnbd/002 index 9ebec92..1d0598c 100755 --- a/tests/rnbd/002 +++ b/tests/rnbd/002 @@ -35,10 +35,7 @@ test_start_stop() _stop_rnbd_client &>/dev/null && ((j++)) done - # We expect at least 10% start/stop successfully - if [[ $j -lt 10 ]]; then - echo "Failed: $j/$i" - fi + TEST_RUN["start/stop success ratio"]="${j}/${i}" _cleanup_rnbd } [...] > diff --git a/tests/rnbd/002 b/tests/rnbd/002 > new file mode 100755 > index 000000000000..9ebec927db72 > --- /dev/null > +++ b/tests/rnbd/002 > @@ -0,0 +1,50 @@ > +#!/bin/bash > +# SPDX-License-Identifier: GPL-3.0+ > +# Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved. > +# > +# Commit 667db86bcbe8 ("RDMA/rtrs: Register ib event handler") introduced a > +# new element .deinit but never used it at all that lead to a > +# 'list_add corruption' kernel warning. > +# > +# This test is intended to check whether the current kernel is affected. > +# The following patch is able to fix this issue. > +# RDMA/rtrs: Add missing deinit() call > +# > +. tests/rnbd/rc > + > +DESCRIPTION="Start Stop RNBD repeatly" I think you meant s/repeatly/repeatedly/
On 07/01/2025 10:33, Shinichiro Kawasaki wrote: > On Jan 03, 2025 / 11:19, Li Zhijian wrote: >> This test case has been in my possession for quite some time. >> I am upstreaming it now because it has once again detected a regression in >> a recent kernel release[0]. >> >> It's just stupid to connect and disconnect RNBD on localhost and expect >> no dmesg exceptions, with some attempts actually succeeding. >> >> [0] https://lore.kernel.org/linux-rdma/20241223025700.292536-1-lizhijian@fujitsu.com/ >> >> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> >> --- >> V3: >> - Always stop the rnbd regardless of the result of start > > Thanks for this V3. In my environment, this test case sometimes passes, but > it still fails. With V2, it failed always, and the j counter had value 0 or 1 > in most cases. With this V3, the j counter has values 5 or larger. When I repeat > the test case, the pass ratio looks like around 50%. It was improved, but still > not enough. IMO, the j > 10 check is dependent on the test environment too much > and other blktests users will likely to see the failure. So I still suggest to > remove the check. Instead, how about the report the j value? The change below > will print it like this: > > rnbd/002 (Start Stop RNBD repeatly) [passed] > runtime 51.674s ... 52.117s > start/stop success ratio 9/100 ... 10/100 Well, it looks good to me. i will update it. Thanks for your code. > > > diff --git a/tests/rnbd/002 b/tests/rnbd/002 > index 9ebec92..1d0598c 100755 > --- a/tests/rnbd/002 > +++ b/tests/rnbd/002 > @@ -35,10 +35,7 @@ test_start_stop() > _stop_rnbd_client &>/dev/null && ((j++)) > done > > - # We expect at least 10% start/stop successfully > - if [[ $j -lt 10 ]]; then > - echo "Failed: $j/$i" > - fi > + TEST_RUN["start/stop success ratio"]="${j}/${i}" > > _cleanup_rnbd > } > > > [...] > >> diff --git a/tests/rnbd/002 b/tests/rnbd/002 >> new file mode 100755 >> index 000000000000..9ebec927db72 >> --- /dev/null >> +++ b/tests/rnbd/002 >> @@ -0,0 +1,50 @@ >> +#!/bin/bash >> +# SPDX-License-Identifier: GPL-3.0+ >> +# Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved. >> +# >> +# Commit 667db86bcbe8 ("RDMA/rtrs: Register ib event handler") introduced a >> +# new element .deinit but never used it at all that lead to a >> +# 'list_add corruption' kernel warning. >> +# >> +# This test is intended to check whether the current kernel is affected. >> +# The following patch is able to fix this issue. >> +# RDMA/rtrs: Add missing deinit() call >> +# >> +. tests/rnbd/rc >> + >> +DESCRIPTION="Start Stop RNBD repeatly" > > I think you meant s/repeatly/repeatedly/ Good catch!
diff --git a/tests/rnbd/002 b/tests/rnbd/002 new file mode 100755 index 000000000000..9ebec927db72 --- /dev/null +++ b/tests/rnbd/002 @@ -0,0 +1,50 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-3.0+ +# Copyright (c) 2024 FUJITSU LIMITED. All Rights Reserved. +# +# Commit 667db86bcbe8 ("RDMA/rtrs: Register ib event handler") introduced a +# new element .deinit but never used it at all that lead to a +# 'list_add corruption' kernel warning. +# +# This test is intended to check whether the current kernel is affected. +# The following patch is able to fix this issue. +# RDMA/rtrs: Add missing deinit() call +# +. tests/rnbd/rc + +DESCRIPTION="Start Stop RNBD repeatly" +CHECK_DMESG=1 +QUICK=1 + +requires() { + _have_rnbd + _have_loop +} + +test_start_stop() +{ + _setup_rnbd || return + + local loop_dev i j=0 + loop_dev="$(losetup -f)" + + for ((i=0;i<100;i++)) + do + _start_rnbd_client "${loop_dev}" &>/dev/null + # Always stop it so that the next start has change to work + _stop_rnbd_client &>/dev/null && ((j++)) + done + + # We expect at least 10% start/stop successfully + if [[ $j -lt 10 ]]; then + echo "Failed: $j/$i" + fi + + _cleanup_rnbd +} + +test() { + echo "Running ${TEST_NAME}" + test_start_stop + echo "Test complete" +} diff --git a/tests/rnbd/002.out b/tests/rnbd/002.out new file mode 100644 index 000000000000..2f055b8c82f9 --- /dev/null +++ b/tests/rnbd/002.out @@ -0,0 +1,2 @@ +Running rnbd/002 +Test complete