* [PATCH v3 1/3] drm/prime: Pass the right module owner through to dma_buf_export()
@ 2016-09-30 13:59 Chris Wilson
2016-09-30 13:59 ` [PATCH v3 2/3] drm/prime: Take a ref on the drm_dev when exporting a dma_buf Chris Wilson
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Chris Wilson @ 2016-09-30 13:59 UTC (permalink / raw)
To: dri-devel
Cc: intel-gfx, Chris Wilson, Petri Latvala, Christian König, stable
dma_buf_export() adds a reference to the owning module to the dmabuf (to
prevent the driver from being unloaded whilst a third party still refers
to the dmabuf). However, drm_gem_prime_export() was passing its own
THIS_MODULE (i.e. drm.ko) rather than the driver. Extract the right
owner from the device->fops instead.
v2: Use C99 initializers to zero out unset elements of
dma_buf_export_info
v3: Extract the right module from dev->fops.
Testcase: igt/vgem_basic/unload
Reported-by: Petri Latvala <petri.latvala@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Petri Latvala <petri.latvala@intel.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: stable@vger.kernel.org
Tested-by: Petri Latvala <petri.latvala@intel.com>
---
drivers/gpu/drm/drm_prime.c | 17 ++++++++++-------
include/drm/drmP.h | 3 ++-
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 57201d68cf61..80907b34d857 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -397,14 +397,17 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = {
* using the PRIME helpers.
*/
struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
- struct drm_gem_object *obj, int flags)
+ struct drm_gem_object *obj,
+ int flags)
{
- DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
-
- exp_info.ops = &drm_gem_prime_dmabuf_ops;
- exp_info.size = obj->size;
- exp_info.flags = flags;
- exp_info.priv = obj;
+ struct dma_buf_export_info exp_info = {
+ .exp_name = KBUILD_MODNAME, /* white lie for debug */
+ .owner = dev->driver->fops->owner,
+ .ops = &drm_gem_prime_dmabuf_ops,
+ .size = obj->size,
+ .flags = flags,
+ .priv = obj,
+ };
if (dev->driver->gem_prime_res_obj)
exp_info.resv = dev->driver->gem_prime_res_obj(obj);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 0e99669159c1..81fcd553edf7 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1012,7 +1012,8 @@ static inline int drm_debugfs_remove_files(const struct drm_info_list *files,
#endif
extern struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
- struct drm_gem_object *obj, int flags);
+ struct drm_gem_object *obj,
+ int flags);
extern int drm_gem_prime_handle_to_fd(struct drm_device *dev,
struct drm_file *file_priv, uint32_t handle, uint32_t flags,
int *prime_fd);
--
2.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 2/3] drm/prime: Take a ref on the drm_dev when exporting a dma_buf
2016-09-30 13:59 [PATCH v3 1/3] drm/prime: Pass the right module owner through to dma_buf_export() Chris Wilson
@ 2016-09-30 13:59 ` Chris Wilson
2016-09-30 13:59 ` [PATCH v3 3/3] drm/vgem: Keep a reference to the dmabuf when mmaped Chris Wilson
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2016-09-30 13:59 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx, Chris Wilson, Petri Latvala, Daniel Vetter, stable
dma_buf may live a long time, longer than the last direct user of the
driver. We already hold a reference to the owner module (that prevents
the object code from disappearing), but there is no reference to the
drm_dev - so the pointers to the driver backend themselves may vanish.
Testcase: igt/vgem_basic/unload
Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Petri Latvala <petri.latvala@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: stable@vger.kernel.org
Tested-by: Petri Latvala <petri.latvala@intel.com>
---
drivers/gpu/drm/armada/armada_gem.c | 9 +++++++--
drivers/gpu/drm/drm_prime.c | 10 +++++++++-
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 1 +
drivers/gpu/drm/tegra/gem.c | 7 ++++++-
drivers/gpu/drm/udl/udl_dmabuf.c | 7 ++++++-
5 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/armada/armada_gem.c b/drivers/gpu/drm/armada/armada_gem.c
index cb8f0347b934..bdd3af043827 100644
--- a/drivers/gpu/drm/armada/armada_gem.c
+++ b/drivers/gpu/drm/armada/armada_gem.c
@@ -541,13 +541,18 @@ armada_gem_prime_export(struct drm_device *dev, struct drm_gem_object *obj,
int flags)
{
DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
+ struct dma_buf *dma_buf;
exp_info.ops = &armada_gem_prime_dmabuf_ops;
exp_info.size = obj->size;
- exp_info.flags = O_RDWR;
+ exp_info.flags = flags;
exp_info.priv = obj;
- return dma_buf_export(&exp_info);
+ dma_buf = dma_buf_export(&exp_info);
+ if (!IS_ERR(dma_buf))
+ drm_dev_ref(dev);
+
+ return dma_buf;
}
struct drm_gem_object *
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 80907b34d857..5081f187d9e3 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -293,9 +293,12 @@ static void drm_gem_unmap_dma_buf(struct dma_buf_attachment *attach,
void drm_gem_dmabuf_release(struct dma_buf *dma_buf)
{
struct drm_gem_object *obj = dma_buf->priv;
+ struct drm_device *dev = obj->dev;
/* drop the reference on the export fd holds */
drm_gem_object_unreference_unlocked(obj);
+
+ drm_dev_unref(dev);
}
EXPORT_SYMBOL(drm_gem_dmabuf_release);
@@ -408,11 +411,16 @@ struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
.flags = flags,
.priv = obj,
};
+ struct dma_buf *dma_buf;
if (dev->driver->gem_prime_res_obj)
exp_info.resv = dev->driver->gem_prime_res_obj(obj);
- return dma_buf_export(&exp_info);
+ dma_buf = dma_buf_export(&exp_info);
+ if (!IS_ERR(dma_buf))
+ drm_dev_ref(dev);
+
+ return dma_buf;
}
EXPORT_SYMBOL(drm_gem_prime_export);
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index 10265bb35604..b58b2e63add1 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -288,6 +288,7 @@ struct dma_buf *i915_gem_prime_export(struct drm_device *dev,
return dma_buf;
export_fences(obj, dma_buf);
+ drm_dev_ref(obj->base.dev);
return dma_buf;
}
diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index aa60d9909ea2..aaae8b6ba668 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -607,13 +607,18 @@ struct dma_buf *tegra_gem_prime_export(struct drm_device *drm,
int flags)
{
DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
+ struct dma_buf *dma_buf;
exp_info.ops = &tegra_gem_prime_dmabuf_ops;
exp_info.size = gem->size;
exp_info.flags = flags;
exp_info.priv = gem;
- return dma_buf_export(&exp_info);
+ dma_buf = dma_buf_export(&exp_info);
+ if (!IS_ERR(dma_buf))
+ drm_dev_ref(drm);
+
+ return dma_buf;
}
struct drm_gem_object *tegra_gem_prime_import(struct drm_device *drm,
diff --git a/drivers/gpu/drm/udl/udl_dmabuf.c b/drivers/gpu/drm/udl/udl_dmabuf.c
index e2243edd1ce3..249cc1072112 100644
--- a/drivers/gpu/drm/udl/udl_dmabuf.c
+++ b/drivers/gpu/drm/udl/udl_dmabuf.c
@@ -203,13 +203,18 @@ struct dma_buf *udl_gem_prime_export(struct drm_device *dev,
struct drm_gem_object *obj, int flags)
{
DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
+ struct dma_buf *dma_buf;
exp_info.ops = &udl_dmabuf_ops;
exp_info.size = obj->size;
exp_info.flags = flags;
exp_info.priv = obj;
- return dma_buf_export(&exp_info);
+ dma_buf = dma_buf_export(&exp_info);
+ if (!IS_ERR(dma_buf))
+ drm_dev_ref(dev);
+
+ return dma_buf;
}
static int udl_prime_create(struct drm_device *dev,
--
2.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 3/3] drm/vgem: Keep a reference to the dmabuf when mmaped
2016-09-30 13:59 [PATCH v3 1/3] drm/prime: Pass the right module owner through to dma_buf_export() Chris Wilson
2016-09-30 13:59 ` [PATCH v3 2/3] drm/prime: Take a ref on the drm_dev when exporting a dma_buf Chris Wilson
@ 2016-09-30 13:59 ` Chris Wilson
2016-10-05 13:11 ` Daniel Vetter
2016-09-30 14:22 ` Christian König
2016-09-30 14:49 ` ✗ Fi.CI.BAT: warning for series starting with [v3,1/3] " Patchwork
3 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2016-09-30 13:59 UTC (permalink / raw)
To: dri-devel; +Cc: intel-gfx
In order to keep the dmabuf alive whilst the mmap is, we need to hold a
reference to the dmabuf and not the backing object. This is important as
the dmabuf not only keeps the object alive, but also the device so that
dmabuf = vgem_create_dmabuf();
ptr = mmap(... dmabuf ...);
close(dmabuf);
persists across module-unload as well as device closure.
Testcase: igt/vgem_basic/unload
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Petri Latvala <petri.latvala@intel.com>
---
drivers/gpu/drm/vgem/vgem_drv.c | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index f36c14729b55..74a83e41efa9 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -198,7 +198,6 @@ static struct drm_ioctl_desc vgem_ioctls[] = {
static int vgem_mmap(struct file *filp, struct vm_area_struct *vma)
{
- unsigned long flags = vma->vm_flags;
int ret;
ret = drm_gem_mmap(filp, vma);
@@ -208,7 +207,7 @@ static int vgem_mmap(struct file *filp, struct vm_area_struct *vma)
/* Keep the WC mmaping set by drm_gem_mmap() but our pages
* are ordinary and not special.
*/
- vma->vm_flags = flags | VM_DONTEXPAND | VM_DONTDUMP;
+ vma->vm_flags &= ~(VM_IO | VM_PFNMAP);
return 0;
}
@@ -281,21 +280,11 @@ static int vgem_prime_mmap(struct drm_gem_object *obj,
{
int ret;
- if (obj->size < vma->vm_end - vma->vm_start)
- return -EINVAL;
-
- if (!obj->filp)
- return -ENODEV;
-
- ret = obj->filp->f_op->mmap(obj->filp, vma);
+ ret = drm_gem_mmap_obj(obj, obj->size, vma);
if (ret)
return ret;
- fput(vma->vm_file);
- vma->vm_file = get_file(obj->filp);
- vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
- vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
-
+ vma->vm_flags &= ~(VM_IO | VM_PFNMAP);
return 0;
}
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] drm/prime: Pass the right module owner through to dma_buf_export()
2016-09-30 13:59 [PATCH v3 1/3] drm/prime: Pass the right module owner through to dma_buf_export() Chris Wilson
@ 2016-09-30 14:22 ` Christian König
2016-09-30 13:59 ` [PATCH v3 3/3] drm/vgem: Keep a reference to the dmabuf when mmaped Chris Wilson
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Christian König @ 2016-09-30 14:22 UTC (permalink / raw)
To: Chris Wilson, dri-devel; +Cc: intel-gfx, Petri Latvala, stable
Am 30.09.2016 um 15:59 schrieb Chris Wilson:
> dma_buf_export() adds a reference to the owning module to the dmabuf (to
> prevent the driver from being unloaded whilst a third party still refers
> to the dmabuf). However, drm_gem_prime_export() was passing its own
> THIS_MODULE (i.e. drm.ko) rather than the driver. Extract the right
> owner from the device->fops instead.
>
> v2: Use C99 initializers to zero out unset elements of
> dma_buf_export_info
> v3: Extract the right module from dev->fops.
>
> Testcase: igt/vgem_basic/unload
> Reported-by: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: stable@vger.kernel.org
> Tested-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/drm_prime.c | 17 ++++++++++-------
> include/drm/drmP.h | 3 ++-
> 2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 57201d68cf61..80907b34d857 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -397,14 +397,17 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = {
> * using the PRIME helpers.
> */
> struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
> - struct drm_gem_object *obj, int flags)
> + struct drm_gem_object *obj,
> + int flags)
> {
> - DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> -
> - exp_info.ops = &drm_gem_prime_dmabuf_ops;
> - exp_info.size = obj->size;
> - exp_info.flags = flags;
> - exp_info.priv = obj;
> + struct dma_buf_export_info exp_info = {
> + .exp_name = KBUILD_MODNAME, /* white lie for debug */
> + .owner = dev->driver->fops->owner,
> + .ops = &drm_gem_prime_dmabuf_ops,
> + .size = obj->size,
> + .flags = flags,
> + .priv = obj,
> + };
>
> if (dev->driver->gem_prime_res_obj)
> exp_info.resv = dev->driver->gem_prime_res_obj(obj);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 0e99669159c1..81fcd553edf7 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1012,7 +1012,8 @@ static inline int drm_debugfs_remove_files(const struct drm_info_list *files,
> #endif
>
> extern struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
> - struct drm_gem_object *obj, int flags);
> + struct drm_gem_object *obj,
> + int flags);
> extern int drm_gem_prime_handle_to_fd(struct drm_device *dev,
> struct drm_file *file_priv, uint32_t handle, uint32_t flags,
> int *prime_fd);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] drm/prime: Pass the right module owner through to dma_buf_export()
@ 2016-09-30 14:22 ` Christian König
0 siblings, 0 replies; 9+ messages in thread
From: Christian König @ 2016-09-30 14:22 UTC (permalink / raw)
To: Chris Wilson, dri-devel; +Cc: intel-gfx, stable
Am 30.09.2016 um 15:59 schrieb Chris Wilson:
> dma_buf_export() adds a reference to the owning module to the dmabuf (to
> prevent the driver from being unloaded whilst a third party still refers
> to the dmabuf). However, drm_gem_prime_export() was passing its own
> THIS_MODULE (i.e. drm.ko) rather than the driver. Extract the right
> owner from the device->fops instead.
>
> v2: Use C99 initializers to zero out unset elements of
> dma_buf_export_info
> v3: Extract the right module from dev->fops.
>
> Testcase: igt/vgem_basic/unload
> Reported-by: Petri Latvala <petri.latvala@intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Petri Latvala <petri.latvala@intel.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: stable@vger.kernel.org
> Tested-by: Petri Latvala <petri.latvala@intel.com>
Reviewed-by: Christian König <christian.koenig@amd.com>
> ---
> drivers/gpu/drm/drm_prime.c | 17 ++++++++++-------
> include/drm/drmP.h | 3 ++-
> 2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 57201d68cf61..80907b34d857 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -397,14 +397,17 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = {
> * using the PRIME helpers.
> */
> struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
> - struct drm_gem_object *obj, int flags)
> + struct drm_gem_object *obj,
> + int flags)
> {
> - DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> -
> - exp_info.ops = &drm_gem_prime_dmabuf_ops;
> - exp_info.size = obj->size;
> - exp_info.flags = flags;
> - exp_info.priv = obj;
> + struct dma_buf_export_info exp_info = {
> + .exp_name = KBUILD_MODNAME, /* white lie for debug */
> + .owner = dev->driver->fops->owner,
> + .ops = &drm_gem_prime_dmabuf_ops,
> + .size = obj->size,
> + .flags = flags,
> + .priv = obj,
> + };
>
> if (dev->driver->gem_prime_res_obj)
> exp_info.resv = dev->driver->gem_prime_res_obj(obj);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 0e99669159c1..81fcd553edf7 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1012,7 +1012,8 @@ static inline int drm_debugfs_remove_files(const struct drm_info_list *files,
> #endif
>
> extern struct dma_buf *drm_gem_prime_export(struct drm_device *dev,
> - struct drm_gem_object *obj, int flags);
> + struct drm_gem_object *obj,
> + int flags);
> extern int drm_gem_prime_handle_to_fd(struct drm_device *dev,
> struct drm_file *file_priv, uint32_t handle, uint32_t flags,
> int *prime_fd);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* ✗ Fi.CI.BAT: warning for series starting with [v3,1/3] drm/prime: Pass the right module owner through to dma_buf_export()
2016-09-30 13:59 [PATCH v3 1/3] drm/prime: Pass the right module owner through to dma_buf_export() Chris Wilson
` (2 preceding siblings ...)
2016-09-30 14:22 ` Christian König
@ 2016-09-30 14:49 ` Patchwork
3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2016-09-30 14:49 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [v3,1/3] drm/prime: Pass the right module owner through to dma_buf_export()
URL : https://patchwork.freedesktop.org/series/13153/
State : warning
== Summary ==
Series 13153v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/13153/revisions/1/mbox/
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
pass -> DMESG-WARN (fi-byt-j1900)
Subgroup suspend-read-crc-pipe-b:
pass -> DMESG-WARN (fi-byt-j1900)
fi-bdw-5557u total:244 pass:229 dwarn:0 dfail:0 fail:0 skip:15
fi-bsw-n3050 total:244 pass:202 dwarn:0 dfail:0 fail:0 skip:42
fi-bxt-t5700 total:244 pass:214 dwarn:0 dfail:0 fail:0 skip:30
fi-byt-j1900 total:244 pass:210 dwarn:2 dfail:0 fail:1 skip:31
fi-byt-n2820 total:244 pass:208 dwarn:0 dfail:0 fail:1 skip:35
fi-hsw-4770 total:244 pass:222 dwarn:0 dfail:0 fail:0 skip:22
fi-hsw-4770r total:244 pass:222 dwarn:0 dfail:0 fail:0 skip:22
fi-ilk-650 total:244 pass:182 dwarn:0 dfail:0 fail:2 skip:60
fi-ivb-3520m total:244 pass:219 dwarn:0 dfail:0 fail:0 skip:25
fi-ivb-3770 total:244 pass:207 dwarn:0 dfail:0 fail:0 skip:37
fi-skl-6260u total:244 pass:230 dwarn:0 dfail:0 fail:0 skip:14
fi-skl-6700hq total:244 pass:221 dwarn:1 dfail:0 fail:0 skip:22
fi-skl-6700k total:244 pass:219 dwarn:1 dfail:0 fail:0 skip:24
fi-skl-6770hq total:244 pass:228 dwarn:1 dfail:0 fail:1 skip:14
fi-snb-2520m total:244 pass:208 dwarn:0 dfail:0 fail:0 skip:36
fi-snb-2600 total:244 pass:207 dwarn:0 dfail:0 fail:0 skip:37
Results at /archive/results/CI_IGT_test/Patchwork_2605/
d0534bd0217c83c083ba146b9c987e19b219e0e4 drm-intel-nightly: 2016y-09m-30d-10h-31m-00s UTC integration manifest
ed416f7 drm/vgem: Keep a reference to the dmabuf when mmaped
8fb9114 drm/prime: Take a ref on the drm_dev when exporting a dma_buf
ab7d9a2 drm/prime: Pass the right module owner through to dma_buf_export()
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/3] drm/vgem: Keep a reference to the dmabuf when mmaped
2016-09-30 13:59 ` [PATCH v3 3/3] drm/vgem: Keep a reference to the dmabuf when mmaped Chris Wilson
@ 2016-10-05 13:11 ` Daniel Vetter
2016-10-05 13:40 ` Chris Wilson
0 siblings, 1 reply; 9+ messages in thread
From: Daniel Vetter @ 2016-10-05 13:11 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx, dri-devel
On Fri, Sep 30, 2016 at 02:59:59PM +0100, Chris Wilson wrote:
> In order to keep the dmabuf alive whilst the mmap is, we need to hold a
> reference to the dmabuf and not the backing object. This is important as
> the dmabuf not only keeps the object alive, but also the device so that
>
> dmabuf = vgem_create_dmabuf();
> ptr = mmap(... dmabuf ...);
> close(dmabuf);
>
> persists across module-unload as well as device closure.
I don't see where we grab the ref to the dma-buf here instead of the
backing storage. And doesn't the exact same issue happen when you use dumb
mmap? Or maybe I'm just a bit confused about what's going on here ...
-Daniel
>
> Testcase: igt/vgem_basic/unload
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Petri Latvala <petri.latvala@intel.com>
> ---
> drivers/gpu/drm/vgem/vgem_drv.c | 17 +++--------------
> 1 file changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
> index f36c14729b55..74a83e41efa9 100644
> --- a/drivers/gpu/drm/vgem/vgem_drv.c
> +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> @@ -198,7 +198,6 @@ static struct drm_ioctl_desc vgem_ioctls[] = {
>
> static int vgem_mmap(struct file *filp, struct vm_area_struct *vma)
> {
> - unsigned long flags = vma->vm_flags;
> int ret;
>
> ret = drm_gem_mmap(filp, vma);
> @@ -208,7 +207,7 @@ static int vgem_mmap(struct file *filp, struct vm_area_struct *vma)
> /* Keep the WC mmaping set by drm_gem_mmap() but our pages
> * are ordinary and not special.
> */
> - vma->vm_flags = flags | VM_DONTEXPAND | VM_DONTDUMP;
> + vma->vm_flags &= ~(VM_IO | VM_PFNMAP);
> return 0;
> }
>
> @@ -281,21 +280,11 @@ static int vgem_prime_mmap(struct drm_gem_object *obj,
> {
> int ret;
>
> - if (obj->size < vma->vm_end - vma->vm_start)
> - return -EINVAL;
> -
> - if (!obj->filp)
> - return -ENODEV;
> -
> - ret = obj->filp->f_op->mmap(obj->filp, vma);
> + ret = drm_gem_mmap_obj(obj, obj->size, vma);
> if (ret)
> return ret;
>
> - fput(vma->vm_file);
> - vma->vm_file = get_file(obj->filp);
> - vma->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP;
> - vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
> -
> + vma->vm_flags &= ~(VM_IO | VM_PFNMAP);
> return 0;
> }
>
> --
> 2.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/3] drm/vgem: Keep a reference to the dmabuf when mmaped
2016-10-05 13:11 ` Daniel Vetter
@ 2016-10-05 13:40 ` Chris Wilson
2016-10-05 13:42 ` Daniel Vetter
0 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2016-10-05 13:40 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, dri-devel
On Wed, Oct 05, 2016 at 03:11:00PM +0200, Daniel Vetter wrote:
> On Fri, Sep 30, 2016 at 02:59:59PM +0100, Chris Wilson wrote:
> > In order to keep the dmabuf alive whilst the mmap is, we need to hold a
> > reference to the dmabuf and not the backing object. This is important as
> > the dmabuf not only keeps the object alive, but also the device so that
> >
> > dmabuf = vgem_create_dmabuf();
> > ptr = mmap(... dmabuf ...);
> > close(dmabuf);
> >
> > persists across module-unload as well as device closure.
>
> I don't see where we grab the ref to the dma-buf here instead of the
> backing storage. And doesn't the exact same issue happen when you use dumb
> mmap? Or maybe I'm just a bit confused about what's going on here ...
The reference to the dmabuf is in vma->vm_file, which was used to keep
the module alive. So this might be overzealous, in that there shouldn't
be a way to get back from the shmemfs object to the module. However,
that does make me aware of a failure path we have in patches that do add
callbacks from shmemfs to the driver...
At least using the ptr after closing the module is ok. So this patch is
not required.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/3] drm/vgem: Keep a reference to the dmabuf when mmaped
2016-10-05 13:40 ` Chris Wilson
@ 2016-10-05 13:42 ` Daniel Vetter
0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2016-10-05 13:42 UTC (permalink / raw)
To: Chris Wilson, Daniel Vetter, dri-devel, intel-gfx
On Wed, Oct 05, 2016 at 02:40:09PM +0100, Chris Wilson wrote:
> On Wed, Oct 05, 2016 at 03:11:00PM +0200, Daniel Vetter wrote:
> > On Fri, Sep 30, 2016 at 02:59:59PM +0100, Chris Wilson wrote:
> > > In order to keep the dmabuf alive whilst the mmap is, we need to hold a
> > > reference to the dmabuf and not the backing object. This is important as
> > > the dmabuf not only keeps the object alive, but also the device so that
> > >
> > > dmabuf = vgem_create_dmabuf();
> > > ptr = mmap(... dmabuf ...);
> > > close(dmabuf);
> > >
> > > persists across module-unload as well as device closure.
> >
> > I don't see where we grab the ref to the dma-buf here instead of the
> > backing storage. And doesn't the exact same issue happen when you use dumb
> > mmap? Or maybe I'm just a bit confused about what's going on here ...
>
> The reference to the dmabuf is in vma->vm_file, which was used to keep
> the module alive. So this might be overzealous, in that there shouldn't
> be a way to get back from the shmemfs object to the module. However,
> that does make me aware of a failure path we have in patches that do add
> callbacks from shmemfs to the driver...
>
> At least using the ptr after closing the module is ok. So this patch is
> not required.
I think if we entirely redirect the mmap to shmem, and not forget to also
update vm_file, the driver could get unloaded without ill effects. I'll
leave this one out for now, picked up 1&2 from v4 to -misc meanwhile.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-10-05 13:42 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-30 13:59 [PATCH v3 1/3] drm/prime: Pass the right module owner through to dma_buf_export() Chris Wilson
2016-09-30 13:59 ` [PATCH v3 2/3] drm/prime: Take a ref on the drm_dev when exporting a dma_buf Chris Wilson
2016-09-30 13:59 ` [PATCH v3 3/3] drm/vgem: Keep a reference to the dmabuf when mmaped Chris Wilson
2016-10-05 13:11 ` Daniel Vetter
2016-10-05 13:40 ` Chris Wilson
2016-10-05 13:42 ` Daniel Vetter
2016-09-30 14:22 ` [PATCH v3 1/3] drm/prime: Pass the right module owner through to dma_buf_export() Christian König
2016-09-30 14:22 ` Christian König
2016-09-30 14:49 ` ✗ Fi.CI.BAT: warning for series starting with [v3,1/3] " Patchwork
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.