lkml.org 
[lkml]   [2020]   [Aug]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: drivers/dma/xilinx/xilinx_dpdma.c:1172 xilinx_dpdma_synchronize() error: double unlocked 'chan->vchan.lock' (orig line 1170)
HI Laurent,

On 12-08-20, 01:50, Laurent Pinchart wrote:
> On Wed, Aug 12, 2020 at 06:37:59AM +0800, kernel test robot wrote:
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head: c636eef2ee3696f261a35f34989842701a107895
> > commit: 7cbb0c63de3fc218fd06ecfedb477772a4d12f76 dmaengine: xilinx: dpdma: Add the Xilinx DisplayPort DMA engine driver
> > date: 4 weeks ago
> > config: h8300-randconfig-m031-20200811 (attached as .config)
> > compiler: h8300-linux-gcc (GCC) 9.3.0
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> >
> > smatch warnings:
> > drivers/dma/xilinx/xilinx_dpdma.c:1172 xilinx_dpdma_synchronize() error: double unlocked 'chan->vchan.lock' (orig line 1170)
>
> Unless I'm mistaken, this is a false positive. I'd appreciate if someone
> could check if I'm missing something obvious.
>
> > vim +1172 drivers/dma/xilinx/xilinx_dpdma.c
> >
> > 1140
> > 1141 /**
> > 1142 * xilinx_dpdma_synchronize - Synchronize callback execution
> > 1143 * @dchan: DMA channel
> > 1144 *
> > 1145 * Synchronizing callback execution ensures that all previously issued
> > 1146 * transfers have completed and all associated callbacks have been called and
> > 1147 * have returned.
> > 1148 *
> > 1149 * This function waits for the DMA channel to stop. It assumes it has been
> > 1150 * paused by a previous call to dmaengine_terminate_async(), and that no new
> > 1151 * pending descriptors have been issued with dma_async_issue_pending(). The
> > 1152 * behaviour is undefined otherwise.
> > 1153 */
> > 1154 static void xilinx_dpdma_synchronize(struct dma_chan *dchan)
> > 1155 {
> > 1156 struct xilinx_dpdma_chan *chan = to_xilinx_chan(dchan);
> > 1157 unsigned long flags;
> > 1158
> > 1159 xilinx_dpdma_chan_stop(chan);
> > 1160
> > 1161 spin_lock_irqsave(&chan->vchan.lock, flags);

Lock is held here

> > 1162 if (chan->desc.pending) {
> > 1163 vchan_terminate_vdesc(&chan->desc.pending->vdesc);

This should be called with lock held

> > 1164 chan->desc.pending = NULL;
> > 1165 }
> > 1166 if (chan->desc.active) {
> > 1167 vchan_terminate_vdesc(&chan->desc.active->vdesc);

same here

> > 1168 chan->desc.active = NULL;
> > 1169 }
> > > 1170 spin_unlock_irqrestore(&chan->vchan.lock, flags);

Dropped

> > 1171
> > > 1172 vchan_synchronize(&chan->vchan);

This should be called without lock

So I agree with you, looks like a false positive to me as well

--
~Vinod

\
 
 \ /
  Last update: 2020-08-12 06:17    [W:0.050 / U:6.340 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site