* [PATCH] dma-buf: fix unintended pin/unpin warnings
@ 2021-05-17 11:57 Christian König
2021-05-17 14:09 ` Alex Deucher
0 siblings, 1 reply; 5+ messages in thread
From: Christian König @ 2021-05-17 11:57 UTC (permalink / raw)
To: dri-devel, daniel
DMA-buf internal users call the pin/unpin functions without having a
dynamic attachment. Avoid the warning and backtrace in the logs.
Signed-off-by: Christian König <christian.koenig@amd.com>
Bugs: https://gitlab.freedesktop.org/drm/intel/-/issues/3481
Fixes: c545781e1c55 ("dma-buf: doc polish for pin/unpin")
CC: stable@kernel.org
---
drivers/dma-buf/dma-buf.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index f264b70c383e..eadd1eaa2fb5 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -760,7 +760,7 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
if (dma_buf_is_dynamic(attach->dmabuf)) {
dma_resv_lock(attach->dmabuf->resv, NULL);
- ret = dma_buf_pin(attach);
+ ret = dmabuf->ops->pin(attach);
if (ret)
goto err_unlock;
}
@@ -786,7 +786,7 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
err_unpin:
if (dma_buf_is_dynamic(attach->dmabuf))
- dma_buf_unpin(attach);
+ dmabuf->ops->unpin(attach);
err_unlock:
if (dma_buf_is_dynamic(attach->dmabuf))
@@ -843,7 +843,7 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach)
__unmap_dma_buf(attach, attach->sgt, attach->dir);
if (dma_buf_is_dynamic(attach->dmabuf)) {
- dma_buf_unpin(attach);
+ dmabuf->ops->unpin(attach);
dma_resv_unlock(attach->dmabuf->resv);
}
}
@@ -956,7 +956,7 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach,
if (dma_buf_is_dynamic(attach->dmabuf)) {
dma_resv_assert_held(attach->dmabuf->resv);
if (!IS_ENABLED(CONFIG_DMABUF_MOVE_NOTIFY)) {
- r = dma_buf_pin(attach);
+ r = attach->dmabuf->ops->pin(attach);
if (r)
return ERR_PTR(r);
}
@@ -968,7 +968,7 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach,
if (IS_ERR(sg_table) && dma_buf_is_dynamic(attach->dmabuf) &&
!IS_ENABLED(CONFIG_DMABUF_MOVE_NOTIFY))
- dma_buf_unpin(attach);
+ attach->dmabuf->ops->unpin(attach);
if (!IS_ERR(sg_table) && attach->dmabuf->ops->cache_sgt_mapping) {
attach->sgt = sg_table;
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] dma-buf: fix unintended pin/unpin warnings
2021-05-17 11:57 [PATCH] dma-buf: fix unintended pin/unpin warnings Christian König
@ 2021-05-17 14:09 ` Alex Deucher
2021-05-17 14:57 ` Daniel Vetter
0 siblings, 1 reply; 5+ messages in thread
From: Alex Deucher @ 2021-05-17 14:09 UTC (permalink / raw)
To: Christian König; +Cc: Maling list - DRI developers
On Mon, May 17, 2021 at 7:57 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> DMA-buf internal users call the pin/unpin functions without having a
> dynamic attachment. Avoid the warning and backtrace in the logs.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> Bugs: https://gitlab.freedesktop.org/drm/intel/-/issues/3481
> Fixes: c545781e1c55 ("dma-buf: doc polish for pin/unpin")
> CC: stable@kernel.org
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/dma-buf/dma-buf.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index f264b70c383e..eadd1eaa2fb5 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -760,7 +760,7 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
>
> if (dma_buf_is_dynamic(attach->dmabuf)) {
> dma_resv_lock(attach->dmabuf->resv, NULL);
> - ret = dma_buf_pin(attach);
> + ret = dmabuf->ops->pin(attach);
> if (ret)
> goto err_unlock;
> }
> @@ -786,7 +786,7 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
>
> err_unpin:
> if (dma_buf_is_dynamic(attach->dmabuf))
> - dma_buf_unpin(attach);
> + dmabuf->ops->unpin(attach);
>
> err_unlock:
> if (dma_buf_is_dynamic(attach->dmabuf))
> @@ -843,7 +843,7 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach)
> __unmap_dma_buf(attach, attach->sgt, attach->dir);
>
> if (dma_buf_is_dynamic(attach->dmabuf)) {
> - dma_buf_unpin(attach);
> + dmabuf->ops->unpin(attach);
> dma_resv_unlock(attach->dmabuf->resv);
> }
> }
> @@ -956,7 +956,7 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach,
> if (dma_buf_is_dynamic(attach->dmabuf)) {
> dma_resv_assert_held(attach->dmabuf->resv);
> if (!IS_ENABLED(CONFIG_DMABUF_MOVE_NOTIFY)) {
> - r = dma_buf_pin(attach);
> + r = attach->dmabuf->ops->pin(attach);
> if (r)
> return ERR_PTR(r);
> }
> @@ -968,7 +968,7 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach,
>
> if (IS_ERR(sg_table) && dma_buf_is_dynamic(attach->dmabuf) &&
> !IS_ENABLED(CONFIG_DMABUF_MOVE_NOTIFY))
> - dma_buf_unpin(attach);
> + attach->dmabuf->ops->unpin(attach);
>
> if (!IS_ERR(sg_table) && attach->dmabuf->ops->cache_sgt_mapping) {
> attach->sgt = sg_table;
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] dma-buf: fix unintended pin/unpin warnings
2021-05-17 14:09 ` Alex Deucher
@ 2021-05-17 14:57 ` Daniel Vetter
2021-05-20 11:37 ` Christian König
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Vetter @ 2021-05-17 14:57 UTC (permalink / raw)
To: Alex Deucher; +Cc: Christian König, Maling list - DRI developers
On Mon, May 17, 2021 at 10:09:13AM -0400, Alex Deucher wrote:
> On Mon, May 17, 2021 at 7:57 AM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
> >
> > DMA-buf internal users call the pin/unpin functions without having a
> > dynamic attachment. Avoid the warning and backtrace in the logs.
> >
> > Signed-off-by: Christian König <christian.koenig@amd.com>
> > Bugs: https://gitlab.freedesktop.org/drm/intel/-/issues/3481
> > Fixes: c545781e1c55 ("dma-buf: doc polish for pin/unpin")
> > CC: stable@kernel.org
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Hm this means we're losing the dma_resv_assert_held check, do we have that
in amdgpu callbacks to make sure we're not accidentally breaking stuff
later on?
But yeah lgtm, Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> > ---
> > drivers/dma-buf/dma-buf.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index f264b70c383e..eadd1eaa2fb5 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -760,7 +760,7 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
> >
> > if (dma_buf_is_dynamic(attach->dmabuf)) {
> > dma_resv_lock(attach->dmabuf->resv, NULL);
> > - ret = dma_buf_pin(attach);
> > + ret = dmabuf->ops->pin(attach);
> > if (ret)
> > goto err_unlock;
> > }
> > @@ -786,7 +786,7 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
> >
> > err_unpin:
> > if (dma_buf_is_dynamic(attach->dmabuf))
> > - dma_buf_unpin(attach);
> > + dmabuf->ops->unpin(attach);
> >
> > err_unlock:
> > if (dma_buf_is_dynamic(attach->dmabuf))
> > @@ -843,7 +843,7 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach)
> > __unmap_dma_buf(attach, attach->sgt, attach->dir);
> >
> > if (dma_buf_is_dynamic(attach->dmabuf)) {
> > - dma_buf_unpin(attach);
> > + dmabuf->ops->unpin(attach);
> > dma_resv_unlock(attach->dmabuf->resv);
> > }
> > }
> > @@ -956,7 +956,7 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach,
> > if (dma_buf_is_dynamic(attach->dmabuf)) {
> > dma_resv_assert_held(attach->dmabuf->resv);
> > if (!IS_ENABLED(CONFIG_DMABUF_MOVE_NOTIFY)) {
> > - r = dma_buf_pin(attach);
> > + r = attach->dmabuf->ops->pin(attach);
> > if (r)
> > return ERR_PTR(r);
> > }
> > @@ -968,7 +968,7 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach,
> >
> > if (IS_ERR(sg_table) && dma_buf_is_dynamic(attach->dmabuf) &&
> > !IS_ENABLED(CONFIG_DMABUF_MOVE_NOTIFY))
> > - dma_buf_unpin(attach);
> > + attach->dmabuf->ops->unpin(attach);
> >
> > if (!IS_ERR(sg_table) && attach->dmabuf->ops->cache_sgt_mapping) {
> > attach->sgt = sg_table;
> > --
> > 2.25.1
> >
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] dma-buf: fix unintended pin/unpin warnings
2021-05-17 14:57 ` Daniel Vetter
@ 2021-05-20 11:37 ` Christian König
2021-05-20 14:13 ` Daniel Vetter
0 siblings, 1 reply; 5+ messages in thread
From: Christian König @ 2021-05-20 11:37 UTC (permalink / raw)
To: Daniel Vetter, Alex Deucher; +Cc: Maling list - DRI developers
Am 17.05.21 um 16:57 schrieb Daniel Vetter:
> On Mon, May 17, 2021 at 10:09:13AM -0400, Alex Deucher wrote:
>> On Mon, May 17, 2021 at 7:57 AM Christian König
>> <ckoenig.leichtzumerken@gmail.com> wrote:
>>> DMA-buf internal users call the pin/unpin functions without having a
>>> dynamic attachment. Avoid the warning and backtrace in the logs.
>>>
>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>> Bugs: https://gitlab.freedesktop.org/drm/intel/-/issues/3481
>>> Fixes: c545781e1c55 ("dma-buf: doc polish for pin/unpin")
>>> CC: stable@kernel.org
>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> Hm this means we're losing the dma_resv_assert_held check, do we have that
> in amdgpu callbacks to make sure we're not accidentally breaking stuff
> later on?
Mhm, well this is just for calling the pin/unpin internally from the
DMA-buf framework itself.
Need to double check, but I think all those cases either have a
dma_resv_assert_held() or are locking the reservation themselves before
calling the function.
But yeah, rather good point.
Christian.
>
> But yeah lgtm, Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
>>> ---
>>> drivers/dma-buf/dma-buf.c | 10 +++++-----
>>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>>> index f264b70c383e..eadd1eaa2fb5 100644
>>> --- a/drivers/dma-buf/dma-buf.c
>>> +++ b/drivers/dma-buf/dma-buf.c
>>> @@ -760,7 +760,7 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
>>>
>>> if (dma_buf_is_dynamic(attach->dmabuf)) {
>>> dma_resv_lock(attach->dmabuf->resv, NULL);
>>> - ret = dma_buf_pin(attach);
>>> + ret = dmabuf->ops->pin(attach);
>>> if (ret)
>>> goto err_unlock;
>>> }
>>> @@ -786,7 +786,7 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
>>>
>>> err_unpin:
>>> if (dma_buf_is_dynamic(attach->dmabuf))
>>> - dma_buf_unpin(attach);
>>> + dmabuf->ops->unpin(attach);
>>>
>>> err_unlock:
>>> if (dma_buf_is_dynamic(attach->dmabuf))
>>> @@ -843,7 +843,7 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach)
>>> __unmap_dma_buf(attach, attach->sgt, attach->dir);
>>>
>>> if (dma_buf_is_dynamic(attach->dmabuf)) {
>>> - dma_buf_unpin(attach);
>>> + dmabuf->ops->unpin(attach);
>>> dma_resv_unlock(attach->dmabuf->resv);
>>> }
>>> }
>>> @@ -956,7 +956,7 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach,
>>> if (dma_buf_is_dynamic(attach->dmabuf)) {
>>> dma_resv_assert_held(attach->dmabuf->resv);
>>> if (!IS_ENABLED(CONFIG_DMABUF_MOVE_NOTIFY)) {
>>> - r = dma_buf_pin(attach);
>>> + r = attach->dmabuf->ops->pin(attach);
>>> if (r)
>>> return ERR_PTR(r);
>>> }
>>> @@ -968,7 +968,7 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach,
>>>
>>> if (IS_ERR(sg_table) && dma_buf_is_dynamic(attach->dmabuf) &&
>>> !IS_ENABLED(CONFIG_DMABUF_MOVE_NOTIFY))
>>> - dma_buf_unpin(attach);
>>> + attach->dmabuf->ops->unpin(attach);
>>>
>>> if (!IS_ERR(sg_table) && attach->dmabuf->ops->cache_sgt_mapping) {
>>> attach->sgt = sg_table;
>>> --
>>> 2.25.1
>>>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] dma-buf: fix unintended pin/unpin warnings
2021-05-20 11:37 ` Christian König
@ 2021-05-20 14:13 ` Daniel Vetter
0 siblings, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2021-05-20 14:13 UTC (permalink / raw)
To: Christian König; +Cc: Maling list - DRI developers
On Thu, May 20, 2021 at 01:37:47PM +0200, Christian König wrote:
> Am 17.05.21 um 16:57 schrieb Daniel Vetter:
> > On Mon, May 17, 2021 at 10:09:13AM -0400, Alex Deucher wrote:
> > > On Mon, May 17, 2021 at 7:57 AM Christian König
> > > <ckoenig.leichtzumerken@gmail.com> wrote:
> > > > DMA-buf internal users call the pin/unpin functions without having a
> > > > dynamic attachment. Avoid the warning and backtrace in the logs.
> > > >
> > > > Signed-off-by: Christian König <christian.koenig@amd.com>
> > > > Bugs: https://gitlab.freedesktop.org/drm/intel/-/issues/3481
> > > > Fixes: c545781e1c55 ("dma-buf: doc polish for pin/unpin")
> > > > CC: stable@kernel.org
> > > Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> > Hm this means we're losing the dma_resv_assert_held check, do we have that
> > in amdgpu callbacks to make sure we're not accidentally breaking stuff
> > later on?
>
> Mhm, well this is just for calling the pin/unpin internally from the DMA-buf
> framework itself.
It's both, but currently I think no users for it (since it would be
pinning for display).
Maybe we could do __ internal wrappers to keep the lockdep checks?
> Need to double check, but I think all those cases either have a
> dma_resv_assert_held() or are locking the reservation themselves before
> calling the function.
Yeah right now it's still all good, but that changes easily :-)
> But yeah, rather good point.
Do you plan to type something that I can ack?
-Daniel
>
> Christian.
>
> >
> > But yeah lgtm, Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >
> > > > ---
> > > > drivers/dma-buf/dma-buf.c | 10 +++++-----
> > > > 1 file changed, 5 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > > > index f264b70c383e..eadd1eaa2fb5 100644
> > > > --- a/drivers/dma-buf/dma-buf.c
> > > > +++ b/drivers/dma-buf/dma-buf.c
> > > > @@ -760,7 +760,7 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
> > > >
> > > > if (dma_buf_is_dynamic(attach->dmabuf)) {
> > > > dma_resv_lock(attach->dmabuf->resv, NULL);
> > > > - ret = dma_buf_pin(attach);
> > > > + ret = dmabuf->ops->pin(attach);
> > > > if (ret)
> > > > goto err_unlock;
> > > > }
> > > > @@ -786,7 +786,7 @@ dma_buf_dynamic_attach(struct dma_buf *dmabuf, struct device *dev,
> > > >
> > > > err_unpin:
> > > > if (dma_buf_is_dynamic(attach->dmabuf))
> > > > - dma_buf_unpin(attach);
> > > > + dmabuf->ops->unpin(attach);
> > > >
> > > > err_unlock:
> > > > if (dma_buf_is_dynamic(attach->dmabuf))
> > > > @@ -843,7 +843,7 @@ void dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach)
> > > > __unmap_dma_buf(attach, attach->sgt, attach->dir);
> > > >
> > > > if (dma_buf_is_dynamic(attach->dmabuf)) {
> > > > - dma_buf_unpin(attach);
> > > > + dmabuf->ops->unpin(attach);
> > > > dma_resv_unlock(attach->dmabuf->resv);
> > > > }
> > > > }
> > > > @@ -956,7 +956,7 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach,
> > > > if (dma_buf_is_dynamic(attach->dmabuf)) {
> > > > dma_resv_assert_held(attach->dmabuf->resv);
> > > > if (!IS_ENABLED(CONFIG_DMABUF_MOVE_NOTIFY)) {
> > > > - r = dma_buf_pin(attach);
> > > > + r = attach->dmabuf->ops->pin(attach);
> > > > if (r)
> > > > return ERR_PTR(r);
> > > > }
> > > > @@ -968,7 +968,7 @@ struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach,
> > > >
> > > > if (IS_ERR(sg_table) && dma_buf_is_dynamic(attach->dmabuf) &&
> > > > !IS_ENABLED(CONFIG_DMABUF_MOVE_NOTIFY))
> > > > - dma_buf_unpin(attach);
> > > > + attach->dmabuf->ops->unpin(attach);
> > > >
> > > > if (!IS_ERR(sg_table) && attach->dmabuf->ops->cache_sgt_mapping) {
> > > > attach->sgt = sg_table;
> > > > --
> > > > 2.25.1
> > > >
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-05-20 14:13 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-17 11:57 [PATCH] dma-buf: fix unintended pin/unpin warnings Christian König
2021-05-17 14:09 ` Alex Deucher
2021-05-17 14:57 ` Daniel Vetter
2021-05-20 11:37 ` Christian König
2021-05-20 14:13 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).