2022-08-31 15:44:46

by Dmitry Osipenko

Subject: [PATCH v4 16/21] dma-buf: Move dma_buf_attach() to dynamic locking specification

Move dma-buf attachment API functions to the dynamic locking specification
by taking the reservation lock around the mapping operations. The strict
locking convention prevents deadlock situations for dma-buf importers and
exporters.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/dma-buf/dma-buf.c | 20 ++++++++------------
1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index ceea4839c641..073942bf5ae9 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -858,8 +858,8 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
dma_buf_is_dynamic(dmabuf)) {
struct sg_table *sgt;

+ dma_resv_lock(attach->dmabuf->resv, NULL);
if (dma_buf_is_dynamic(attach->dmabuf)) {
- dma_resv_lock(attach->dmabuf->resv, NULL);
ret = dmabuf->ops->pin(attach);
if (ret)
goto err_unlock;
@@ -872,8 +872,7 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
ret = PTR_ERR(sgt);
goto err_unpin;
}
- if (dma_buf_is_dynamic(attach->dmabuf))
- dma_resv_unlock(attach->dmabuf->resv);
+ dma_resv_unlock(attach->dmabuf->resv);
attach->sgt = sgt;
attach->dir = DMA_BIDIRECTIONAL;
}
@@ -889,8 +888,7 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
dmabuf->ops->unpin(attach);

err_unlock:
- if (dma_buf_is_dynamic(attach->dmabuf))
- dma_resv_unlock(attach->dmabuf->resv);
+ dma_resv_unlock(attach->dmabuf->resv);

dma_buf_detach(dmabuf, attach);
return ERR_PTR(ret);
@@ -936,21 +934,19 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach)
if (WARN_ON(!dmabuf || !attach))
return;

+ dma_resv_lock(attach->dmabuf->resv, NULL);
+
if (attach->sgt) {
- if (dma_buf_is_dynamic(attach->dmabuf))
- dma_resv_lock(attach->dmabuf->resv, NULL);

__unmap_dma_buf(attach, attach->sgt, attach->dir);

- if (dma_buf_is_dynamic(attach->dmabuf)) {
+ if (dma_buf_is_dynamic(attach->dmabuf))
dmabuf->ops->unpin(attach);
- dma_resv_unlock(attach->dmabuf->resv);
- }
}
-
- dma_resv_lock(dmabuf->resv, NULL);
list_del(&attach->node);
+
dma_resv_unlock(dmabuf->resv);
+
if (dmabuf->ops->detach)
dmabuf->ops->detach(dmabuf, attach);

--
2.37.2

2022-09-01 07:21:29

by Christian König

Subject: Re: [PATCH v4 16/21] dma-buf: Move dma_buf_attach() to dynamic locking specification

Am 31.08.22 um 17:37 schrieb Dmitry Osipenko:
> Move dma-buf attachment API functions to the dynamic locking specification
> by taking the reservation lock around the mapping operations. The strict
> locking convention prevents deadlock situations for dma-buf importers and
> exporters.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>

Reviewed-by: Christian König <[email protected]>

> ---
> drivers/dma-buf/dma-buf.c | 20 ++++++++------------
> 1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index ceea4839c641..073942bf5ae9 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -858,8 +858,8 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
> dma_buf_is_dynamic(dmabuf)) {
> struct sg_table *sgt;
>
> + dma_resv_lock(attach->dmabuf->resv, NULL);
> if (dma_buf_is_dynamic(attach->dmabuf)) {
> - dma_resv_lock(attach->dmabuf->resv, NULL);
> ret = dmabuf->ops->pin(attach);
> if (ret)
> goto err_unlock;
> @@ -872,8 +872,7 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
> ret = PTR_ERR(sgt);
> goto err_unpin;
> }
> - if (dma_buf_is_dynamic(attach->dmabuf))
> - dma_resv_unlock(attach->dmabuf->resv);
> + dma_resv_unlock(attach->dmabuf->resv);
> attach->sgt = sgt;
> attach->dir = DMA_BIDIRECTIONAL;
> }
> @@ -889,8 +888,7 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
> dmabuf->ops->unpin(attach);
>
> err_unlock:
> - if (dma_buf_is_dynamic(attach->dmabuf))
> - dma_resv_unlock(attach->dmabuf->resv);
> + dma_resv_unlock(attach->dmabuf->resv);
>
> dma_buf_detach(dmabuf, attach);
> return ERR_PTR(ret);
> @@ -936,21 +934,19 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach)
> if (WARN_ON(!dmabuf || !attach))
> return;
>
> + dma_resv_lock(attach->dmabuf->resv, NULL);
> +
> if (attach->sgt) {
> - if (dma_buf_is_dynamic(attach->dmabuf))
> - dma_resv_lock(attach->dmabuf->resv, NULL);
>
> __unmap_dma_buf(attach, attach->sgt, attach->dir);
>
> - if (dma_buf_is_dynamic(attach->dmabuf)) {
> + if (dma_buf_is_dynamic(attach->dmabuf))
> dmabuf->ops->unpin(attach);
> - dma_resv_unlock(attach->dmabuf->resv);
> - }
> }
> -
> - dma_resv_lock(dmabuf->resv, NULL);
> list_del(&attach->node);
> +
> dma_resv_unlock(dmabuf->resv);
> +
> if (dmabuf->ops->detach)
> dmabuf->ops->detach(dmabuf, attach);
>