Hi Lee and DRM folks.
On Fri, Nov 06, 2020 at 09:49:30PM +0000, Lee Jones wrote:
> This set is part of a larger effort attempting to clean-up W=1
> kernel builds, which are currently overwhelmingly riddled with
> niggly little warnings.
>
> There are 5000 warnings to work through. It will take a couple more
> sets. Although, ("drm/amd/display/dc/basics/fixpt31_32: Move
> variables to where they're used") does take care of 2000 of them!
>
> Lee Jones (19):
> drm/ttm/ttm_range_manager: Demote non-conformant kernel-doc header
> drm/r128/ati_pcigart: Source file headers are not good candidates for
> kernel-doc
Applied
> drm/selftests/test-drm_dp_mst_helper: Move
> 'sideband_msg_req_encode_decode' onto the heap
> drm/mga/mga_dma: Demote kernel-doc abusers to standard comment blocks
> drm/mga/mga_state: Remove unused variable 'buf_priv'
Applied x2
> drm/radeon/atom: Move prototype into shared location
> drm/radeon/radeon_kms: Include header containing our own prototypes
> drm/omapdrm/omap_gem: Fix misnamed and missing parameter descriptions
> drm/omapdrm/omap_dmm_tiler: Demote abusive use of kernel-doc format
> drm/radeon/radeon: Move prototype into shared header
> drm/radeon/radeon_drv: Source file headers are not good candidates for
> kernel-doc
> drm/amd/display/dc/basics/fixpt31_32: Move variables to where they're
> used
> drm/radeon/radeon_drv: Move prototypes to a shared headerfile
> drm/amd/amdgpu/amdgpu_device: Provide documentation for 'reg_addr'
> params
> drm/radeon: Move prototypes to shared header
> drm/amd/amdgpu/amdgpu_kms: Remove 'struct drm_amdgpu_info_device
> dev_info' from the stack
> drm/radeon/radeon_kms: Fix misnaming of 'radeon_info_ioctl's dev param
> drm/radeon/atombios_crtc: Remove description of non-existent function
> param 'encoder'
> drm/v3d/v3d_drv: Remove unused static variable 'v3d_v3d_pm_ops'
I have applied the three patches that has no obvious maintainer as indicated
above. I assume the respective maintaines to pick radeon, omapdrm, ttm,
amd, v3d and selftest patches.
Sam
From: Daniel Vetter <daniel.vetter(a)ffwll.ch>
commit f49a51bfdc8ea717c97ccd4cc98b7e6daaa5553a upstream.
When we forward an mmap to the dma_buf exporter, they get to own
everything. Unfortunately drm_gem_mmap_obj() overwrote
vma->vm_private_data after the driver callback, wreaking the
exporter complete. This was noticed because vb2_common_vm_close blew
up on mali gpu with panfrost after commit 26d3ac3cb04d
("drm/shmem-helpers: Redirect mmap for imported dma-buf").
Unfortunately drm_gem_mmap_obj also acquires a surplus reference that
we need to drop in shmem helpers, which is a bit of a mislayer
situation. Maybe the entire dma_buf_mmap forwarding should be pulled
into core gem code.
Note that the only two other drivers which forward mmap in their own
code (etnaviv and exynos) get this somewhat right by overwriting the
gem mmap code. But they seem to still have the leak. This might be a
good excuse to move these drivers over to shmem helpers completely.
Reviewed-by: Boris Brezillon <boris.brezillon(a)collabora.com>
Acked-by: Christian König <christian.koenig(a)amd.com>
Cc: Christian König <christian.koenig(a)amd.com>
Cc: Sumit Semwal <sumit.semwal(a)linaro.org>
Cc: Lucas Stach <l.stach(a)pengutronix.de>
Cc: Russell King <linux+etnaviv(a)armlinux.org.uk>
Cc: Christian Gmeiner <christian.gmeiner(a)gmail.com>
Cc: Inki Dae <inki.dae(a)samsung.com>
Cc: Joonyoung Shim <jy0922.shim(a)samsung.com>
Cc: Seung-Woo Kim <sw0312.kim(a)samsung.com>
Cc: Kyungmin Park <kyungmin.park(a)samsung.com>
Fixes: 26d3ac3cb04d ("drm/shmem-helpers: Redirect mmap for imported dma-buf")
Cc: Boris Brezillon <boris.brezillon(a)collabora.com>
Cc: Thomas Zimmermann <tzimmermann(a)suse.de>
Cc: Gerd Hoffmann <kraxel(a)redhat.com>
Cc: Rob Herring <robh(a)kernel.org>
Cc: dri-devel(a)lists.freedesktop.org
Cc: linux-media(a)vger.kernel.org
Cc: linaro-mm-sig(a)lists.linaro.org
Cc: <stable(a)vger.kernel.org> # v5.9+
Reported-and-tested-by: piotr.oniszczuk(a)gmail.com
Cc: piotr.oniszczuk(a)gmail.com
Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20201027214922.3566743-1-dani…
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
---
drivers/gpu/drm/drm_gem.c | 4 ++--
drivers/gpu/drm/drm_gem_shmem_helper.c | 7 ++++++-
2 files changed, 8 insertions(+), 3 deletions(-)
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1085,6 +1085,8 @@ int drm_gem_mmap_obj(struct drm_gem_obje
*/
drm_gem_object_get(obj);
+ vma->vm_private_data = obj;
+
if (obj->funcs && obj->funcs->mmap) {
ret = obj->funcs->mmap(obj, vma);
if (ret) {
@@ -1107,8 +1109,6 @@ int drm_gem_mmap_obj(struct drm_gem_obje
vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
}
- vma->vm_private_data = obj;
-
return 0;
}
EXPORT_SYMBOL(drm_gem_mmap_obj);
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -594,8 +594,13 @@ int drm_gem_shmem_mmap(struct drm_gem_ob
/* Remove the fake offset */
vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node);
- if (obj->import_attach)
+ if (obj->import_attach) {
+ /* Drop the reference drm_gem_mmap_obj() acquired.*/
+ drm_gem_object_put(obj);
+ vma->vm_private_data = NULL;
+
return dma_buf_mmap(obj->dma_buf, vma, 0);
+ }
shmem = to_drm_gem_shmem_obj(obj);
This is a note to let you know that I've just added the patch titled
drm/shme-helpers: Fix dma_buf_mmap forwarding bug
to the 5.9-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=sum…
The filename of the patch is:
drm-shme-helpers-fix-dma_buf_mmap-forwarding-bug.patch
and it can be found in the queue-5.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable(a)vger.kernel.org> know about it.
>From f49a51bfdc8ea717c97ccd4cc98b7e6daaa5553a Mon Sep 17 00:00:00 2001
From: Daniel Vetter <daniel.vetter(a)ffwll.ch>
Date: Tue, 27 Oct 2020 22:49:22 +0100
Subject: drm/shme-helpers: Fix dma_buf_mmap forwarding bug
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
From: Daniel Vetter <daniel.vetter(a)ffwll.ch>
commit f49a51bfdc8ea717c97ccd4cc98b7e6daaa5553a upstream.
When we forward an mmap to the dma_buf exporter, they get to own
everything. Unfortunately drm_gem_mmap_obj() overwrote
vma->vm_private_data after the driver callback, wreaking the
exporter complete. This was noticed because vb2_common_vm_close blew
up on mali gpu with panfrost after commit 26d3ac3cb04d
("drm/shmem-helpers: Redirect mmap for imported dma-buf").
Unfortunately drm_gem_mmap_obj also acquires a surplus reference that
we need to drop in shmem helpers, which is a bit of a mislayer
situation. Maybe the entire dma_buf_mmap forwarding should be pulled
into core gem code.
Note that the only two other drivers which forward mmap in their own
code (etnaviv and exynos) get this somewhat right by overwriting the
gem mmap code. But they seem to still have the leak. This might be a
good excuse to move these drivers over to shmem helpers completely.
Reviewed-by: Boris Brezillon <boris.brezillon(a)collabora.com>
Acked-by: Christian König <christian.koenig(a)amd.com>
Cc: Christian König <christian.koenig(a)amd.com>
Cc: Sumit Semwal <sumit.semwal(a)linaro.org>
Cc: Lucas Stach <l.stach(a)pengutronix.de>
Cc: Russell King <linux+etnaviv(a)armlinux.org.uk>
Cc: Christian Gmeiner <christian.gmeiner(a)gmail.com>
Cc: Inki Dae <inki.dae(a)samsung.com>
Cc: Joonyoung Shim <jy0922.shim(a)samsung.com>
Cc: Seung-Woo Kim <sw0312.kim(a)samsung.com>
Cc: Kyungmin Park <kyungmin.park(a)samsung.com>
Fixes: 26d3ac3cb04d ("drm/shmem-helpers: Redirect mmap for imported dma-buf")
Cc: Boris Brezillon <boris.brezillon(a)collabora.com>
Cc: Thomas Zimmermann <tzimmermann(a)suse.de>
Cc: Gerd Hoffmann <kraxel(a)redhat.com>
Cc: Rob Herring <robh(a)kernel.org>
Cc: dri-devel(a)lists.freedesktop.org
Cc: linux-media(a)vger.kernel.org
Cc: linaro-mm-sig(a)lists.linaro.org
Cc: <stable(a)vger.kernel.org> # v5.9+
Reported-and-tested-by: piotr.oniszczuk(a)gmail.com
Cc: piotr.oniszczuk(a)gmail.com
Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20201027214922.3566743-1-dani…
Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
---
drivers/gpu/drm/drm_gem.c | 4 ++--
drivers/gpu/drm/drm_gem_shmem_helper.c | 7 ++++++-
2 files changed, 8 insertions(+), 3 deletions(-)
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1085,6 +1085,8 @@ int drm_gem_mmap_obj(struct drm_gem_obje
*/
drm_gem_object_get(obj);
+ vma->vm_private_data = obj;
+
if (obj->funcs && obj->funcs->mmap) {
ret = obj->funcs->mmap(obj, vma);
if (ret) {
@@ -1107,8 +1109,6 @@ int drm_gem_mmap_obj(struct drm_gem_obje
vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
}
- vma->vm_private_data = obj;
-
return 0;
}
EXPORT_SYMBOL(drm_gem_mmap_obj);
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -594,8 +594,13 @@ int drm_gem_shmem_mmap(struct drm_gem_ob
/* Remove the fake offset */
vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node);
- if (obj->import_attach)
+ if (obj->import_attach) {
+ /* Drop the reference drm_gem_mmap_obj() acquired.*/
+ drm_gem_object_put(obj);
+ vma->vm_private_data = NULL;
+
return dma_buf_mmap(obj->dma_buf, vma, 0);
+ }
shmem = to_drm_gem_shmem_obj(obj);
Patches currently in stable-queue which might be from daniel.vetter(a)ffwll.ch are
queue-5.9/drm-ast-separate-drm-driver-from-pci-code.patch
queue-5.9/drm-shme-helpers-fix-dma_buf_mmap-forwarding-bug.patch
On Tue, Oct 27, 2020 at 12:52:30PM +0000, Parav Pandit wrote:
>
> > From: hch(a)lst.de <hch(a)lst.de>
> > Sent: Tuesday, October 27, 2020 1:41 PM
> >
> > On Mon, Oct 26, 2020 at 05:23:48AM +0000, Parav Pandit wrote:
> > > Hi Christoph,
> > >
> > > > From: Jakub Kicinski <kuba(a)kernel.org>
> > > > Sent: Saturday, October 24, 2020 11:45 PM
> > > >
> > > > CC: rdma, looks like rdma from the stack trace
> > > >
> > > > On Fri, 23 Oct 2020 20:07:17 -0700 syzbot wrote:
> > > > > syzbot has found a reproducer for the following issue on:
> > > > >
> > > > > HEAD commit: 3cb12d27 Merge tag 'net-5.10-rc1' of
> > git://git.kernel.org/..
> > >
> > > In [1] you mentioned that dma_mask should not be set for dma_virt_ops.
> > > So patch [2] removed it.
> > >
> > > But check to validate the dma mask for all dma_ops was added in [3].
> > >
> > > What is the right way? Did I misunderstood your comment about
> > dma_mask in [1]?
> >
> > No, I did not say we don't need the mask. I said copying over the various
> > dma-related fields from the parent is bogus.
> >
> > I think rxe (and ther other drivers/infiniband/sw drivers) need a simple
> > dma_coerce_mask_and_coherent and nothing else.
>
> I see. Does below fix make sense?
> Is DMA_MASK_NONE correct?
DMA_MASK_NONE is gone in 5.10. I think you want DMA_BIT_MASK(64).
That isn't actually correct for 32-bit platforms, but good enough.
On Wed, Oct 28, 2020 at 09:44:15AM +0100, Boris Brezillon wrote:
> On Tue, 27 Oct 2020 22:49:22 +0100
> Daniel Vetter <daniel.vetter(a)ffwll.ch> wrote:
>
> > When we forward an mmap to the dma_buf exporter, they get to own
> > everything. Unfortunately drm_gem_mmap_obj() overwrote
> > vma->vm_private_data after the driver callback, wreaking the
> > exporter complete. This was noticed because vb2_common_vm_close blew
> > up on mali gpu with panfrost after commit 26d3ac3cb04d
> > ("drm/shmem-helpers: Redirect mmap for imported dma-buf").
> >
> > Unfortunately drm_gem_mmap_obj also acquires a surplus reference that
> > we need to drop in shmem helpers, which is a bit of a mislayer
> > situation. Maybe the entire dma_buf_mmap forwarding should be pulled
> > into core gem code.
> >
> > Note that the only two other drivers which forward mmap in their own
> > code (etnaviv and exynos) get this somewhat right by overwriting the
> > gem mmap code. But they seem to still have the leak. This might be a
> > good excuse to move these drivers over to shmem helpers completely.
> >
> > Note to stable team: There's a trivial context conflict with
> > d693def4fd1c ("drm: Remove obsolete GEM and PRIME callbacks from
> > struct drm_driver"). I'm assuming it's clear where to put the first
> > hunk, otherwise I can send a 5.9 version of this.
> >
> > Cc: Christian König <christian.koenig(a)amd.com>
> > Cc: Sumit Semwal <sumit.semwal(a)linaro.org>
> > Cc: Lucas Stach <l.stach(a)pengutronix.de>
> > Cc: Russell King <linux+etnaviv(a)armlinux.org.uk>
> > Cc: Christian Gmeiner <christian.gmeiner(a)gmail.com>
> > Cc: Inki Dae <inki.dae(a)samsung.com>
> > Cc: Joonyoung Shim <jy0922.shim(a)samsung.com>
> > Cc: Seung-Woo Kim <sw0312.kim(a)samsung.com>
> > Cc: Kyungmin Park <kyungmin.park(a)samsung.com>
> > Fixes: 26d3ac3cb04d ("drm/shmem-helpers: Redirect mmap for imported dma-buf")
> > Cc: Boris Brezillon <boris.brezillon(a)collabora.com>
>
> Reviewed-by: Boris Brezillon <boris.brezillon(a)collabora.com>
Patch pushed to drm-misc-fixes, thanks for taking a look.
-Daniel
>
> > Cc: Thomas Zimmermann <tzimmermann(a)suse.de>
> > Cc: Gerd Hoffmann <kraxel(a)redhat.com>
> > Cc: Rob Herring <robh(a)kernel.org>
> > Cc: dri-devel(a)lists.freedesktop.org
> > Cc: linux-media(a)vger.kernel.org
> > Cc: linaro-mm-sig(a)lists.linaro.org
> > Cc: <stable(a)vger.kernel.org> # v5.9+
> > Reported-and-tested-by: piotr.oniszczuk(a)gmail.com
> > Cc: piotr.oniszczuk(a)gmail.com
> > Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com>
> > ---
> > drivers/gpu/drm/drm_gem.c | 4 ++--
> > drivers/gpu/drm/drm_gem_shmem_helper.c | 7 ++++++-
> > 2 files changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > index 1da67d34e55d..d586068f5509 100644
> > --- a/drivers/gpu/drm/drm_gem.c
> > +++ b/drivers/gpu/drm/drm_gem.c
> > @@ -1076,6 +1076,8 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
> > */
> > drm_gem_object_get(obj);
> >
> > + vma->vm_private_data = obj;
> > +
> > if (obj->funcs->mmap) {
> > ret = obj->funcs->mmap(obj, vma);
> > if (ret) {
> > @@ -1096,8 +1098,6 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
> > vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
> > }
> >
> > - vma->vm_private_data = obj;
> > -
> > return 0;
> > }
> > EXPORT_SYMBOL(drm_gem_mmap_obj);
> > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > index fb11df7aced5..8233bda4692f 100644
> > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > @@ -598,8 +598,13 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
> > /* Remove the fake offset */
> > vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node);
> >
> > - if (obj->import_attach)
> > + if (obj->import_attach) {
> > + /* Drop the reference drm_gem_mmap_obj() acquired.*/
> > + drm_gem_object_put(obj);
> > + vma->vm_private_data = NULL;
> > +
> > return dma_buf_mmap(obj->dma_buf, vma, 0);
> > + }
> >
> > shmem = to_drm_gem_shmem_obj(obj);
> >
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
When we forward an mmap to the dma_buf exporter, they get to own
everything. Unfortunately drm_gem_mmap_obj() overwrote
vma->vm_private_data after the driver callback, wreaking the
exporter complete. This was noticed because vb2_common_vm_close blew
up on mali gpu with panfrost after commit 26d3ac3cb04d
("drm/shmem-helpers: Redirect mmap for imported dma-buf").
Unfortunately drm_gem_mmap_obj also acquires a surplus reference that
we need to drop in shmem helpers, which is a bit of a mislayer
situation. Maybe the entire dma_buf_mmap forwarding should be pulled
into core gem code.
Note that the only two other drivers which forward mmap in their own
code (etnaviv and exynos) get this somewhat right by overwriting the
gem mmap code. But they seem to still have the leak. This might be a
good excuse to move these drivers over to shmem helpers completely.
Note to stable team: There's a trivial context conflict with
d693def4fd1c ("drm: Remove obsolete GEM and PRIME callbacks from
struct drm_driver"). I'm assuming it's clear where to put the first
hunk, otherwise I can send a 5.9 version of this.
Cc: Christian König <christian.koenig(a)amd.com>
Cc: Sumit Semwal <sumit.semwal(a)linaro.org>
Cc: Lucas Stach <l.stach(a)pengutronix.de>
Cc: Russell King <linux+etnaviv(a)armlinux.org.uk>
Cc: Christian Gmeiner <christian.gmeiner(a)gmail.com>
Cc: Inki Dae <inki.dae(a)samsung.com>
Cc: Joonyoung Shim <jy0922.shim(a)samsung.com>
Cc: Seung-Woo Kim <sw0312.kim(a)samsung.com>
Cc: Kyungmin Park <kyungmin.park(a)samsung.com>
Fixes: 26d3ac3cb04d ("drm/shmem-helpers: Redirect mmap for imported dma-buf")
Cc: Boris Brezillon <boris.brezillon(a)collabora.com>
Cc: Thomas Zimmermann <tzimmermann(a)suse.de>
Cc: Gerd Hoffmann <kraxel(a)redhat.com>
Cc: Rob Herring <robh(a)kernel.org>
Cc: dri-devel(a)lists.freedesktop.org
Cc: linux-media(a)vger.kernel.org
Cc: linaro-mm-sig(a)lists.linaro.org
Cc: <stable(a)vger.kernel.org> # v5.9+
Reported-and-tested-by: piotr.oniszczuk(a)gmail.com
Cc: piotr.oniszczuk(a)gmail.com
Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com>
---
drivers/gpu/drm/drm_gem.c | 4 ++--
drivers/gpu/drm/drm_gem_shmem_helper.c | 7 ++++++-
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 1da67d34e55d..d586068f5509 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1076,6 +1076,8 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
*/
drm_gem_object_get(obj);
+ vma->vm_private_data = obj;
+
if (obj->funcs->mmap) {
ret = obj->funcs->mmap(obj, vma);
if (ret) {
@@ -1096,8 +1098,6 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
}
- vma->vm_private_data = obj;
-
return 0;
}
EXPORT_SYMBOL(drm_gem_mmap_obj);
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index fb11df7aced5..8233bda4692f 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -598,8 +598,13 @@ int drm_gem_shmem_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
/* Remove the fake offset */
vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node);
- if (obj->import_attach)
+ if (obj->import_attach) {
+ /* Drop the reference drm_gem_mmap_obj() acquired.*/
+ drm_gem_object_put(obj);
+ vma->vm_private_data = NULL;
+
return dma_buf_mmap(obj->dma_buf, vma, 0);
+ }
shmem = to_drm_gem_shmem_obj(obj);
--
2.28.0
On Mon, Oct 26, 2020 at 05:23:48AM +0000, Parav Pandit wrote:
> Hi Christoph,
>
> > From: Jakub Kicinski <kuba(a)kernel.org>
> > Sent: Saturday, October 24, 2020 11:45 PM
> >
> > CC: rdma, looks like rdma from the stack trace
> >
> > On Fri, 23 Oct 2020 20:07:17 -0700 syzbot wrote:
> > > syzbot has found a reproducer for the following issue on:
> > >
> > > HEAD commit: 3cb12d27 Merge tag 'net-5.10-rc1' of git://git.kernel.org/..
>
> In [1] you mentioned that dma_mask should not be set for dma_virt_ops.
> So patch [2] removed it.
>
> But check to validate the dma mask for all dma_ops was added in [3].
>
> What is the right way? Did I misunderstood your comment about dma_mask in [1]?
No, I did not say we don't need the mask. I said copying over the
various dma-related fields from the parent is bogus.
I think rxe (and ther other drivers/infiniband/sw drivers) need a simple
dma_coerce_mask_and_coherent and nothing else.
From: Rob Clark <robdclark(a)chromium.org>
This doesn't remove *all* the struct_mutex, but it covers the worst
of it, ie. shrinker/madvise/free/retire. The submit path still uses
struct_mutex, but it still needs *something* serialize a portion of
the submit path, and lock_stat mostly just shows the lock contention
there being with other submits. And there are a few other bits of
struct_mutex usage in less critical paths (debugfs, etc). But this
seems like a reasonable step in the right direction.
v2: teach lockdep about shrinker locking patters (danvet) and
convert to obj->resv locking (danvet)
v3: fix get_vaddr locking for legacy userspace (relocs), devcoredump,
and rd/hangrd
v4: couple minor review comments (krh), fix deadlock with imported
dma-buf's (ie. from v4l2, etc)
Rob Clark (23):
drm/msm: Fix a couple incorrect usages of get_vaddr_active()
drm/msm/gem: Add obj->lock wrappers
drm/msm/gem: Rename internal get_iova_locked helper
drm/msm/gem: Move prototypes to msm_gem.h
drm/msm/gem: Add some _locked() helpers
drm/msm/gem: Move locking in shrinker path
drm/msm/submit: Move copy_from_user ahead of locking bos
drm/msm: Do rpm get sooner in the submit path
drm/msm/gem: Switch over to obj->resv for locking
drm/msm: Use correct drm_gem_object_put() in fail case
drm/msm: Drop chatty trace
drm/msm: Move update_fences()
drm/msm: Add priv->mm_lock to protect active/inactive lists
drm/msm: Document and rename preempt_lock
drm/msm: Protect ring->submits with it's own lock
drm/msm: Refcount submits
drm/msm: Remove obj->gpu
drm/msm: Drop struct_mutex from the retire path
drm/msm: Drop struct_mutex in free_object() path
drm/msm: Remove msm_gem_free_work
drm/msm: Drop struct_mutex in madvise path
drm/msm: Drop struct_mutex in shrinker path
drm/msm: Don't implicit-sync if only a single ring
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 6 +-
drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 12 +-
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 6 +-
drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 1 +
drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 1 +
drivers/gpu/drm/msm/dsi/dsi_host.c | 1 +
drivers/gpu/drm/msm/msm_debugfs.c | 7 +
drivers/gpu/drm/msm/msm_drv.c | 21 +-
drivers/gpu/drm/msm/msm_drv.h | 73 +-----
drivers/gpu/drm/msm/msm_fbdev.c | 1 +
drivers/gpu/drm/msm/msm_gem.c | 271 +++++++++++-----------
drivers/gpu/drm/msm/msm_gem.h | 133 +++++++++--
drivers/gpu/drm/msm/msm_gem_shrinker.c | 81 ++-----
drivers/gpu/drm/msm/msm_gem_submit.c | 164 ++++++++-----
drivers/gpu/drm/msm/msm_gpu.c | 110 +++++----
drivers/gpu/drm/msm/msm_gpu.h | 5 +-
drivers/gpu/drm/msm/msm_rd.c | 2 +-
drivers/gpu/drm/msm/msm_ringbuffer.c | 3 +-
drivers/gpu/drm/msm/msm_ringbuffer.h | 13 +-
19 files changed, 506 insertions(+), 405 deletions(-)
--
2.26.2
i915 does tons of allocations from this worker, which lockdep catches.
Also generic infrastructure like this with big potential for how
dma_fence or other cross driver contracts work, really should be
reviewed on dri-devel. Implementing custom wheels for everything
within the driver is a classic case of "platform problem" [1]. Which in
upstream we really shouldn't have.
Since there's no quick way to solve these splats (dma_fence_work is
used a bunch in basic buffer management and command submission) like
for amdgpu, I'm giving up at this point here. Annotating i915
scheduler and gpu reset could would be interesting, but since lockdep
is one-shot we can't see what surprises would lurk there.
1: https://lwn.net/Articles/443531/
Cc: linux-media(a)vger.kernel.org
Cc: linaro-mm-sig(a)lists.linaro.org
Cc: linux-rdma(a)vger.kernel.org
Cc: amd-gfx(a)lists.freedesktop.org
Cc: intel-gfx(a)lists.freedesktop.org
Cc: Chris Wilson <chris(a)chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst(a)linux.intel.com>
Cc: Christian König <christian.koenig(a)amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com>
---
drivers/gpu/drm/i915/i915_sw_fence_work.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.c b/drivers/gpu/drm/i915/i915_sw_fence_work.c
index a3a81bb8f2c3..5b74acadaef5 100644
--- a/drivers/gpu/drm/i915/i915_sw_fence_work.c
+++ b/drivers/gpu/drm/i915/i915_sw_fence_work.c
@@ -17,12 +17,15 @@ static void fence_work(struct work_struct *work)
{
struct dma_fence_work *f = container_of(work, typeof(*f), work);
int err;
+ bool fence_cookie;
+ fence_cookie = dma_fence_begin_signalling();
err = f->ops->work(f);
if (err)
dma_fence_set_error(&f->dma, err);
fence_complete(f);
+ dma_fence_end_signalling(fence_cookie);
dma_fence_put(&f->dma);
}
--
2.28.0