On 06/13/2018 05:04 AM, Oleksandr Andrushchenko wrote: > On 06/13/2018 06:14 AM, Boris Ostrovsky wrote: >> >> >> On 06/12/2018 09:42 AM, Oleksandr Andrushchenko wrote: >> >>> int gntdev_dmabuf_imp_release(struct gntdev_dmabuf_priv *priv, u32 >>> fd) >>> { >>> - return -EINVAL; >>> + struct gntdev_dmabuf *gntdev_dmabuf; >>> + struct dma_buf_attachment *attach; >>> + struct dma_buf *dma_buf; >>> + >>> + gntdev_dmabuf = dmabuf_imp_find_unlink(priv, fd); >>> + if (IS_ERR(gntdev_dmabuf)) >>> + return PTR_ERR(gntdev_dmabuf); >>> + >>> + pr_debug("Releasing DMA buffer with fd %d\n", fd); >>> + >>> + attach = gntdev_dmabuf->u.imp.attach; >>> + >>> + if (gntdev_dmabuf->u.imp.sgt) >>> + dma_buf_unmap_attachment(attach, gntdev_dmabuf->u.imp.sgt, >>> + DMA_BIDIRECTIONAL); >>> + dma_buf = attach->dmabuf; >>> + dma_buf_detach(attach->dmabuf, attach); >>> + dma_buf_put(dma_buf); >>> + >>> + dmabuf_imp_end_foreign_access(gntdev_dmabuf->u.imp.refs, >>> + gntdev_dmabuf->nr_pages); >> >> >> >> Should you first end foreign access, before doing anything? >> > I am rolling back in reverse order here, so I think we first need > to finish local activities with the buffer and then end foreign > access. Looking at gntdev_dmabuf_imp_to_refs(), the order is dmabuf_imp_alloc_storage() dma_buf_attach() dma_buf_map_attachment() dmabuf_imp_grant_foreign_access() Or was I looking at wrong place? -boris