| Message ID | 20240607-xdma-fixes-v2-1-0282319ce345@bootlin.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 462237d2d93fc9e9221d1cf9f773954d27da83c0 |
| Headers | show |
| Series | [v2] dmaengine: xilinx: xdma: Fix data synchronisation in xdma_channel_isr() | expand |
> Requests the vchan lock before using xdma->stop_request. Better wording alternative?: A data synchronisation construct was missing in this function implementation. Thus apply the vchan lock before checking the data structure member “xchan->stop_requested”. > --- > drivers/dma/xilinx/xdma.c | 4 ++-- How do think about to avoid a duplicate marker line? … > +++ b/drivers/dma/xilinx/xdma.c > @@ -885,11 +885,11 @@ static irqreturn_t xdma_channel_isr(int irq, void *dev_id) > u32 st; > bool repeat_tx; > > + spin_lock(&xchan->vchan.lock); > + > if (xchan->stop_requested) > complete(&xchan->last_interrupt); > > - spin_lock(&xchan->vchan.lock); > - > /* get submitted request */ … Under which circumstances will development interests grow for the usage of a statement like “guard(raw_spinlock)(&xchan->vchan.lock);”? https://elixir.bootlin.com/linux/v6.10-rc2/source/include/linux/cleanup.h#L124 Regards, Markus
… > +++ b/drivers/dma/xilinx/xdma.c > @@ -885,11 +885,11 @@ static irqreturn_t xdma_channel_isr(int irq, void *dev_id) > u32 st; > bool repeat_tx; > > + spin_lock(&xchan->vchan.lock); > + > if (xchan->stop_requested) > complete(&xchan->last_interrupt); … Can any lock scope adjustments matter also for another variable definition? https://elixir.bootlin.com/linux/v6.10-rc2/source/drivers/dma/xilinx/xdma.c#L873 struct xdma_device *xdev = xchan->xdev_hdl; Regards, Markus
On Fri, 07 Jun 2024 10:34:38 +0200, Louis Chauvet wrote: > Requests the vchan lock before using xdma->stop_request. > > Applied, thanks! [1/1] dmaengine: xilinx: xdma: Fix data synchronisation in xdma_channel_isr() commit: 462237d2d93fc9e9221d1cf9f773954d27da83c0 Best regards,
diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c index e143a7330816..718842fdaf98 100644 --- a/drivers/dma/xilinx/xdma.c +++ b/drivers/dma/xilinx/xdma.c @@ -885,11 +885,11 @@ static irqreturn_t xdma_channel_isr(int irq, void *dev_id) u32 st; bool repeat_tx; + spin_lock(&xchan->vchan.lock); + if (xchan->stop_requested) complete(&xchan->last_interrupt); - spin_lock(&xchan->vchan.lock); - /* get submitted request */ vd = vchan_next_desc(&xchan->vchan); if (!vd)
Requests the vchan lock before using xdma->stop_request. Fixes: 6a40fb824596 ("dmaengine: xilinx: xdma: Fix synchronization issue") Cc: stable@vger.kernel.org Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com> --- Changes in v2: - Apply comments from Markus - Link to v1: https://lore.kernel.org/r/20240527-xdma-fixes-v1-1-f31434b56842@bootlin.com --- drivers/dma/xilinx/xdma.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) --- base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0 change-id: 20240527-xdma-fixes-74bbe2dcbeb8 Best regards,