Message ID | 20180803072016.21544-1-sjhuang@iluvatar.ai (mailing list archive) |
---|---|
Headers | show |
Series | Use dmaenginem_async_device_register to simplify code | expand |
On 08/03/2018 09:19 AM, Huang Shijie wrote: > All the patches are using dmaenginem_async_device_register to simplify code > except the last one: > dmaengine: add COMPILE_TEST for the drivers > > I use the last one to do the compiler test. > There are still 20 drivers which do not use the dmaenginem_async_device_register. > Let me take a rest, if this patch set is accepted, I will do the rest. Lots of race conditions in this series. The DMA device needs to be removed before any of the resources it uses are disabled/released. As a rule of thumb you can only convert something to a managed allocation/reregistration if it is the last action in remove. -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 03, 2018 at 09:51:43AM +0200, Lars-Peter Clausen wrote: > On 08/03/2018 09:19 AM, Huang Shijie wrote: > > All the patches are using dmaenginem_async_device_register to simplify code > > except the last one: > > dmaengine: add COMPILE_TEST for the drivers > > > > I use the last one to do the compiler test. > > There are still 20 drivers which do not use the dmaenginem_async_device_register. > > Let me take a rest, if this patch set is accepted, I will do the rest. > > Lots of race conditions in this series. The DMA device needs to be removed > before any of the resources it uses are disabled/released. > > As a rule of thumb you can only convert something to a managed > allocation/reregistration if it is the last action in remove. Yes. Agree. I am collecting the DMA maintainers opinions now.. If the managed unregistration is not the last action, we can drop the patch. Thanks Huang Shijie -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 2018-08-03 10:19, Huang Shijie wrote: > All the patches are using dmaenginem_async_device_register to simplify code > except the last one: > dmaengine: add COMPILE_TEST for the drivers > > I use the last one to do the compiler test. > There are still 20 drivers which do not use the dmaenginem_async_device_register. > Let me take a rest, if this patch set is accepted, I will do the rest. I think for most of the drivers this series is going to open a race which is essentially: prior: _remove() { ... dma_async_device_register() /* Free resources, disable HW, etc */ return 0; } after: _remove() { ... /* Free resources, disable HW, etc */ /* * The dma device is still registered and a channel can be * requested */ dma_async_device_register() return 0; } It might be theoretical, but conversion to managed device resources is not straight forward in some cases. - Péter > > Huang Shijie (46): > dmaengine: zx_dma: use dmaenginem_async_device_register to simplify > the code > dmaengine: zynqmp_dma: use dmaenginem_async_device_register to > simplify the code > dmaengine: xilinx_dma: use dmaenginem_async_device_register to > simplify the code > dmaengine: xgene-dma: use dmaenginem_async_device_register to simplify > the code > dmaengine: txx9dmac: use dmaenginem_async_device_register to simplify > the code > dmaengine: timb_dma: use dmaenginem_async_device_register to simplify > the code > dmaengine: omap-dma: use dmaenginem_async_device_register to simplify > the code > dmaengine: edma: use dmaenginem_async_device_register to simplify the > code > dmaengine: cppi41: use dmaenginem_async_device_register to simplify > the code > dmaengine: tegra210-adma: use helper dmaenginem_async_device_register > dmaengine: tegra20-apb-dma: use helper > dmaenginem_async_device_register > dmaengine: sun6i-dma: use helper dmaenginem_async_device_register > dmaengine: sun4i-dma: use dmaenginem_async_device_register to simplify > the code > dmaengine: coh901318: use dmaenginem_async_device_register to simplify > the code > dmaengine: s3c24xx-dma: use dmaenginem_async_device_register to > simplify the code > dmaengine: ste_dma40: use dmaenginem_async_device_register to simplify > the code > dmaengine: stm32-mdma: use dmaenginem_async_device_register to > simplify the code > dmaengine: stm32-dma: use dmaenginem_async_device_register to simplify > the code > dmaengine: sprd-dma: use dmaenginem_async_device_register to simplify > the code > dmaengine: sirf-dma: use dmaenginem_async_device_register to simplify > the code > dmaengine: bam_dma: use dmaenginem_async_device_register to simplify > the code > dmaengine: sudmac: use dmaenginem_async_device_register to simplify > the code > dmaengine: sa11x0-dma: use dmaenginem_async_device_register to > simplify the code > dmaengine: nbpfaxi: use dmaenginem_async_device_register to simplify > the code > dmaengine: mmp_tdma: use dmaenginem_async_device_register to simplify > the code > dmaengine: shdmac: use dmaenginem_async_device_register to simplify > the code > dmaengine: usb-dmac: use dmaenginem_async_device_register to simplify > the code > dmaengine: rcar-dmac: use dmaenginem_async_device_register to simplify > the code > dmaengine: hidma: use dmaenginem_async_device_register to simplify the > code > dmaengine: pxa_dma: use dmaenginem_async_device_register to simplify > the code > dmaengine: moxart-dma: use dmaenginem_async_device_register to > simplify the code > dmaengine: pl330: use dmaenginem_async_device_register to simplify the > code > dmaengine: pch_dma: use dmaenginem_async_device_register to simplify > the code > dmaengine: mxs-dma: use dmaenginem_async_device_register to simplify > the code > dmaengine: mtk-hsdma: use dmaenginem_async_device_register to simplify > the code > dmaengine: k3dma: use dmaenginem_async_device_register to simplify the > code > dmaengine: imx-sdma: use dmaenginem_async_device_register to simplify > the code > dmaengine: imx-dma: use dmaenginem_async_device_register to simplify > the code > dmaengine: img-mdc-dma: use dmaenginem_async_device_register to > simplify the code > dmaengine: fsl-edma: use dmaenginem_async_device_register to simplify > the code > dmaengine: at_hdmac: use dmaenginem_async_device_register to simplify > the code > dmaengine: at_xdmac: use dmaenginem_async_device_register to simplify > the code > dmaengine: dma-jz4780: use dmaenginem_async_device_register to > simplify the code > dmaengine: dma-jz4740: use dmaenginem_async_device_register to > simplify the code > dmaengine: dma-axi-dmac: use dmaenginem_async_device_register to > simplify the code > dmaengine: add COMPILE_TEST for the drivers > > drivers/dma/Kconfig | 24 ++++++++++++------------ > drivers/dma/at_hdmac.c | 4 +--- > drivers/dma/at_xdmac.c | 7 ++----- > drivers/dma/coh901318.c | 14 ++++---------- > drivers/dma/dma-axi-dmac.c | 7 ++----- > drivers/dma/dma-jz4740.c | 7 ++----- > drivers/dma/dma-jz4780.c | 8 ++------ > drivers/dma/fsl-edma.c | 4 +--- > drivers/dma/img-mdc-dma.c | 7 ++----- > drivers/dma/imx-dma.c | 8 ++------ > drivers/dma/imx-sdma.c | 7 ++----- > drivers/dma/k3dma.c | 7 ++----- > drivers/dma/mediatek/mtk-hsdma.c | 4 +--- > drivers/dma/mmp_tdma.c | 7 ++----- > drivers/dma/moxart-dma.c | 5 +---- > drivers/dma/mxs-dma.c | 3 +-- > drivers/dma/nbpfaxi.c | 7 ++----- > drivers/dma/pch_dma.c | 4 +--- > drivers/dma/pl330.c | 4 +--- > drivers/dma/pxa_dma.c | 3 +-- > drivers/dma/qcom/bam_dma.c | 7 ++----- > drivers/dma/qcom/hidma.c | 3 +-- > drivers/dma/s3c24xx-dma.c | 11 +++-------- > drivers/dma/sa11x0-dma.c | 4 +--- > drivers/dma/sh/rcar-dmac.c | 5 +---- > drivers/dma/sh/shdmac.c | 5 +---- > drivers/dma/sh/sudmac.c | 4 +--- > drivers/dma/sh/usb-dmac.c | 3 +-- > drivers/dma/sirf-dma.c | 7 ++----- > drivers/dma/sprd-dma.c | 7 ++----- > drivers/dma/ste_dma40.c | 14 +++++--------- > drivers/dma/stm32-dma.c | 3 +-- > drivers/dma/stm32-mdma.c | 4 +--- > drivers/dma/sun4i-dma.c | 7 ++----- > drivers/dma/sun6i-dma.c | 7 ++----- > drivers/dma/tegra20-apb-dma.c | 8 ++------ > drivers/dma/tegra210-adma.c | 8 ++------ > drivers/dma/ti/Kconfig | 2 +- > drivers/dma/ti/cppi41.c | 7 ++----- > drivers/dma/ti/edma.c | 8 ++------ > drivers/dma/ti/omap-dma.c | 5 +---- > drivers/dma/timb_dma.c | 3 +-- > drivers/dma/txx9dmac.c | 4 +--- > drivers/dma/xgene-dma.c | 16 ++-------------- > drivers/dma/xilinx/xilinx_dma.c | 5 +---- > drivers/dma/xilinx/zynqmp_dma.c | 4 +--- > drivers/dma/zx_dma.c | 7 ++----- > 47 files changed, 88 insertions(+), 221 deletions(-) > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 03, 2018 at 11:20:49AM +0300, Peter Ujfalusi wrote: > Hi, > > On 2018-08-03 10:19, Huang Shijie wrote: > > All the patches are using dmaenginem_async_device_register to simplify code > > except the last one: > > dmaengine: add COMPILE_TEST for the drivers > > > > I use the last one to do the compiler test. > > There are still 20 drivers which do not use the dmaenginem_async_device_register. > > Let me take a rest, if this patch set is accepted, I will do the rest. > > I think for most of the drivers this series is going to open a race > which is essentially: > > prior: > _remove() > { > ... > dma_async_device_register() > /* Free resources, disable HW, etc */ > return 0; > } > > after: > _remove() > { > ... > /* Free resources, disable HW, etc */ > /* > * The dma device is still registered and a channel can be > * requested > */ > > dma_async_device_register() > return 0; > } > > It might be theoretical, but conversion to managed device resources is > not straight forward in some cases. Yes. okay, let me think again.. @Vinod, please just ignore this patch set. Thanks Huang Shijie -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 03, 2018 at 11:20:49AM +0300, Peter Ujfalusi wrote: > Hi, > > On 2018-08-03 10:19, Huang Shijie wrote: > > All the patches are using dmaenginem_async_device_register to simplify code > > except the last one: > > dmaengine: add COMPILE_TEST for the drivers > > > > I use the last one to do the compiler test. > > There are still 20 drivers which do not use the dmaenginem_async_device_register. > > Let me take a rest, if this patch set is accepted, I will do the rest. > > I think for most of the drivers this series is going to open a race > which is essentially: > > prior: > _remove() > { > ... > dma_async_device_register() > /* Free resources, disable HW, etc */ If these resources are managed by the devm_* APIs. We can use the dmaenginem_async_device_register(). The Devres will release all the things in the following order: ................................................ dma_async_device_unregister() /* Free resources, disable HW, etc */ ................................................ Thanks Huang Shijie -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html