On Thu, Jan 21, 2021 at 11:56 PM Sumit Semwal <[email protected]> wrote: > > Hi John, Suren, > > > On Wed, 20 Jan 2021 at 02:15, John Stultz <[email protected]> wrote: > > > > We shouldn't vunmap more then we vmap, but if we do, make > > sure we complain loudly. > > I was checking the general usage of vunmap in the kernel, and I > couldn't find many instances where we need to WARN_ON for the vunmap > count more than vmap count. Is there a specific need for this in the heaps?
Hi Sumit, My worry was that buffer->vmap_cnt could silently go negative. But if this warning is not consistent with other places we do refcounted vmap/vunmap then feel free to ignore my suggestion. Thanks! > > Best, > Sumit. > > > > Cc: Sumit Semwal <[email protected]> > > Cc: Liam Mark <[email protected]> > > Cc: Laura Abbott <[email protected]> > > Cc: Brian Starkey <[email protected]> > > Cc: Hridya Valsaraju <[email protected]> > > Cc: Suren Baghdasaryan <[email protected]> > > Cc: Sandeep Patil <[email protected]> > > Cc: Daniel Mentz <[email protected]> > > Cc: Chris Goldsworthy <[email protected]> > > Cc: Ørjan Eide <[email protected]> > > Cc: Robin Murphy <[email protected]> > > Cc: Ezequiel Garcia <[email protected]> > > Cc: Simon Ser <[email protected]> > > Cc: James Jones <[email protected]> > > Cc: [email protected] > > Cc: [email protected] > > Suggested-by: Suren Baghdasaryan <[email protected]> > > Signed-off-by: John Stultz <[email protected]> > > --- > > drivers/dma-buf/heaps/cma_heap.c | 1 + > > drivers/dma-buf/heaps/system_heap.c | 1 + > > 2 files changed, 2 insertions(+) > > > > diff --git a/drivers/dma-buf/heaps/cma_heap.c > > b/drivers/dma-buf/heaps/cma_heap.c > > index 364fc2f3e499..0c76cbc3fb11 100644 > > --- a/drivers/dma-buf/heaps/cma_heap.c > > +++ b/drivers/dma-buf/heaps/cma_heap.c > > @@ -232,6 +232,7 @@ static void cma_heap_vunmap(struct dma_buf *dmabuf, > > struct dma_buf_map *map) > > struct cma_heap_buffer *buffer = dmabuf->priv; > > > > mutex_lock(&buffer->lock); > > + WARN_ON(buffer->vmap_cnt == 0); > > if (!--buffer->vmap_cnt) { > > vunmap(buffer->vaddr); > > buffer->vaddr = NULL; > > diff --git a/drivers/dma-buf/heaps/system_heap.c > > b/drivers/dma-buf/heaps/system_heap.c > > index 405351aad2a8..2321c91891f6 100644 > > --- a/drivers/dma-buf/heaps/system_heap.c > > +++ b/drivers/dma-buf/heaps/system_heap.c > > @@ -273,6 +273,7 @@ static void system_heap_vunmap(struct dma_buf *dmabuf, > > struct dma_buf_map *map) > > struct system_heap_buffer *buffer = dmabuf->priv; > > > > mutex_lock(&buffer->lock); > > + WARN_ON(buffer->vmap_cnt == 0); > > if (!--buffer->vmap_cnt) { > > vunmap(buffer->vaddr); > > buffer->vaddr = NULL; > > -- > > 2.17.1 > >
