* [PATCH] dma-buf: Fix confusion of dynamic dma-buf vs dynamic attachment
@ 2021-03-05 10:51 ` Chris Wilson
0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2021-03-05 10:51 UTC (permalink / raw)
To: dri-devel; +Cc: Chris Wilson, Daniel Vetter, Christian König, stable
Commit c545781e1c55 ("dma-buf: doc polish for pin/unpin") disagrees with
the introduction of dynamism in commit: bb42df4662a4 ("dma-buf: add
dynamic DMA-buf handling v15") resulting in warning spew on
importing dma-buf. Silence the warning from the latter by only pinning
the attachment if the attachment rather than the dmabuf is to be
dynamic.
Fixes: bb42df4662a4 ("dma-buf: add dynamic DMA-buf handling v15")
Fixes: c545781e1c55 ("dma-buf: doc polish for pin/unpin")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Christian König <christian.koenig@amd.com>
Cc: <stable@vger.kernel.org> # v5.7+
---
drivers/dma-buf/dma-buf.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index f264b70c383e..09f5ae458515 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -758,8 +758,8 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
dma_buf_is_dynamic(dmabuf)) {
struct sg_table *sgt;
- if (dma_buf_is_dynamic(attach->dmabuf)) {
- dma_resv_lock(attach->dmabuf->resv, NULL);
+ if (dma_buf_attachment_is_dynamic(attach)) {
+ dma_resv_lock(dmabuf->resv, NULL);
ret = dma_buf_pin(attach);
if (ret)
goto err_unlock;
@@ -772,8 +772,9 @@ 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);
+ if (dma_buf_attachment_is_dynamic(attach))
+ dma_resv_unlock(dmabuf->resv);
+
attach->sgt = sgt;
attach->dir = DMA_BIDIRECTIONAL;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] dma-buf: Fix confusion of dynamic dma-buf vs dynamic attachment
@ 2021-03-05 10:51 ` Chris Wilson
0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2021-03-05 10:51 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter, Christian König, stable, Chris Wilson
Commit c545781e1c55 ("dma-buf: doc polish for pin/unpin") disagrees with
the introduction of dynamism in commit: bb42df4662a4 ("dma-buf: add
dynamic DMA-buf handling v15") resulting in warning spew on
importing dma-buf. Silence the warning from the latter by only pinning
the attachment if the attachment rather than the dmabuf is to be
dynamic.
Fixes: bb42df4662a4 ("dma-buf: add dynamic DMA-buf handling v15")
Fixes: c545781e1c55 ("dma-buf: doc polish for pin/unpin")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Christian König <christian.koenig@amd.com>
Cc: <stable@vger.kernel.org> # v5.7+
---
drivers/dma-buf/dma-buf.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index f264b70c383e..09f5ae458515 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -758,8 +758,8 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
dma_buf_is_dynamic(dmabuf)) {
struct sg_table *sgt;
- if (dma_buf_is_dynamic(attach->dmabuf)) {
- dma_resv_lock(attach->dmabuf->resv, NULL);
+ if (dma_buf_attachment_is_dynamic(attach)) {
+ dma_resv_lock(dmabuf->resv, NULL);
ret = dma_buf_pin(attach);
if (ret)
goto err_unlock;
@@ -772,8 +772,9 @@ 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);
+ if (dma_buf_attachment_is_dynamic(attach))
+ dma_resv_unlock(dmabuf->resv);
+
attach->sgt = sgt;
attach->dir = DMA_BIDIRECTIONAL;
}
--
2.20.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] dma-buf: Fix confusion of dynamic dma-buf vs dynamic attachment
2021-03-05 10:51 ` Chris Wilson
@ 2021-03-05 10:54 ` Christian König
-1 siblings, 0 replies; 6+ messages in thread
From: Christian König @ 2021-03-05 10:54 UTC (permalink / raw)
To: Chris Wilson, dri-devel; +Cc: Daniel Vetter, stable
Am 05.03.21 um 11:51 schrieb Chris Wilson:
> Commit c545781e1c55 ("dma-buf: doc polish for pin/unpin") disagrees with
> the introduction of dynamism in commit: bb42df4662a4 ("dma-buf: add
> dynamic DMA-buf handling v15") resulting in warning spew on
> importing dma-buf. Silence the warning from the latter by only pinning
> the attachment if the attachment rather than the dmabuf is to be
> dynamic.
NAK, this is intentionally like this. You need to pin the DMA-buf if it
is dynamic and the attachment isn't.
Otherwise the DMA-buf would be able to move even when it has an
attachment which can't handle that.
We should rather fix the documentation if that is wrong on this point.
Regards,
Christian.
>
> Fixes: bb42df4662a4 ("dma-buf: add dynamic DMA-buf handling v15")
> Fixes: c545781e1c55 ("dma-buf: doc polish for pin/unpin")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: <stable@vger.kernel.org> # v5.7+
> ---
> drivers/dma-buf/dma-buf.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index f264b70c383e..09f5ae458515 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -758,8 +758,8 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
> dma_buf_is_dynamic(dmabuf)) {
> struct sg_table *sgt;
>
> - if (dma_buf_is_dynamic(attach->dmabuf)) {
> - dma_resv_lock(attach->dmabuf->resv, NULL);
> + if (dma_buf_attachment_is_dynamic(attach)) {
> + dma_resv_lock(dmabuf->resv, NULL);
> ret = dma_buf_pin(attach);
> if (ret)
> goto err_unlock;
> @@ -772,8 +772,9 @@ 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);
> + if (dma_buf_attachment_is_dynamic(attach))
> + dma_resv_unlock(dmabuf->resv);
> +
> attach->sgt = sgt;
> attach->dir = DMA_BIDIRECTIONAL;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] dma-buf: Fix confusion of dynamic dma-buf vs dynamic attachment
@ 2021-03-05 10:54 ` Christian König
0 siblings, 0 replies; 6+ messages in thread
From: Christian König @ 2021-03-05 10:54 UTC (permalink / raw)
To: Chris Wilson, dri-devel; +Cc: Daniel Vetter, stable
Am 05.03.21 um 11:51 schrieb Chris Wilson:
> Commit c545781e1c55 ("dma-buf: doc polish for pin/unpin") disagrees with
> the introduction of dynamism in commit: bb42df4662a4 ("dma-buf: add
> dynamic DMA-buf handling v15") resulting in warning spew on
> importing dma-buf. Silence the warning from the latter by only pinning
> the attachment if the attachment rather than the dmabuf is to be
> dynamic.
NAK, this is intentionally like this. You need to pin the DMA-buf if it
is dynamic and the attachment isn't.
Otherwise the DMA-buf would be able to move even when it has an
attachment which can't handle that.
We should rather fix the documentation if that is wrong on this point.
Regards,
Christian.
>
> Fixes: bb42df4662a4 ("dma-buf: add dynamic DMA-buf handling v15")
> Fixes: c545781e1c55 ("dma-buf: doc polish for pin/unpin")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: <stable@vger.kernel.org> # v5.7+
> ---
> drivers/dma-buf/dma-buf.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index f264b70c383e..09f5ae458515 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -758,8 +758,8 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
> dma_buf_is_dynamic(dmabuf)) {
> struct sg_table *sgt;
>
> - if (dma_buf_is_dynamic(attach->dmabuf)) {
> - dma_resv_lock(attach->dmabuf->resv, NULL);
> + if (dma_buf_attachment_is_dynamic(attach)) {
> + dma_resv_lock(dmabuf->resv, NULL);
> ret = dma_buf_pin(attach);
> if (ret)
> goto err_unlock;
> @@ -772,8 +772,9 @@ 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);
> + if (dma_buf_attachment_is_dynamic(attach))
> + dma_resv_unlock(dmabuf->resv);
> +
> attach->sgt = sgt;
> attach->dir = DMA_BIDIRECTIONAL;
> }
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] dma-buf: Fix confusion of dynamic dma-buf vs dynamic attachment
2021-03-05 10:54 ` Christian König
@ 2021-03-11 14:12 ` Daniel Vetter
-1 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2021-03-11 14:12 UTC (permalink / raw)
To: Christian König; +Cc: Chris Wilson, dri-devel, Daniel Vetter, stable
On Fri, Mar 05, 2021 at 11:54:49AM +0100, Christian König wrote:
> Am 05.03.21 um 11:51 schrieb Chris Wilson:
> > Commit c545781e1c55 ("dma-buf: doc polish for pin/unpin") disagrees with
> > the introduction of dynamism in commit: bb42df4662a4 ("dma-buf: add
> > dynamic DMA-buf handling v15") resulting in warning spew on
> > importing dma-buf. Silence the warning from the latter by only pinning
> > the attachment if the attachment rather than the dmabuf is to be
> > dynamic.
>
> NAK, this is intentionally like this. You need to pin the DMA-buf if it is
> dynamic and the attachment isn't.
>
> Otherwise the DMA-buf would be able to move even when it has an attachment
> which can't handle that.
>
> We should rather fix the documentation if that is wrong on this point.
The doc is right, it's for the exporter function for importers. For
non-dynamic importers dma-buf.c code itself does ensure the pinning
happens. So non-dynamic importers really have no business calling
pin/unpin, because they always get a mapping that's put into system memory
and pinned there.
Ofc for driver specific stuff with direct interfaces you can do whatever
you feel like, but probably good to match these semantics.
But looking at the patch, I think this is more about the locking, not the
pin/unpin stuff. Locking rules definitely depend upon what the exporter
requires, and again dma-buf.c should do all the impendence mismatch that's
needed.
So I think we're all good with the doc, but please double-check.
-Daniel
>
> Regards,
> Christian.
>
> >
> > Fixes: bb42df4662a4 ("dma-buf: add dynamic DMA-buf handling v15")
> > Fixes: c545781e1c55 ("dma-buf: doc polish for pin/unpin")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Christian König <christian.koenig@amd.com>
> > Cc: <stable@vger.kernel.org> # v5.7+
> > ---
> > drivers/dma-buf/dma-buf.c | 9 +++++----
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index f264b70c383e..09f5ae458515 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -758,8 +758,8 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
> > dma_buf_is_dynamic(dmabuf)) {
> > struct sg_table *sgt;
> > - if (dma_buf_is_dynamic(attach->dmabuf)) {
> > - dma_resv_lock(attach->dmabuf->resv, NULL);
> > + if (dma_buf_attachment_is_dynamic(attach)) {
> > + dma_resv_lock(dmabuf->resv, NULL);
> > ret = dma_buf_pin(attach);
> > if (ret)
> > goto err_unlock;
> > @@ -772,8 +772,9 @@ 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);
> > + if (dma_buf_attachment_is_dynamic(attach))
> > + dma_resv_unlock(dmabuf->resv);
> > +
> > attach->sgt = sgt;
> > attach->dir = DMA_BIDIRECTIONAL;
> > }
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] dma-buf: Fix confusion of dynamic dma-buf vs dynamic attachment
@ 2021-03-11 14:12 ` Daniel Vetter
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2021-03-11 14:12 UTC (permalink / raw)
To: Christian König; +Cc: Daniel Vetter, stable, dri-devel, Chris Wilson
On Fri, Mar 05, 2021 at 11:54:49AM +0100, Christian König wrote:
> Am 05.03.21 um 11:51 schrieb Chris Wilson:
> > Commit c545781e1c55 ("dma-buf: doc polish for pin/unpin") disagrees with
> > the introduction of dynamism in commit: bb42df4662a4 ("dma-buf: add
> > dynamic DMA-buf handling v15") resulting in warning spew on
> > importing dma-buf. Silence the warning from the latter by only pinning
> > the attachment if the attachment rather than the dmabuf is to be
> > dynamic.
>
> NAK, this is intentionally like this. You need to pin the DMA-buf if it is
> dynamic and the attachment isn't.
>
> Otherwise the DMA-buf would be able to move even when it has an attachment
> which can't handle that.
>
> We should rather fix the documentation if that is wrong on this point.
The doc is right, it's for the exporter function for importers. For
non-dynamic importers dma-buf.c code itself does ensure the pinning
happens. So non-dynamic importers really have no business calling
pin/unpin, because they always get a mapping that's put into system memory
and pinned there.
Ofc for driver specific stuff with direct interfaces you can do whatever
you feel like, but probably good to match these semantics.
But looking at the patch, I think this is more about the locking, not the
pin/unpin stuff. Locking rules definitely depend upon what the exporter
requires, and again dma-buf.c should do all the impendence mismatch that's
needed.
So I think we're all good with the doc, but please double-check.
-Daniel
>
> Regards,
> Christian.
>
> >
> > Fixes: bb42df4662a4 ("dma-buf: add dynamic DMA-buf handling v15")
> > Fixes: c545781e1c55 ("dma-buf: doc polish for pin/unpin")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Christian König <christian.koenig@amd.com>
> > Cc: <stable@vger.kernel.org> # v5.7+
> > ---
> > drivers/dma-buf/dma-buf.c | 9 +++++----
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index f264b70c383e..09f5ae458515 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -758,8 +758,8 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
> > dma_buf_is_dynamic(dmabuf)) {
> > struct sg_table *sgt;
> > - if (dma_buf_is_dynamic(attach->dmabuf)) {
> > - dma_resv_lock(attach->dmabuf->resv, NULL);
> > + if (dma_buf_attachment_is_dynamic(attach)) {
> > + dma_resv_lock(dmabuf->resv, NULL);
> > ret = dma_buf_pin(attach);
> > if (ret)
> > goto err_unlock;
> > @@ -772,8 +772,9 @@ 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);
> > + if (dma_buf_attachment_is_dynamic(attach))
> > + dma_resv_unlock(dmabuf->resv);
> > +
> > attach->sgt = sgt;
> > attach->dir = DMA_BIDIRECTIONAL;
> > }
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-11 14:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-05 10:51 [PATCH] dma-buf: Fix confusion of dynamic dma-buf vs dynamic attachment Chris Wilson
2021-03-05 10:51 ` Chris Wilson
2021-03-05 10:54 ` Christian König
2021-03-05 10:54 ` Christian König
2021-03-11 14:12 ` Daniel Vetter
2021-03-11 14:12 ` Daniel Vetter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.