| Message ID | 20230529223935.2672495-6-dmitry.osipenko@collabora.com |
|---|---|
| State | Accepted |
| Commit | 8021fa16b7ec0a3b0457dc15484a7691f99265bc |
| Headers | show |
| Series | Move dma-buf mmap() reservation locking down to exporters | expand |
On 6/21/23 08:42, Christian König wrote: > Am 20.06.23 um 17:58 schrieb Dmitry Osipenko: >> On 5/31/23 22:58, Dmitry Osipenko wrote: >>> On 5/30/23 01:39, Dmitry Osipenko wrote: >>>> Change locking policy of mmap() callback, making exporters responsible >>>> for handling dma-buf reservation locking. Previous locking policy >>>> stated >>>> that dma-buf is locked for both importers and exporters by the dma-buf >>>> core, which caused a deadlock problem for DRM drivers in a case of >>>> self-imported dma-bufs which required to take the lock from the DRM >>>> exporter side. >>>> >>>> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> >>>> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com> >>>> --- >>>> drivers/dma-buf/dma-buf.c | 17 +++-------------- >>>> 1 file changed, 3 insertions(+), 14 deletions(-) >>> Christian, you acked the drm patch of this series sometime ago, perhaps >>> it also implies implicit ack to this patch, but I'd prefer to have the >>> explicit ack. I'll apply this series to drm-misc later this week if >>> you'll approve this dma-buf change. Thanks in advance! >> I'll merge the patches tomorrow. If there are any additional comments, >> then please don't hesitate to post them. > > Sorry for not responding earlier, I have been moving both my office as > well as my household and still catching up. > > I don't have time for an in-deep review, but my ack stands for the whole > series. Thanks! I'll add yours ack and merge patches soon
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index aa4ea8530cb3..21916bba77d5 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -131,7 +131,6 @@ static struct file_system_type dma_buf_fs_type = { static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma) { struct dma_buf *dmabuf; - int ret; if (!is_dma_buf_file(file)) return -EINVAL; @@ -147,11 +146,7 @@ static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma) dmabuf->size >> PAGE_SHIFT) return -EINVAL; - dma_resv_lock(dmabuf->resv, NULL); - ret = dmabuf->ops->mmap(dmabuf, vma); - dma_resv_unlock(dmabuf->resv); - - return ret; + return dmabuf->ops->mmap(dmabuf, vma); } static loff_t dma_buf_llseek(struct file *file, loff_t offset, int whence) @@ -850,6 +845,7 @@ static struct sg_table * __map_dma_buf(struct dma_buf_attachment *attach, * - &dma_buf_ops.release() * - &dma_buf_ops.begin_cpu_access() * - &dma_buf_ops.end_cpu_access() + * - &dma_buf_ops.mmap() * * 2. These &dma_buf_ops callbacks are invoked with locked dma-buf * reservation and exporter can't take the lock: @@ -858,7 +854,6 @@ static struct sg_table * __map_dma_buf(struct dma_buf_attachment *attach, * - &dma_buf_ops.unpin() * - &dma_buf_ops.map_dma_buf() * - &dma_buf_ops.unmap_dma_buf() - * - &dma_buf_ops.mmap() * - &dma_buf_ops.vmap() * - &dma_buf_ops.vunmap() * @@ -1463,8 +1458,6 @@ EXPORT_SYMBOL_NS_GPL(dma_buf_end_cpu_access, DMA_BUF); int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma, unsigned long pgoff) { - int ret; - if (WARN_ON(!dmabuf || !vma)) return -EINVAL; @@ -1485,11 +1478,7 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma, vma_set_file(vma, dmabuf->file); vma->vm_pgoff = pgoff; - dma_resv_lock(dmabuf->resv, NULL); - ret = dmabuf->ops->mmap(dmabuf, vma); - dma_resv_unlock(dmabuf->resv); - - return ret; + return dmabuf->ops->mmap(dmabuf, vma); } EXPORT_SYMBOL_NS_GPL(dma_buf_mmap, DMA_BUF);