Re: [PATCH v1 1/6] dmaengine: dmatest: Fix iteration non-stop logic

[Nicolas Ferre <nicolas.ferre@microchip.com>]

Andy,

On 24/04/2020 at 18:11, Andy Shevchenko wrote:
> Under some circumstances, i.e. when test is still running and about to
> time out and user runs, for example,
> 
>          grep -H . /sys/module/dmatest/parameters/*
> 
> the iterations parameter is not respected and test is going on and on until
> user gives
> 
>          echo 0 > /sys/module/dmatest/parameters/run
> 
> This is not what expected.
> 
> The history of this bug is interesting. I though that the commit
>    2d88ce76eb98 ("dmatest: add a 'wait' parameter")
> is a culprit, but looking closer to the code I think it simple revealed the
> broken logic from the day one, i.e. in the commit
>    0a2ff57d6fba ("dmaengine: dmatest: add a maximum number of test iterations")
> which adds iterations parameter.
> 
> So, to the point, the conditional of checking the thread to be stopped being
> first part of conjunction logic prevents to check iterations. Thus, we have to
> always check both conditions to be able to stop after given iterations.
> 
> Since it wasn't visible before second commit appeared, I add a respective
> Fixes tag.
> 
> Fixes: 2d88ce76eb98 ("dmatest: add a 'wait' parameter")
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Nicolas Ferre <nicolas.ferre@microchip.com>

Yes, makes sense indeed:
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>


> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   drivers/dma/dmatest.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
> index a2cadfa2e6d78..4993e3e5c5b01 100644
> --- a/drivers/dma/dmatest.c
> +++ b/drivers/dma/dmatest.c
> @@ -662,8 +662,8 @@ static int dmatest_func(void *data)
>                  flags = DMA_CTRL_ACK | DMA_PREP_INTERRUPT;
> 
>          ktime = ktime_get();
> -       while (!kthread_should_stop()
> -              && !(params->iterations && total_tests >= params->iterations)) {
> +       while (!(kthread_should_stop() ||
> +              (params->iterations && total_tests >= params->iterations))) {
>                  struct dma_async_tx_descriptor *tx = NULL;
>                  struct dmaengine_unmap_data *um;
>                  dma_addr_t *dsts;
> --
> 2.26.2
> 


-- 
Nicolas Ferre