Hi Vinod,
On 26.09.2017 18:40, Vinod Koul wrote:
On Tue, Sep 26, 2017 at 11:29:40AM +0200, Stefan Roese wrote:
Hi Sylvain,
On 26.09.2017 10:05, Sylvain Lesne wrote:
On 09/25/2017 04:38 PM, Stefan Roese wrote:
Hi Sylvain,
On 18.09.2017 13:08, Sylvain Lesne wrote:
Since this lock is acquired in both process and IRQ context, failing to
to disable IRQs when trying to acquire the lock in process context can
lead to deadlocks.
Signed-off-by: Sylvain Lesne <lesne@xxxxxxxxxxx>
---
I'm not sure that this is the "smartest" fix, but I think something
should be done to fix the spinlock usage in this driver (lockdep
agrees!).
I'm a bit unsure here as well, as introducing the irqsave spinlocks
here will be a bit costly time-wise.
Perhaps its better to move the addition of new descriptors in the IDLE
state from the ISR back the IRQ tasklet, as I've initially proposed in
v2 of this patch:
Keeping DMAengine idle is not really very smart! we would like to get
maximum throughput from it.
https://patchwork.kernel.org/patch/9805967/
This way, the lock will not be used from IRQ context any more and
the "normal" spinlock calls should be enough.
What do you think?
Thanks for the feedback!
I agree that simplifying the locks is probably the best idea.
Since Vinod suggested that you move this in the IRQ for performance
reasons, moving it back to the tasklet could cause a performance hit,
but this is probably better than adding irqsave everywhere.
Yes, I would prefer this alternative as well. Perhaps Vinod has
some comments as well?
Since the list is modified in irq and non irq context, using irqsave variant
of spinlock makes sense. Disabling irq here is okay for getting better
throughput.
Fine with me. Lets go with Sylvian's spinlock patch then.
Reviewed-by: Stefan Roese <sr@xxxxxxx>
Thanks,
Stefan
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
