Re: [PATCH v3] Add udmabuf misc device

From: Gerd Hoffmann
Date: Tue May 29 2018 - 04:48:59 EST


Hi,

> > +static void *kmap_atomic_udmabuf(struct dma_buf *buf, unsigned long page_num)
> > +{
> > + struct udmabuf *ubuf = buf->priv;
> > + struct page *page = ubuf->pages[page_num];
> > +
> > + return kmap_atomic(page);
> > +}
> > +
> > +static void *kmap_udmabuf(struct dma_buf *buf, unsigned long page_num)
> > +{
> > + struct udmabuf *ubuf = buf->priv;
> > + struct page *page = ubuf->pages[page_num];
> > +
> > + return kmap(page);
> > +}
>
> The above leaks like mad since no kunamp?

/me checks code. Oops. Yes.

The docs say map() is required and unmap() is not (for both atomic and
non-atomic cases), so I assumed there is a default implementation just
doing kunmap(page). Which is not the case. /me looks a bit surprised.

I'll fix it for v4.

> Also I think we have 0 users of the kmap atomic interfaces ... so not sure
> whether it's worth it to implement those.

Well, the docs are correct. kmap_atomic() is required, dma-buf.c calls
the function pointer without checking it exists beforehand ...

cheers,
Gerd