| Message ID | 20221025090306.297886-1-tudor.ambarus@microchip.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | dmaengine: at_hdmac: Fix concurrency bugs and then convert to virt-dma | expand |
Hi, Peter, Can I have your tested-by tag on this patch set please? Thanks, ta
On 25/10/2022 at 11:02, Tudor Ambarus wrote: > v2: > - reorder patches so that fixes come first -> easier to backport to > stable kernels. > - drop the devm_request_irq() patch as we had to disable the irq anyway > in remove() in order to avoid spurios IRQs. Using devm variant brings no > palpable benefit. > - reword pm_ptr commit message > > > at_hdmac driver had poor list handling and concurrency bugs. > We experienced calling of the completion call twice for the > same descriptor. Peter Rosin encountered the same while > reporting a different bug: > https://lore.kernel.org/lkml/13c6c9a2-6db5-c3bf-349b-4c127ad3496a@axentia.se/ > > Two sets of tests were performed: > 1/ tested just the fixes, to make sure everything is fine and the > concurrency bugs are squashed even without the conversion to virt-dma. > All went fine. > 2/ tested the entire series including the conversion the virt-dma > All went fine. > > I tested NAND (prep_dma_memcpy), MMC (prep_dma_slave_sg), > usart (cyclic mode), dmatest (memcpy, memset). All these tests are reassuring. The important piece to preserve is the computation of the residue as it went with lots of experiments in both silicon and simulation with the IP designer. The errors were very difficult to reproduce. As this part seems well understood and maintained it its good conditions, I can give my Ack to the series. So, for the whole series, after a quick-but-not-too-quick review: Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com> Thanks for this effort Tudor. Best regards, Nicolas > With the conversion to virt-dma I replaced the election of a new transfer > in the tasklet with the election of the new transfer in the interrupt > handler. We should have a shorter idle window as we remove the scheduling > latency of the tasklet. Using mtd_speedtest showed similar performances > when using NAND with DMA. That could be because of using a low timming > mode on NAND. > > > Tudor Ambarus (32): > dmaengine: at_hdmac: Fix at_lli struct definition > dmaengine: at_hdmac: Don't start transactions at tx_submit level > dmaengine: at_hdmac: Start transfer for cyclic channels in > issue_pending > dmaengine: at_hdmac: Fix premature completion of desc in issue_pending > dmaengine: at_hdmac: Do not call the complete callback on > device_terminate_all > dmaengine: at_hdmac: Protect atchan->status with the channel lock > dmaengine: at_hdmac: Fix concurrency problems by removing > atc_complete_all() > dmaengine: at_hdmac: Fix concurrency over descriptor > dmaengine: at_hdmac: Free the memset buf without holding the chan lock > dmaengine: at_hdmac: Fix concurrency over the active list > dmaengine: at_hdmac: Fix descriptor handling when issuing it to > hardware > dmaengine: at_hdmac: Fix completion of unissued descriptor in case of > errors > dmaengine: at_hdmac: Don't allow CPU to reorder channel enable > dmaengine: at_hdmac: Fix impossible condition > dmaengine: at_hdmac: Check return code of dma_async_device_register > dmaengine: at_hdmac: Do not print messages on console while holding > the lock > dmaengine: at_hdmac: Return dma_cookie_status()'s ret code when > txstate is NULL > dmaengine: at_hdmac: Remove superfluous cast > dmaengine: at_hdmac: Pass residue by address to avoid unnecessary > implicit casts > dmaengine: at_hdmac: s/atc_get_bytes_left/atc_get_residue > dmaengine: at_hdmac: Introduce atc_get_llis_residue() > dmaengine: at_hdmac: Use devm_kzalloc() and struct_size() > dmaengine: at_hdmac: Use devm_platform_ioremap_resource > dmaengine: at_hdmac: Use devm_clk_get() > dmaengine: at_hdmac: Use pm_ptr() > dmaengine: at_hdmac: Set include entries in alphabetic order > dmaengine: at_hdmac: Keep register definitions and structures private > to at_hdmac.c > dmaengine: at_hdmac: Use bitfield access macros > dmaengine: at_hdmac: Rename "dma_common" to "dma_device" > dmaengine: at_hdmac: Rename "chan_common" to "dma_chan" > dmaengine: at_hdmac: Remove unused member of at_dma_chan > dmaengine: at_hdmac: Convert driver to use virt-dma > > MAINTAINERS | 1 - > drivers/dma/Kconfig | 1 + > drivers/dma/at_hdmac.c | 1899 ++++++++++++++++++----------------- > drivers/dma/at_hdmac_regs.h | 478 --------- > 4 files changed, 990 insertions(+), 1389 deletions(-) > delete mode 100644 drivers/dma/at_hdmac_regs.h >
On 25-10-22, 12:02, Tudor Ambarus wrote: > v2: > - reorder patches so that fixes come first -> easier to backport to > stable kernels. > - drop the devm_request_irq() patch as we had to disable the irq anyway > in remove() in order to avoid spurios IRQs. Using devm variant brings no > palpable benefit. > - reword pm_ptr commit message > > > at_hdmac driver had poor list handling and concurrency bugs. > We experienced calling of the completion call twice for the > same descriptor. Peter Rosin encountered the same while > reporting a different bug: > https://lore.kernel.org/lkml/13c6c9a2-6db5-c3bf-349b-4c127ad3496a@axentia.se/ > > Two sets of tests were performed: > 1/ tested just the fixes, to make sure everything is fine and the > concurrency bugs are squashed even without the conversion to virt-dma. > All went fine. > 2/ tested the entire series including the conversion the virt-dma > All went fine. > > I tested NAND (prep_dma_memcpy), MMC (prep_dma_slave_sg), > usart (cyclic mode), dmatest (memcpy, memset). > With the conversion to virt-dma I replaced the election of a new transfer > in the tasklet with the election of the new transfer in the interrupt > handler. We should have a shorter idle window as we remove the scheduling > latency of the tasklet. Using mtd_speedtest showed similar performances > when using NAND with DMA. That could be because of using a low timming > mode on NAND. This does not apply on dmaengine-fixes, can you please rebase and resend
On 11/4/22 15:22, Vinod Koul wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 25-10-22, 12:02, Tudor Ambarus wrote: >> v2: >> - reorder patches so that fixes come first -> easier to backport to >> stable kernels. >> - drop the devm_request_irq() patch as we had to disable the irq anyway >> in remove() in order to avoid spurios IRQs. Using devm variant brings no >> palpable benefit. >> - reword pm_ptr commit message >> >> >> at_hdmac driver had poor list handling and concurrency bugs. >> We experienced calling of the completion call twice for the >> same descriptor. Peter Rosin encountered the same while >> reporting a different bug: >> https://lore.kernel.org/lkml/13c6c9a2-6db5-c3bf-349b-4c127ad3496a@axentia.se/ >> >> Two sets of tests were performed: >> 1/ tested just the fixes, to make sure everything is fine and the >> concurrency bugs are squashed even without the conversion to virt-dma. >> All went fine. >> 2/ tested the entire series including the conversion the virt-dma >> All went fine. >> >> I tested NAND (prep_dma_memcpy), MMC (prep_dma_slave_sg), >> usart (cyclic mode), dmatest (memcpy, memset). >> With the conversion to virt-dma I replaced the election of a new transfer >> in the tasklet with the election of the new transfer in the interrupt >> handler. We should have a shorter idle window as we remove the scheduling >> latency of the tasklet. Using mtd_speedtest showed similar performances >> when using NAND with DMA. That could be because of using a low timming >> mode on NAND. > > This does not apply on dmaengine-fixes, can you please rebase and resend > Sure, I can, but on my side it applies just fine, both on the fixes and next branches. I used git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git Shall I use other tree? Thanks, ta $ git am ./v2_20221025_tudor_ambarus_dmaengine_at_hdmac_fix_concurrency_bugs_and_then_convert_to_virt_dma.mbx Applying: dmaengine: at_hdmac: Fix at_lli struct definition Applying: dmaengine: at_hdmac: Don't start transactions at tx_submit level Applying: dmaengine: at_hdmac: Start transfer for cyclic channels in issue_pending Applying: dmaengine: at_hdmac: Fix premature completion of desc in issue_pending Applying: dmaengine: at_hdmac: Do not call the complete callback on device_terminate_all Applying: dmaengine: at_hdmac: Protect atchan->status with the channel lock Applying: dmaengine: at_hdmac: Fix concurrency problems by removing atc_complete_all() Applying: dmaengine: at_hdmac: Fix concurrency over descriptor Applying: dmaengine: at_hdmac: Free the memset buf without holding the chan lock Applying: dmaengine: at_hdmac: Fix concurrency over the active list Applying: dmaengine: at_hdmac: Fix descriptor handling when issuing it to hardware Applying: dmaengine: at_hdmac: Fix completion of unissued descriptor in case of errors Applying: dmaengine: at_hdmac: Don't allow CPU to reorder channel enable Applying: dmaengine: at_hdmac: Fix impossible condition Applying: dmaengine: at_hdmac: Check return code of dma_async_device_register Applying: dmaengine: at_hdmac: Do not print messages on console while holding the lock Applying: dmaengine: at_hdmac: Return dma_cookie_status()'s ret code when txstate is NULL Applying: dmaengine: at_hdmac: Remove superfluous cast Applying: dmaengine: at_hdmac: Pass residue by address to avoid unnecessary implicit casts Applying: dmaengine: at_hdmac: s/atc_get_bytes_left/atc_get_residue Applying: dmaengine: at_hdmac: Introduce atc_get_llis_residue() Applying: dmaengine: at_hdmac: Use devm_kzalloc() and struct_size() Applying: dmaengine: at_hdmac: Use devm_platform_ioremap_resource Applying: dmaengine: at_hdmac: Use devm_clk_get() Applying: dmaengine: at_hdmac: Use pm_ptr() Applying: dmaengine: at_hdmac: Set include entries in alphabetic order Applying: dmaengine: at_hdmac: Keep register definitions and structures private to at_hdmac.c Applying: dmaengine: at_hdmac: Use bitfield access macros Applying: dmaengine: at_hdmac: Rename "dma_common" to "dma_device" Applying: dmaengine: at_hdmac: Rename "chan_common" to "dma_chan" Applying: dmaengine: at_hdmac: Remove unused member of at_dma_chan Applying: dmaengine: at_hdmac: Convert driver to use virt-dma b4 am 20221025090306.297886-1-tudor.ambarus@microchip.com Looking up https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus%40microchip.com Analyzing 37 messages in the thread Checking attestation on all messages, may take a moment... --- [PATCH v2 1/32] dmaengine: at_hdmac: Fix at_lli struct definition [PATCH v2 2/32] dmaengine: at_hdmac: Don't start transactions at tx_submit level [PATCH v2 3/32] dmaengine: at_hdmac: Start transfer for cyclic channels in issue_pending [PATCH v2 4/32] dmaengine: at_hdmac: Fix premature completion of desc in issue_pending [PATCH v2 5/32] dmaengine: at_hdmac: Do not call the complete callback on device_terminate_all [PATCH v2 6/32] dmaengine: at_hdmac: Protect atchan->status with the channel lock [PATCH v2 7/32] dmaengine: at_hdmac: Fix concurrency problems by removing atc_complete_all() [PATCH v2 8/32] dmaengine: at_hdmac: Fix concurrency over descriptor [PATCH v2 9/32] dmaengine: at_hdmac: Free the memset buf without holding the chan lock [PATCH v2 10/32] dmaengine: at_hdmac: Fix concurrency over the active list [PATCH v2 11/32] dmaengine: at_hdmac: Fix descriptor handling when issuing it to hardware [PATCH v2 12/32] dmaengine: at_hdmac: Fix completion of unissued descriptor in case of errors [PATCH v2 13/32] dmaengine: at_hdmac: Don't allow CPU to reorder channel enable [PATCH v2 14/32] dmaengine: at_hdmac: Fix impossible condition [PATCH v2 15/32] dmaengine: at_hdmac: Check return code of dma_async_device_register [PATCH v2 16/32] dmaengine: at_hdmac: Do not print messages on console while holding the lock [PATCH v2 17/32] dmaengine: at_hdmac: Return dma_cookie_status()'s ret code when txstate is NULL [PATCH v2 18/32] dmaengine: at_hdmac: Remove superfluous cast [PATCH v2 19/32] dmaengine: at_hdmac: Pass residue by address to avoid unnecessary implicit casts [PATCH v2 20/32] dmaengine: at_hdmac: s/atc_get_bytes_left/atc_get_residue [PATCH v2 21/32] dmaengine: at_hdmac: Introduce atc_get_llis_residue() [PATCH v2 22/32] dmaengine: at_hdmac: Use devm_kzalloc() and struct_size() [PATCH v2 23/32] dmaengine: at_hdmac: Use devm_platform_ioremap_resource [PATCH v2 24/32] dmaengine: at_hdmac: Use devm_clk_get() [PATCH v2 25/32] dmaengine: at_hdmac: Use pm_ptr() [PATCH v2 26/32] dmaengine: at_hdmac: Set include entries in alphabetic order [PATCH v2 27/32] dmaengine: at_hdmac: Keep register definitions and structures private to at_hdmac.c [PATCH v2 28/32] dmaengine: at_hdmac: Use bitfield access macros [PATCH v2 29/32] dmaengine: at_hdmac: Rename "dma_common" to "dma_device" [PATCH v2 30/32] dmaengine: at_hdmac: Rename "chan_common" to "dma_chan" [PATCH v2 31/32] dmaengine: at_hdmac: Remove unused member of at_dma_chan [PATCH v2 32/32] dmaengine: at_hdmac: Convert driver to use virt-dma + Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com> --- Total patches: 32 --- NOTE: Some trailers were sent to the cover letter: Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com> NOTE: Rerun with -t to apply them to all patches --- Cover: ./v2_20221025_tudor_ambarus_dmaengine_at_hdmac_fix_concurrency_bugs_and_then_convert_to_virt_dma.cover Link: https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus@microchip.com Base: not specified git am ./v2_20221025_tudor_ambarus_dmaengine_at_hdmac_fix_concurrency_bugs_and_then_convert_to_virt_dma.mbx $ git log --oneline 79029dc341c7 (HEAD, vkoul-dma/fixes) dmaengine: stm32-dma: fix potential race between pause and resume
>> >> This does not apply on dmaengine-fixes, can you please rebase and resend >> > > Sure, I can, but on my side it applies just fine, both on the fixes and next > branches. I used git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git > Shall I use other tree? $ git log --oneline 79029dc341c7 (HEAD, vkoul-dma/fixes) dmaengine: stm32-dma: fix potential race between pause and resume [cut] $ b4 am 20221025090306.297886-1-tudor.ambarus@microchip.com Looking up https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus%40microchip.com Grabbing thread from lore.kernel.org/all/20221025090306.297886-1-tudor.ambarus%40microchip.com/t.mbox.gz Analyzing 37 messages in the thread Checking attestation on all messages, may take a moment... --- [PATCH v2 1/32] dmaengine: at_hdmac: Fix at_lli struct definition [PATCH v2 2/32] dmaengine: at_hdmac: Don't start transactions at tx_submit level [PATCH v2 3/32] dmaengine: at_hdmac: Start transfer for cyclic channels in issue_pending [PATCH v2 4/32] dmaengine: at_hdmac: Fix premature completion of desc in issue_pending [PATCH v2 5/32] dmaengine: at_hdmac: Do not call the complete callback on device_terminate_all [PATCH v2 6/32] dmaengine: at_hdmac: Protect atchan->status with the channel lock [PATCH v2 7/32] dmaengine: at_hdmac: Fix concurrency problems by removing atc_complete_all() [PATCH v2 8/32] dmaengine: at_hdmac: Fix concurrency over descriptor [PATCH v2 9/32] dmaengine: at_hdmac: Free the memset buf without holding the chan lock [PATCH v2 10/32] dmaengine: at_hdmac: Fix concurrency over the active list [PATCH v2 11/32] dmaengine: at_hdmac: Fix descriptor handling when issuing it to hardware [PATCH v2 12/32] dmaengine: at_hdmac: Fix completion of unissued descriptor in case of errors [PATCH v2 13/32] dmaengine: at_hdmac: Don't allow CPU to reorder channel enable [PATCH v2 14/32] dmaengine: at_hdmac: Fix impossible condition [PATCH v2 15/32] dmaengine: at_hdmac: Check return code of dma_async_device_register [PATCH v2 16/32] dmaengine: at_hdmac: Do not print messages on console while holding the lock [PATCH v2 17/32] dmaengine: at_hdmac: Return dma_cookie_status()'s ret code when txstate is NULL [PATCH v2 18/32] dmaengine: at_hdmac: Remove superfluous cast [PATCH v2 19/32] dmaengine: at_hdmac: Pass residue by address to avoid unnecessary implicit casts [PATCH v2 20/32] dmaengine: at_hdmac: s/atc_get_bytes_left/atc_get_residue [PATCH v2 21/32] dmaengine: at_hdmac: Introduce atc_get_llis_residue() [PATCH v2 22/32] dmaengine: at_hdmac: Use devm_kzalloc() and struct_size() [PATCH v2 23/32] dmaengine: at_hdmac: Use devm_platform_ioremap_resource [PATCH v2 24/32] dmaengine: at_hdmac: Use devm_clk_get() [PATCH v2 25/32] dmaengine: at_hdmac: Use pm_ptr() [PATCH v2 26/32] dmaengine: at_hdmac: Set include entries in alphabetic order [PATCH v2 27/32] dmaengine: at_hdmac: Keep register definitions and structures private to at_hdmac.c [PATCH v2 28/32] dmaengine: at_hdmac: Use bitfield access macros [PATCH v2 29/32] dmaengine: at_hdmac: Rename "dma_common" to "dma_device" [PATCH v2 30/32] dmaengine: at_hdmac: Rename "chan_common" to "dma_chan" [PATCH v2 31/32] dmaengine: at_hdmac: Remove unused member of at_dma_chan [PATCH v2 32/32] dmaengine: at_hdmac: Convert driver to use virt-dma + Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com> --- Total patches: 32 --- NOTE: Some trailers were sent to the cover letter: Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com> NOTE: Rerun with -t to apply them to all patches --- Cover: ./v2_20221025_tudor_ambarus_dmaengine_at_hdmac_fix_concurrency_bugs_and_then_convert_to_virt_dma.cover Link: https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus@microchip.com Base: not specified git am ./v2_20221025_tudor_ambarus_dmaengine_at_hdmac_fix_concurrency_bugs_and_then_convert_to_virt_dma.mbx $ git am ./v2_20221025_tudor_ambarus_dmaengine_at_hdmac_fix_concurrency_bugs_and_then_convert_to_virt_dma.mbx Applying: dmaengine: at_hdmac: Fix at_lli struct definition Applying: dmaengine: at_hdmac: Don't start transactions at tx_submit level Applying: dmaengine: at_hdmac: Start transfer for cyclic channels in issue_pending Applying: dmaengine: at_hdmac: Fix premature completion of desc in issue_pending Applying: dmaengine: at_hdmac: Do not call the complete callback on device_terminate_all Applying: dmaengine: at_hdmac: Protect atchan->status with the channel lock Applying: dmaengine: at_hdmac: Fix concurrency problems by removing atc_complete_all() Applying: dmaengine: at_hdmac: Fix concurrency over descriptor Applying: dmaengine: at_hdmac: Free the memset buf without holding the chan lock Applying: dmaengine: at_hdmac: Fix concurrency over the active list Applying: dmaengine: at_hdmac: Fix descriptor handling when issuing it to hardware Applying: dmaengine: at_hdmac: Fix completion of unissued descriptor in case of errors Applying: dmaengine: at_hdmac: Don't allow CPU to reorder channel enable Applying: dmaengine: at_hdmac: Fix impossible condition Applying: dmaengine: at_hdmac: Check return code of dma_async_device_register Applying: dmaengine: at_hdmac: Do not print messages on console while holding the lock Applying: dmaengine: at_hdmac: Return dma_cookie_status()'s ret code when txstate is NULL Applying: dmaengine: at_hdmac: Remove superfluous cast Applying: dmaengine: at_hdmac: Pass residue by address to avoid unnecessary implicit casts Applying: dmaengine: at_hdmac: s/atc_get_bytes_left/atc_get_residue Applying: dmaengine: at_hdmac: Introduce atc_get_llis_residue() Applying: dmaengine: at_hdmac: Use devm_kzalloc() and struct_size() Applying: dmaengine: at_hdmac: Use devm_platform_ioremap_resource Applying: dmaengine: at_hdmac: Use devm_clk_get() Applying: dmaengine: at_hdmac: Use pm_ptr() Applying: dmaengine: at_hdmac: Set include entries in alphabetic order Applying: dmaengine: at_hdmac: Keep register definitions and structures private to at_hdmac.c Applying: dmaengine: at_hdmac: Use bitfield access macros Applying: dmaengine: at_hdmac: Rename "dma_common" to "dma_device" Applying: dmaengine: at_hdmac: Rename "chan_common" to "dma_chan" Applying: dmaengine: at_hdmac: Remove unused member of at_dma_chan Applying: dmaengine: at_hdmac: Convert driver to use virt-dma $ git log --oneline 0216a9804005 (HEAD) dmaengine: at_hdmac: Convert driver to use virt-dma 3c38e6f0c683 dmaengine: at_hdmac: Remove unused member of at_dma_chan 3f801af0fe24 dmaengine: at_hdmac: Rename "chan_common" to "dma_chan" deee5deb54d7 dmaengine: at_hdmac: Rename "dma_common" to "dma_device" 6561448a5bef dmaengine: at_hdmac: Use bitfield access macros 744ba3189722 dmaengine: at_hdmac: Keep register definitions and structures private to at_hdmac.c c7b1826777a1 dmaengine: at_hdmac: Set include entries in alphabetic order fb34dfd24504 dmaengine: at_hdmac: Use pm_ptr() 348276780e9c dmaengine: at_hdmac: Use devm_clk_get() 800462a74e78 dmaengine: at_hdmac: Use devm_platform_ioremap_resource d7236b9dd3b4 dmaengine: at_hdmac: Use devm_kzalloc() and struct_size() e6eaacf98492 dmaengine: at_hdmac: Introduce atc_get_llis_residue() 010cabae81ae dmaengine: at_hdmac: s/atc_get_bytes_left/atc_get_residue 9d09a47eb73a dmaengine: at_hdmac: Pass residue by address to avoid unnecessary implicit casts 2dd9e9cbb086 dmaengine: at_hdmac: Remove superfluous cast 5ef926e13fb8 dmaengine: at_hdmac: Return dma_cookie_status()'s ret code when txstate is NULL 3b09d91a06ef dmaengine: at_hdmac: Do not print messages on console while holding the lock e521dfad36a3 dmaengine: at_hdmac: Check return code of dma_async_device_register 5edd5e1c0a8d dmaengine: at_hdmac: Fix impossible condition 5621e6e7be9b dmaengine: at_hdmac: Don't allow CPU to reorder channel enable 263a0ea4bc91 dmaengine: at_hdmac: Fix completion of unissued descriptor in case of errors afb3cd55a2c1 dmaengine: at_hdmac: Fix descriptor handling when issuing it to hardware cc94d7bdae69 dmaengine: at_hdmac: Fix concurrency over the active list e654eceb0331 dmaengine: at_hdmac: Free the memset buf without holding the chan lock 2536923b7374 dmaengine: at_hdmac: Fix concurrency over descriptor c928306dea15 dmaengine: at_hdmac: Fix concurrency problems by removing atc_complete_all() 55f6c9b21ad1 dmaengine: at_hdmac: Protect atchan->status with the channel lock ecb13670616f dmaengine: at_hdmac: Do not call the complete callback on device_terminate_all e0926ead50f9 dmaengine: at_hdmac: Fix premature completion of desc in issue_pending 8354c91e9484 dmaengine: at_hdmac: Start transfer for cyclic channels in issue_pending 90395825181c dmaengine: at_hdmac: Don't start transactions at tx_submit level 93af084cf60a dmaengine: at_hdmac: Fix at_lli struct definition 79029dc341c7 (vkoul-dma/fixes) dmaengine: stm32-dma: fix potential race between pause and resume
On 04-11-22, 15:21, Tudor.Ambarus@microchip.com wrote: > > >> > >> This does not apply on dmaengine-fixes, can you please rebase and resend > >> > > > > Sure, I can, but on my side it applies just fine, both on the fixes and next > > branches. I used git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git > > Shall I use other tree? this is right tree, I was on fixes branch (for initial fixes) and i think 7th or 8th one failed to apply for me... > > $ git log --oneline > 79029dc341c7 (HEAD, vkoul-dma/fixes) dmaengine: stm32-dma: fix potential race between pause and resume > > [cut] > > $ b4 am 20221025090306.297886-1-tudor.ambarus@microchip.com > Looking up https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus%40microchip.com > Grabbing thread from lore.kernel.org/all/20221025090306.297886-1-tudor.ambarus%40microchip.com/t.mbox.gz > Analyzing 37 messages in the thread > Checking attestation on all messages, may take a moment... > --- > [PATCH v2 1/32] dmaengine: at_hdmac: Fix at_lli struct definition > [PATCH v2 2/32] dmaengine: at_hdmac: Don't start transactions at tx_submit level > [PATCH v2 3/32] dmaengine: at_hdmac: Start transfer for cyclic channels in issue_pending > [PATCH v2 4/32] dmaengine: at_hdmac: Fix premature completion of desc in issue_pending > [PATCH v2 5/32] dmaengine: at_hdmac: Do not call the complete callback on device_terminate_all > [PATCH v2 6/32] dmaengine: at_hdmac: Protect atchan->status with the channel lock > [PATCH v2 7/32] dmaengine: at_hdmac: Fix concurrency problems by removing atc_complete_all() > [PATCH v2 8/32] dmaengine: at_hdmac: Fix concurrency over descriptor > [PATCH v2 9/32] dmaengine: at_hdmac: Free the memset buf without holding the chan lock > [PATCH v2 10/32] dmaengine: at_hdmac: Fix concurrency over the active list > [PATCH v2 11/32] dmaengine: at_hdmac: Fix descriptor handling when issuing it to hardware > [PATCH v2 12/32] dmaengine: at_hdmac: Fix completion of unissued descriptor in case of errors > [PATCH v2 13/32] dmaengine: at_hdmac: Don't allow CPU to reorder channel enable > [PATCH v2 14/32] dmaengine: at_hdmac: Fix impossible condition > [PATCH v2 15/32] dmaengine: at_hdmac: Check return code of dma_async_device_register > [PATCH v2 16/32] dmaengine: at_hdmac: Do not print messages on console while holding the lock > [PATCH v2 17/32] dmaengine: at_hdmac: Return dma_cookie_status()'s ret code when txstate is NULL > [PATCH v2 18/32] dmaengine: at_hdmac: Remove superfluous cast > [PATCH v2 19/32] dmaengine: at_hdmac: Pass residue by address to avoid unnecessary implicit casts > [PATCH v2 20/32] dmaengine: at_hdmac: s/atc_get_bytes_left/atc_get_residue > [PATCH v2 21/32] dmaengine: at_hdmac: Introduce atc_get_llis_residue() > [PATCH v2 22/32] dmaengine: at_hdmac: Use devm_kzalloc() and struct_size() > [PATCH v2 23/32] dmaengine: at_hdmac: Use devm_platform_ioremap_resource > [PATCH v2 24/32] dmaengine: at_hdmac: Use devm_clk_get() > [PATCH v2 25/32] dmaengine: at_hdmac: Use pm_ptr() > [PATCH v2 26/32] dmaengine: at_hdmac: Set include entries in alphabetic order > [PATCH v2 27/32] dmaengine: at_hdmac: Keep register definitions and structures private to at_hdmac.c > [PATCH v2 28/32] dmaengine: at_hdmac: Use bitfield access macros > [PATCH v2 29/32] dmaengine: at_hdmac: Rename "dma_common" to "dma_device" > [PATCH v2 30/32] dmaengine: at_hdmac: Rename "chan_common" to "dma_chan" > [PATCH v2 31/32] dmaengine: at_hdmac: Remove unused member of at_dma_chan > [PATCH v2 32/32] dmaengine: at_hdmac: Convert driver to use virt-dma > + Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com> > --- > Total patches: 32 > --- > NOTE: Some trailers were sent to the cover letter: > Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com> > NOTE: Rerun with -t to apply them to all patches > --- > Cover: ./v2_20221025_tudor_ambarus_dmaengine_at_hdmac_fix_concurrency_bugs_and_then_convert_to_virt_dma.cover > Link: https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus@microchip.com > Base: not specified > git am ./v2_20221025_tudor_ambarus_dmaengine_at_hdmac_fix_concurrency_bugs_and_then_convert_to_virt_dma.mbx > > $ git am ./v2_20221025_tudor_ambarus_dmaengine_at_hdmac_fix_concurrency_bugs_and_then_convert_to_virt_dma.mbx > Applying: dmaengine: at_hdmac: Fix at_lli struct definition > Applying: dmaengine: at_hdmac: Don't start transactions at tx_submit level > Applying: dmaengine: at_hdmac: Start transfer for cyclic channels in issue_pending > Applying: dmaengine: at_hdmac: Fix premature completion of desc in issue_pending > Applying: dmaengine: at_hdmac: Do not call the complete callback on device_terminate_all > Applying: dmaengine: at_hdmac: Protect atchan->status with the channel lock > Applying: dmaengine: at_hdmac: Fix concurrency problems by removing atc_complete_all() > Applying: dmaengine: at_hdmac: Fix concurrency over descriptor > Applying: dmaengine: at_hdmac: Free the memset buf without holding the chan lock > Applying: dmaengine: at_hdmac: Fix concurrency over the active list > Applying: dmaengine: at_hdmac: Fix descriptor handling when issuing it to hardware > Applying: dmaengine: at_hdmac: Fix completion of unissued descriptor in case of errors > Applying: dmaengine: at_hdmac: Don't allow CPU to reorder channel enable > Applying: dmaengine: at_hdmac: Fix impossible condition > Applying: dmaengine: at_hdmac: Check return code of dma_async_device_register > Applying: dmaengine: at_hdmac: Do not print messages on console while holding the lock > Applying: dmaengine: at_hdmac: Return dma_cookie_status()'s ret code when txstate is NULL > Applying: dmaengine: at_hdmac: Remove superfluous cast > Applying: dmaengine: at_hdmac: Pass residue by address to avoid unnecessary implicit casts > Applying: dmaengine: at_hdmac: s/atc_get_bytes_left/atc_get_residue > Applying: dmaengine: at_hdmac: Introduce atc_get_llis_residue() > Applying: dmaengine: at_hdmac: Use devm_kzalloc() and struct_size() > Applying: dmaengine: at_hdmac: Use devm_platform_ioremap_resource > Applying: dmaengine: at_hdmac: Use devm_clk_get() > Applying: dmaengine: at_hdmac: Use pm_ptr() > Applying: dmaengine: at_hdmac: Set include entries in alphabetic order > Applying: dmaengine: at_hdmac: Keep register definitions and structures private to at_hdmac.c > Applying: dmaengine: at_hdmac: Use bitfield access macros > Applying: dmaengine: at_hdmac: Rename "dma_common" to "dma_device" > Applying: dmaengine: at_hdmac: Rename "chan_common" to "dma_chan" > Applying: dmaengine: at_hdmac: Remove unused member of at_dma_chan > Applying: dmaengine: at_hdmac: Convert driver to use virt-dma > > $ git log --oneline > 0216a9804005 (HEAD) dmaengine: at_hdmac: Convert driver to use virt-dma > 3c38e6f0c683 dmaengine: at_hdmac: Remove unused member of at_dma_chan > 3f801af0fe24 dmaengine: at_hdmac: Rename "chan_common" to "dma_chan" > deee5deb54d7 dmaengine: at_hdmac: Rename "dma_common" to "dma_device" > 6561448a5bef dmaengine: at_hdmac: Use bitfield access macros > 744ba3189722 dmaengine: at_hdmac: Keep register definitions and structures private to at_hdmac.c > c7b1826777a1 dmaengine: at_hdmac: Set include entries in alphabetic order > fb34dfd24504 dmaengine: at_hdmac: Use pm_ptr() > 348276780e9c dmaengine: at_hdmac: Use devm_clk_get() > 800462a74e78 dmaengine: at_hdmac: Use devm_platform_ioremap_resource > d7236b9dd3b4 dmaengine: at_hdmac: Use devm_kzalloc() and struct_size() > e6eaacf98492 dmaengine: at_hdmac: Introduce atc_get_llis_residue() > 010cabae81ae dmaengine: at_hdmac: s/atc_get_bytes_left/atc_get_residue > 9d09a47eb73a dmaengine: at_hdmac: Pass residue by address to avoid unnecessary implicit casts > 2dd9e9cbb086 dmaengine: at_hdmac: Remove superfluous cast > 5ef926e13fb8 dmaengine: at_hdmac: Return dma_cookie_status()'s ret code when txstate is NULL > 3b09d91a06ef dmaengine: at_hdmac: Do not print messages on console while holding the lock > e521dfad36a3 dmaengine: at_hdmac: Check return code of dma_async_device_register > 5edd5e1c0a8d dmaengine: at_hdmac: Fix impossible condition > 5621e6e7be9b dmaengine: at_hdmac: Don't allow CPU to reorder channel enable > 263a0ea4bc91 dmaengine: at_hdmac: Fix completion of unissued descriptor in case of errors > afb3cd55a2c1 dmaengine: at_hdmac: Fix descriptor handling when issuing it to hardware > cc94d7bdae69 dmaengine: at_hdmac: Fix concurrency over the active list > e654eceb0331 dmaengine: at_hdmac: Free the memset buf without holding the chan lock > 2536923b7374 dmaengine: at_hdmac: Fix concurrency over descriptor > c928306dea15 dmaengine: at_hdmac: Fix concurrency problems by removing atc_complete_all() > 55f6c9b21ad1 dmaengine: at_hdmac: Protect atchan->status with the channel lock > ecb13670616f dmaengine: at_hdmac: Do not call the complete callback on device_terminate_all > e0926ead50f9 dmaengine: at_hdmac: Fix premature completion of desc in issue_pending > 8354c91e9484 dmaengine: at_hdmac: Start transfer for cyclic channels in issue_pending > 90395825181c dmaengine: at_hdmac: Don't start transactions at tx_submit level > 93af084cf60a dmaengine: at_hdmac: Fix at_lli struct definition > 79029dc341c7 (vkoul-dma/fixes) dmaengine: stm32-dma: fix potential race between pause and resume
On 04-11-22, 23:29, Vinod Koul wrote: > On 04-11-22, 15:21, Tudor.Ambarus@microchip.com wrote: > > > > >> > > >> This does not apply on dmaengine-fixes, can you please rebase and resend > > >> > > > > > > Sure, I can, but on my side it applies just fine, both on the fixes and next > > > branches. I used git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git > > > Shall I use other tree? > > this is right tree, I was on fixes branch (for initial fixes) and i > think 7th or 8th one failed to apply for me... > > > > > $ git log --oneline > > 79029dc341c7 (HEAD, vkoul-dma/fixes) dmaengine: stm32-dma: fix potential race between pause and resume > > > > [cut] > > > > $ b4 am 20221025090306.297886-1-tudor.ambarus@microchip.com > > Looking up https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus%40microchip.com > > Grabbing thread from lore.kernel.org/all/20221025090306.297886-1-tudor.ambarus%40microchip.com/t.mbox.gz > > Analyzing 37 messages in the thread > > Checking attestation on all messages, may take a moment... Okay grabbing the b4 version did the trick, no sure why my local one failed. I have applied these now. Thanks
On 11/7/22 06:33, Vinod Koul wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 04-11-22, 23:29, Vinod Koul wrote: >> On 04-11-22, 15:21, Tudor.Ambarus@microchip.com wrote: >>> >>>>> >>>>> This does not apply on dmaengine-fixes, can you please rebase and resend >>>>> >>>> >>>> Sure, I can, but on my side it applies just fine, both on the fixes and next >>>> branches. I used git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/dmaengine.git >>>> Shall I use other tree? >> >> this is right tree, I was on fixes branch (for initial fixes) and i >> think 7th or 8th one failed to apply for me... >> >>> >>> $ git log --oneline >>> 79029dc341c7 (HEAD, vkoul-dma/fixes) dmaengine: stm32-dma: fix potential race between pause and resume >>> >>> [cut] >>> >>> $ b4 am 20221025090306.297886-1-tudor.ambarus@microchip.com >>> Looking up https://lore.kernel.org/r/20221025090306.297886-1-tudor.ambarus%40microchip.com >>> Grabbing thread from lore.kernel.org/all/20221025090306.297886-1-tudor.ambarus%40microchip.com/t.mbox.gz >>> Analyzing 37 messages in the thread >>> Checking attestation on all messages, may take a moment... > > Okay grabbing the b4 version did the trick, no sure why my local one > failed. I have applied these now. Thanks Cool, thank you!
v2: - reorder patches so that fixes come first -> easier to backport to stable kernels. - drop the devm_request_irq() patch as we had to disable the irq anyway in remove() in order to avoid spurios IRQs. Using devm variant brings no palpable benefit. - reword pm_ptr commit message at_hdmac driver had poor list handling and concurrency bugs. We experienced calling of the completion call twice for the same descriptor. Peter Rosin encountered the same while reporting a different bug: https://lore.kernel.org/lkml/13c6c9a2-6db5-c3bf-349b-4c127ad3496a@axentia.se/ Two sets of tests were performed: 1/ tested just the fixes, to make sure everything is fine and the concurrency bugs are squashed even without the conversion to virt-dma. All went fine. 2/ tested the entire series including the conversion the virt-dma All went fine. I tested NAND (prep_dma_memcpy), MMC (prep_dma_slave_sg), usart (cyclic mode), dmatest (memcpy, memset). With the conversion to virt-dma I replaced the election of a new transfer in the tasklet with the election of the new transfer in the interrupt handler. We should have a shorter idle window as we remove the scheduling latency of the tasklet. Using mtd_speedtest showed similar performances when using NAND with DMA. That could be because of using a low timming mode on NAND. Tudor Ambarus (32): dmaengine: at_hdmac: Fix at_lli struct definition dmaengine: at_hdmac: Don't start transactions at tx_submit level dmaengine: at_hdmac: Start transfer for cyclic channels in issue_pending dmaengine: at_hdmac: Fix premature completion of desc in issue_pending dmaengine: at_hdmac: Do not call the complete callback on device_terminate_all dmaengine: at_hdmac: Protect atchan->status with the channel lock dmaengine: at_hdmac: Fix concurrency problems by removing atc_complete_all() dmaengine: at_hdmac: Fix concurrency over descriptor dmaengine: at_hdmac: Free the memset buf without holding the chan lock dmaengine: at_hdmac: Fix concurrency over the active list dmaengine: at_hdmac: Fix descriptor handling when issuing it to hardware dmaengine: at_hdmac: Fix completion of unissued descriptor in case of errors dmaengine: at_hdmac: Don't allow CPU to reorder channel enable dmaengine: at_hdmac: Fix impossible condition dmaengine: at_hdmac: Check return code of dma_async_device_register dmaengine: at_hdmac: Do not print messages on console while holding the lock dmaengine: at_hdmac: Return dma_cookie_status()'s ret code when txstate is NULL dmaengine: at_hdmac: Remove superfluous cast dmaengine: at_hdmac: Pass residue by address to avoid unnecessary implicit casts dmaengine: at_hdmac: s/atc_get_bytes_left/atc_get_residue dmaengine: at_hdmac: Introduce atc_get_llis_residue() dmaengine: at_hdmac: Use devm_kzalloc() and struct_size() dmaengine: at_hdmac: Use devm_platform_ioremap_resource dmaengine: at_hdmac: Use devm_clk_get() dmaengine: at_hdmac: Use pm_ptr() dmaengine: at_hdmac: Set include entries in alphabetic order dmaengine: at_hdmac: Keep register definitions and structures private to at_hdmac.c dmaengine: at_hdmac: Use bitfield access macros dmaengine: at_hdmac: Rename "dma_common" to "dma_device" dmaengine: at_hdmac: Rename "chan_common" to "dma_chan" dmaengine: at_hdmac: Remove unused member of at_dma_chan dmaengine: at_hdmac: Convert driver to use virt-dma MAINTAINERS | 1 - drivers/dma/Kconfig | 1 + drivers/dma/at_hdmac.c | 1899 ++++++++++++++++++----------------- drivers/dma/at_hdmac_regs.h | 478 --------- 4 files changed, 990 insertions(+), 1389 deletions(-) delete mode 100644 drivers/dma/at_hdmac_regs.h