On Tue, May 29, 2018 at 10:48 AM, Gerd Hoffmann kraxel@redhat.com wrote:
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 ...
Frankly with the pletoria of dummy kmap functions that just return NULL; it might be better to move that into core dma-buf code and make it optional for real. Since it's indeed very surprising. -Daniel