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

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

[Jie Hai]

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));
 
-- 
2.33.0

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

[Jie Hai]

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));
>   

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

[Jie Hai]

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));
>   

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

[Jie Hai]

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));
>   

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

[Vinod Koul]

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

-- 
~Vinod

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

[Jie Hai]

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
> 

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

[Vinod Koul]

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
> > 

-- 
~Vinod

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

[Jie Hai]

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 <haijie1@huawei.com>
---
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