Re: [PATCH v2 3/6] dmaengine: sun6i: serialize channel reassignment with interrupt handling

[James Hilliard]

On Fri, Oct 23, 2026 at 2:33 PM Frank Li <Frank.li@xxxxxxxxxxx> wrote:
>
> On Fri, Oct 23, 2026 at 12:42:37PM -0600, James Hilliard wrote:
> > The allocator changes vchan->phy and pchan->vchan under sdev->lock, but
> > termination reads the physical channel before taking either lock. The IRQ
> > handler also takes an owner snapshot without protecting it. Termination
> > can therefore miss a new assignment or detach an owner while an interrupt
> > is about to complete its descriptor. An IRQ status snapshot can survive
> > reassignment and be applied to the next transfer.
> >
> > Use sdev->lock for physical-channel ownership, pending channels and the
> > shared interrupt registers, nesting vc.lock inside it for descriptor
> > lists and cookie updates. Allocate and start each pending channel in one
> > critical section. Read and acknowledge IRQ status under the same outer
> > lock, and protect cyclic notifications as well as ordinary completions.
> >
> > Clear the active descriptor pointer before publishing finite completion
> > to virt-dma, whose tasklet may free it independently of the allocator.
> > An assigned channel without an active descriptor is ready for the
> > allocator to start its next transfer or release it. This replaces the
> > completed-descriptor pointer and keeps residue queries and termination
> > from consulting a descriptor already handed to virt-dma.
> >
> > Mask and clear a stopped channel's IRQs before releasing its ownership,
> > and clear pending status before starting a new descriptor. Schedule the
> > allocator after termination so other pending channels can use the released
> > physical channel without depending on another device interrupt.
> >
> > This also keeps process-context acquisitions IRQ-safe. Client callbacks
> > remain outside these locks in the virtual-channel completion tasklet.
> >
> > Fixes: 555859308723 ("dmaengine: sun6i: Add driver for the Allwinner A31 DMA controller")
> > Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
> > Closes: https://lore.kernel.org/r/20260706065650.34B131F00A3A@xxxxxxxxxxxxxxx
> > Signed-off-by: James Hilliard <james.hilliard1@xxxxxxxxx>
> > ---
>
> Is it possible split to more small change? And you almost add sdev->lock,
> any place only use vc.lock without sdev->lock?

Yes. virt-dma takes vc.lock alone in vchan_tx_prep(), vchan_tx_submit()
and vchan_complete() to protect its descriptor lists and cookies.

sdev->lock protects physical-channel ownership and shared registers.
The sun6i paths need both locks when updating that state together with
the virt-dma lists and cookies.

>
> Frank