LinuxLists.cc - [PATCH] dmaengine: dmatest: fix timeout caused by kthread

2023-07-20 11:49:13

Subject: [PATCH] dmaengine: dmatest: fix timeout caused by kthread_stop

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 <[email protected]>
---
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

2023-09-25 01:11:47

by Jie Hai

[permalink] [raw]

Subject: Re: [PATCH] dmaengine: dmatest: fix timeout caused by kthread_stop

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 <[email protected]>
> ---
> 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));
>

2023-12-08 01:54:14

by Jie Hai

[permalink] [raw]

Subject: Re: [PATCH] dmaengine: dmatest: fix timeout caused by kthread_stop

2024-01-31 01:42:19

by Jie Hai

[permalink] [raw]

Subject: Re: [PATCH] dmaengine: dmatest: fix timeout caused by kthread_stop

2024-04-17 17:18:52

by Vinod Koul

[permalink] [raw]

Subject: Re: [PATCH] dmaengine: dmatest: fix timeout caused by kthread_stop

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 <[email protected]>
> ---
> 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

--
~Vinod

2024-04-23 03:40:41

by Jie Hai

[permalink] [raw]

Subject: Re: [PATCH] dmaengine: dmatest: fix timeout caused by kthread_stop

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 <[email protected]>
>> ---
>> 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
>

2024-04-25 09:12:01

by Vinod Koul

[permalink] [raw]

Subject: Re: [PATCH] dmaengine: dmatest: fix timeout caused by kthread_stop

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 <[email protected]>
> > > ---
> > > 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
> >

--
~Vinod

2024-04-25 12:46:35

by Jie Hai

[permalink] [raw]

Subject: [PATCH v2] dmaengine: dmatest: fix timeout caused by kthread_stop

The change introduced by commit a7c01fa93aeb ("signal: break
out of wait loops on kthread_stop()") may cause dmatest aborts
any ongoing tests and possible failure on the tests.

The operations are as follows:
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

And the failed dmesg logs are:
[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)

The test 57691 of dma0chan0-copy0 does not time out, it is the
signal brought by kthread_stop() interrupted the function
wait_event_freezable_timeout() waiting for test completing.
The timeout log is misleading and the ongoing test can be
completed if wait_event_freezable_timeout() is replaced by
wait_event_timeout().

Signed-off-by: Jie Hai <[email protected]>
---
v2:
1. Add more details in commit log.
---
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 a4f608837849..854165f55e7b 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -841,7 +841,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.30.0