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 <[email protected]>
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/dma-buf/dma-buf.c | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
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);
--
2.40.1
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 <[email protected]>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> 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!
--
Best regards,
Dmitry
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 <[email protected]>
>> Signed-off-by: Dmitry Osipenko <[email protected]>
>> ---
>> 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.
--
Best regards,
Dmitry
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 <[email protected]>
>>> Signed-off-by: Dmitry Osipenko <[email protected]>
>>> ---
>>> 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.
Regards,
Christian.
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 <[email protected]>
>>>> Signed-off-by: Dmitry Osipenko <[email protected]>
>>>> ---
>>>> 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
--
Best regards,
Dmitry