Hi All,
vc->desc_free() used to be called in non atomic context which makes
sense to me. This changed over time and now vc->desc_free() is sometimes
called in atomic context and sometimes not.
The story starts with 13bb26ae8850 ("dmaengine: virt-dma: don't always
free descriptor upon completion"). This introduced a vc->desc_allocated
list which is mostly handled with the lock held, except in vchan_complete().
vchan_complete() moves the completed descs onto a separate list for the sake
of iterating over that list without the lock held allowing to call
vc->desc_free() without lock. 13bb26ae8850 changes this to:
@@ -83,8 +110,10 @@ static void vchan_complete(unsigned long arg)
cb_data = vd->tx.callback_param;
list_del(&vd->node);
-
- vc->desc_free(vd);
+ if (dmaengine_desc_test_reuse(&vd->tx))
+ list_add(&vd->node, &vc->desc_allocated);
+ else
+ vc->desc_free(vd);
vc->desc_free() is still called without lock, but the list operation is done
without locking as well which is wrong.
Now with 6af149d2b142 ("dmaengine: virt-dma: Add helper to free/reuse a
descriptor") the hunk above was moved to a separate function
(vchan_vdesc_fini()). With 1c7f072d94e8 ("dmaengine: virt-dma: Support for
race free transfer termination") the helper is started to be called with
lock held resulting in vc->desc_free() being called under the lock as
well. It is still called from vchan_complete() without lock.
I think vc->desc_free() being called under a spin_lock is unfortunate as
the i.MX SDMA driver does a dma_free_coherent() there which is required
to be called with interrupts enabled.
I am not sure where to go from here hence I'm writing this mail. Do we
agree that vc->desc_free() should be called without lock?
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
