| Message ID | 20230720114102.51053-1-haijie1@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | dmaengine: dmatest: fix timeout caused by kthread_stop | expand |
Hi, Vkoul, Kindly ping... Thanks, Jie Hai On 2023/7/20 19:41, Jie Hai wrote: > The change introduced by commit a7c01fa93aeb ("signal: break > out of wait loops on kthread_stop()") causes dmatest aborts > any ongoing tests and possible failure on the tests. This patch > use wait_event_timeout instead of wait_event_freezable_timeout > to avoid interrupting ongoing tests by signal brought by > kthread_stop(). > > Signed-off-by: Jie Hai <haijie1@huawei.com> > --- > drivers/dma/dmatest.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c > index ffe621695e47..c06b8b16645a 100644 > --- a/drivers/dma/dmatest.c > +++ b/drivers/dma/dmatest.c > @@ -827,7 +827,7 @@ static int dmatest_func(void *data) > } else { > dma_async_issue_pending(chan); > > - wait_event_freezable_timeout(thread->done_wait, > + ret = wait_event_timeout(thread->done_wait, > done->done, > msecs_to_jiffies(params->timeout)); >
Hi, Vkoul, Kindly ping... Thanks, Jie Hai On 2023/7/20 19:41, Jie Hai wrote: > The change introduced by commit a7c01fa93aeb ("signal: break > out of wait loops on kthread_stop()") causes dmatest aborts > any ongoing tests and possible failure on the tests. This patch > use wait_event_timeout instead of wait_event_freezable_timeout > to avoid interrupting ongoing tests by signal brought by > kthread_stop(). > > Signed-off-by: Jie Hai <haijie1@huawei.com> > --- > drivers/dma/dmatest.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c > index ffe621695e47..c06b8b16645a 100644 > --- a/drivers/dma/dmatest.c > +++ b/drivers/dma/dmatest.c > @@ -827,7 +827,7 @@ static int dmatest_func(void *data) > } else { > dma_async_issue_pending(chan); > > - wait_event_freezable_timeout(thread->done_wait, > + ret = wait_event_timeout(thread->done_wait, > done->done, > msecs_to_jiffies(params->timeout)); >
Hi, Vkoul, Kindly ping... Thanks, Jie Hai On 2023/7/20 19:41, Jie Hai wrote: > The change introduced by commit a7c01fa93aeb ("signal: break > out of wait loops on kthread_stop()") causes dmatest aborts > any ongoing tests and possible failure on the tests. This patch > use wait_event_timeout instead of wait_event_freezable_timeout > to avoid interrupting ongoing tests by signal brought by > kthread_stop(). > > Signed-off-by: Jie Hai <haijie1@huawei.com> > --- > drivers/dma/dmatest.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c > index ffe621695e47..c06b8b16645a 100644 > --- a/drivers/dma/dmatest.c > +++ b/drivers/dma/dmatest.c > @@ -827,7 +827,7 @@ static int dmatest_func(void *data) > } else { > dma_async_issue_pending(chan); > > - wait_event_freezable_timeout(thread->done_wait, > + ret = wait_event_timeout(thread->done_wait, > done->done, > msecs_to_jiffies(params->timeout)); >
On 20-07-23, 19:41, Jie Hai wrote: > The change introduced by commit a7c01fa93aeb ("signal: break > out of wait loops on kthread_stop()") causes dmatest aborts > any ongoing tests and possible failure on the tests. This patch Have you see this failure? Any log of that.. > use wait_event_timeout instead of wait_event_freezable_timeout > to avoid interrupting ongoing tests by signal brought by > kthread_stop(). > > Signed-off-by: Jie Hai <haijie1@huawei.com> > --- > drivers/dma/dmatest.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c > index ffe621695e47..c06b8b16645a 100644 > --- a/drivers/dma/dmatest.c > +++ b/drivers/dma/dmatest.c > @@ -827,7 +827,7 @@ static int dmatest_func(void *data) > } else { > dma_async_issue_pending(chan); > > - wait_event_freezable_timeout(thread->done_wait, > + ret = wait_event_timeout(thread->done_wait, > done->done, > msecs_to_jiffies(params->timeout)); > > -- > 2.33.0
Hi, Vinod Koul, Stop an ongoing test by "echo 0 > /sys/module/dmatest/parameters/run". If the current code is executed inside the while loop "while (!(kthread_should_stop() || (params->iterations && total_tests >= params->iterations)))" and before the call of "wait_event_freezable_timeout", the "wait_event_freezable_timeout" will be interrupted and result in "time out" for the test even if the test is not completed. Operations to the problem is as follows, and the failures are probabilistic: modprobe hisi_dma modprobe dmatest echo 0 > /sys/module/dmatest/parameters/iterations echo "dma0chan0" > /sys/module/dmatest/parameters/channel echo "dma0chan1" > /sys/module/dmatest/parameters/channel echo "dma0chan2" > /sys/module/dmatest/parameters/channel echo 1 > /sys/module/dmatest/parameters/run echo 0 > /sys/module/dmatest/parameters/run dmesg: [52575.636992] dmatest: Added 1 threads using dma0chan0 [52575.637555] dmatest: Added 1 threads using dma0chan1 [52575.638044] dmatest: Added 1 threads using dma0chan2 [52581.020355] dmatest: Started 1 threads using dma0chan0 [52581.020585] dmatest: Started 1 threads using dma0chan1 [52581.020814] dmatest: Started 1 threads using dma0chan2 [52587.705782] dmatest: dma0chan0-copy0: result #57691: 'test timed out' with src_off=0xfe6 dst_off=0x89 len=0x1d9a (0) [52587.706527] dmatest: dma0chan0-copy0: summary 57691 tests, 1 failures 51179.98 iops 411323 KB/s (0) [52587.707028] dmatest: dma0chan1-copy0: result #63178: 'test timed out' with src_off=0xdf dst_off=0x6ab len=0x389e (0) [52587.707767] dmatest: dma0chan1-copy0: summary 63178 tests, 1 failures 62851.60 iops 503835 KB/s (0) [52587.708376] dmatest: dma0chan2-copy0: result #60527: 'test timed out' with src_off=0x10e dst_off=0x58 len=0x3ea4 (0) [52587.708951] dmatest: dma0chan2-copy0: summary 60527 tests, 1 failures 52403.78 iops 420014 KB/s (0) On 2024/4/18 1:18, Vinod Koul wrote: > On 20-07-23, 19:41, Jie Hai wrote: >> The change introduced by commit a7c01fa93aeb ("signal: break >> out of wait loops on kthread_stop()") causes dmatest aborts >> any ongoing tests and possible failure on the tests. This patch > > Have you see this failure? Any log of that.. > >> use wait_event_timeout instead of wait_event_freezable_timeout >> to avoid interrupting ongoing tests by signal brought by >> kthread_stop(). >> >> Signed-off-by: Jie Hai <haijie1@huawei.com> >> --- >> drivers/dma/dmatest.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c >> index ffe621695e47..c06b8b16645a 100644 >> --- a/drivers/dma/dmatest.c >> +++ b/drivers/dma/dmatest.c >> @@ -827,7 +827,7 @@ static int dmatest_func(void *data) >> } else { >> dma_async_issue_pending(chan); >> >> - wait_event_freezable_timeout(thread->done_wait, >> + ret = wait_event_timeout(thread->done_wait, >> done->done, >> msecs_to_jiffies(params->timeout)); >> >> -- >> 2.33.0 >
On 23-04-24, 11:39, Jie Hai wrote: > Hi, Vinod Koul, > > Stop an ongoing test by > "echo 0 > /sys/module/dmatest/parameters/run". > If the current code is executed inside the while loop > "while (!(kthread_should_stop() || > (params->iterations && > total_tests >= params->iterations)))" > and before the call of "wait_event_freezable_timeout", > the "wait_event_freezable_timeout" will be interrupted > and result in "time out" for the test even if the test > is not completed. > > > > Operations to the problem is as follows, > and the failures are probabilistic: > > modprobe hisi_dma > modprobe dmatest > > echo 0 > /sys/module/dmatest/parameters/iterations > echo "dma0chan0" > /sys/module/dmatest/parameters/channel > echo "dma0chan1" > /sys/module/dmatest/parameters/channel > echo "dma0chan2" > /sys/module/dmatest/parameters/channel > echo 1 > /sys/module/dmatest/parameters/run > echo 0 > /sys/module/dmatest/parameters/run > > dmesg: > > [52575.636992] dmatest: Added 1 threads using dma0chan0 > [52575.637555] dmatest: Added 1 threads using dma0chan1 > [52575.638044] dmatest: Added 1 threads using dma0chan2 > [52581.020355] dmatest: Started 1 threads using dma0chan0 > [52581.020585] dmatest: Started 1 threads using dma0chan1 > [52581.020814] dmatest: Started 1 threads using dma0chan2 > [52587.705782] dmatest: dma0chan0-copy0: result #57691: 'test timed out' > with src_off=0xfe6 dst_off=0x89 len=0x1d9a (0) > [52587.706527] dmatest: dma0chan0-copy0: summary 57691 tests, 1 failures > 51179.98 iops 411323 KB/s (0) > [52587.707028] dmatest: dma0chan1-copy0: result #63178: 'test timed out' > with src_off=0xdf dst_off=0x6ab len=0x389e (0) > [52587.707767] dmatest: dma0chan1-copy0: summary 63178 tests, 1 failures > 62851.60 iops 503835 KB/s (0) > [52587.708376] dmatest: dma0chan2-copy0: result #60527: 'test timed out' > with src_off=0x10e dst_off=0x58 len=0x3ea4 (0) > [52587.708951] dmatest: dma0chan2-copy0: summary 60527 tests, 1 failures > 52403.78 iops 420014 KB/s (0) This is usefel in the commitlog, please add this and update the patchset > > > On 2024/4/18 1:18, Vinod Koul wrote: > > On 20-07-23, 19:41, Jie Hai wrote: > > > The change introduced by commit a7c01fa93aeb ("signal: break > > > out of wait loops on kthread_stop()") causes dmatest aborts > > > any ongoing tests and possible failure on the tests. This patch > > > > Have you see this failure? Any log of that.. > > > > > use wait_event_timeout instead of wait_event_freezable_timeout > > > to avoid interrupting ongoing tests by signal brought by > > > kthread_stop(). > > > > > > Signed-off-by: Jie Hai <haijie1@huawei.com> > > > --- > > > drivers/dma/dmatest.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c > > > index ffe621695e47..c06b8b16645a 100644 > > > --- a/drivers/dma/dmatest.c > > > +++ b/drivers/dma/dmatest.c > > > @@ -827,7 +827,7 @@ static int dmatest_func(void *data) > > > } else { > > > dma_async_issue_pending(chan); > > > - wait_event_freezable_timeout(thread->done_wait, > > > + ret = wait_event_timeout(thread->done_wait, > > > done->done, > > > msecs_to_jiffies(params->timeout)); > > > -- > > > 2.33.0 > >
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c index ffe621695e47..c06b8b16645a 100644 --- a/drivers/dma/dmatest.c +++ b/drivers/dma/dmatest.c @@ -827,7 +827,7 @@ static int dmatest_func(void *data) } else { dma_async_issue_pending(chan); - wait_event_freezable_timeout(thread->done_wait, + ret = wait_event_timeout(thread->done_wait, done->done, msecs_to_jiffies(params->timeout));
The change introduced by commit a7c01fa93aeb ("signal: break out of wait loops on kthread_stop()") causes dmatest aborts any ongoing tests and possible failure on the tests. This patch use wait_event_timeout instead of wait_event_freezable_timeout to avoid interrupting ongoing tests by signal brought by kthread_stop(). Signed-off-by: Jie Hai <haijie1@huawei.com> --- drivers/dma/dmatest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)