* DRM-related kmalloc-32 memory leak in 2.6.35
[not found] ` <alpine.DEB.2.00.1008241036250.344@router.home>
@ 2010-08-25 20:59 ` Matt Mackall
2010-09-27 18:52 ` Andrew Morton
0 siblings, 1 reply; 9+ messages in thread
From: Matt Mackall @ 2010-08-25 20:59 UTC (permalink / raw)
To: Christoph Lameter, David Airlie
Cc: David Rientjes, Pekka Enberg, Linux Mailing List
On Tue, 2010-08-24 at 10:37 -0500, Christoph Lameter wrote:
> On Tue, 24 Aug 2010, Matt Mackall wrote:
>
> > kmalloc-32 1113344 1113344 32 128 1 : tunables 0 0
> > 0 : slabdata 8698 8698 0
> >
> > That's /proc/slabinfo on my laptop with SLUB. It looks like my last
> > reboot popped me back to 2.6.33 so it may also be old news, but I
> > couldn't spot any reports with Google.
>
> Boot with "slub_debug" as a kernel parameter
>
> and then do a
>
> cat /sys/kernel/slab/kmalloc-32/alloc_calls
>
> to find the caller allocating the objets.
Still present in 2.6.35. Appears to be DRM:
845 drm_vm_open_locked+0x72/0x109 age=43/37572/59269 pid=2089
cpus=0-1
That's after about a minute of uptime. Grows to 100k in about a day.
dmesg bits:
[ 0.834653] [drm] Initialized drm 1.1.0 20060810
[ 0.834986] pci 0000:00:02.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16
[ 0.834995] pci 0000:00:02.0: setting latency timer to 64
[ 1.002572] mtrr: type mismatch for e0000000,10000000 old: write-back new: write-combining
[ 1.002580] [drm] MTRR allocation failed. Graphics performance may suffer.
[ 1.019880] acpi device:03: registered as cooling_device2
[ 1.021520] input: Video Bus as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/LNXVIDEO:00/input/input3
[ 1.021543] ACPI: Video Device [GFX0] (multi-head: yes rom: no post: no)
[ 1.021855] [drm] Initialized i915 1.6.0 20080730 for 0000:00:02.0 on minor 0
This is with:
00:02.0 VGA compatible controller: Intel Corporation Mobile GM965/GL960
Integrated Graphics Controller (rev 03)
00:02.1 Display controller: Intel Corporation Mobile GM965/GL960
Integrated Graphics Controller (rev 03)
--
Mathematics is the supreme nostalgia of our time.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: DRM-related kmalloc-32 memory leak in 2.6.35
2010-08-25 20:59 ` DRM-related kmalloc-32 memory leak in 2.6.35 Matt Mackall
@ 2010-09-27 18:52 ` Andrew Morton
2010-09-27 20:08 ` [PATCH] drm: Prune GEM vma entries Chris Wilson
2010-09-27 20:40 ` DRM-related kmalloc-32 memory leak in 2.6.35 David Rientjes
0 siblings, 2 replies; 9+ messages in thread
From: Andrew Morton @ 2010-09-27 18:52 UTC (permalink / raw)
To: Matt Mackall
Cc: Christoph Lameter, David Airlie, David Rientjes, Pekka Enberg,
Linux Mailing List, dri-devel
On Wed, 25 Aug 2010 15:59:09 -0500
Matt Mackall <mpm@selenic.com> wrote:
> On Tue, 2010-08-24 at 10:37 -0500, Christoph Lameter wrote:
> > On Tue, 24 Aug 2010, Matt Mackall wrote:
> >
> > > kmalloc-32 1113344 1113344 32 128 1 : tunables 0 0
> > > 0 : slabdata 8698 8698 0
> > >
> > > That's /proc/slabinfo on my laptop with SLUB. It looks like my last
> > > reboot popped me back to 2.6.33 so it may also be old news, but I
> > > couldn't spot any reports with Google.
> >
> > Boot with "slub_debug" as a kernel parameter
> >
> > and then do a
> >
> > cat /sys/kernel/slab/kmalloc-32/alloc_calls
> >
> > to find the caller allocating the objets.
>
> Still present in 2.6.35. Appears to be DRM:
>
> 845 drm_vm_open_locked+0x72/0x109 age=43/37572/59269 pid=2089
> cpus=0-1
>
> That's after about a minute of uptime. Grows to 100k in about a day.
>
> dmesg bits:
> [ 0.834653] [drm] Initialized drm 1.1.0 20060810
> [ 0.834986] pci 0000:00:02.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16
> [ 0.834995] pci 0000:00:02.0: setting latency timer to 64
> [ 1.002572] mtrr: type mismatch for e0000000,10000000 old: write-back new: write-combining
> [ 1.002580] [drm] MTRR allocation failed. Graphics performance may suffer.
> [ 1.019880] acpi device:03: registered as cooling_device2
> [ 1.021520] input: Video Bus as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/LNXVIDEO:00/input/input3
> [ 1.021543] ACPI: Video Device [GFX0] (multi-head: yes rom: no post: no)
> [ 1.021855] [drm] Initialized i915 1.6.0 20080730 for 0000:00:02.0 on minor 0
>
> This is with:
>
> 00:02.0 VGA compatible controller: Intel Corporation Mobile GM965/GL960
> Integrated Graphics Controller (rev 03)
> 00:02.1 Display controller: Intel Corporation Mobile GM965/GL960
> Integrated Graphics Controller (rev 03)
>
Matt tells me that this (drop-dead box-killing!) bug is still present
in current kernels. Could someone take a look please?
The code seems very simple, and I couldn't spot a problem. Probably
this means that I'm even simpler.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] drm: Prune GEM vma entries
2010-09-27 18:52 ` Andrew Morton
@ 2010-09-27 20:08 ` Chris Wilson
2010-09-27 20:08 ` Andrew Morton
` (2 more replies)
2010-09-27 20:40 ` DRM-related kmalloc-32 memory leak in 2.6.35 David Rientjes
1 sibling, 3 replies; 9+ messages in thread
From: Chris Wilson @ 2010-09-27 20:08 UTC (permalink / raw)
To: Matt Mackall, Andrew Morton
Cc: dri-devel, linux-kernel, Chris Wilson, Dave Airlie, Jesse Barnes
Hook the GEM vm open/close ops into the generic drm vm open/close so
that the vma entries are created and destroy appropriately.
Reported-by: Matt Mackall <mpm@selenic.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/drm_gem.c | 9 ++++++++-
drivers/gpu/drm/drm_vm.c | 28 ++++++++++++++++++----------
include/drm/drmP.h | 1 +
3 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index bf92d07..6fe2cd2 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -528,6 +528,10 @@ void drm_gem_vm_open(struct vm_area_struct *vma)
struct drm_gem_object *obj = vma->vm_private_data;
drm_gem_object_reference(obj);
+
+ mutex_lock(&obj->dev->struct_mutex);
+ drm_vm_open_locked(vma);
+ mutex_unlock(&obj->dev->struct_mutex);
}
EXPORT_SYMBOL(drm_gem_vm_open);
@@ -535,7 +539,10 @@ void drm_gem_vm_close(struct vm_area_struct *vma)
{
struct drm_gem_object *obj = vma->vm_private_data;
- drm_gem_object_unreference_unlocked(obj);
+ mutex_lock(&obj->dev->struct_mutex);
+ drm_vm_close_locked(vma);
+ drm_gem_object_unreference(obj);
+ mutex_unlock(&obj->dev->struct_mutex);
}
EXPORT_SYMBOL(drm_gem_vm_close);
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index fda6746..5df4506 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -433,15 +433,7 @@ static void drm_vm_open(struct vm_area_struct *vma)
mutex_unlock(&dev->struct_mutex);
}
-/**
- * \c close method for all virtual memory types.
- *
- * \param vma virtual memory area.
- *
- * Search the \p vma private data entry in drm_device::vmalist, unlink it, and
- * free it.
- */
-static void drm_vm_close(struct vm_area_struct *vma)
+void drm_vm_close_locked(struct vm_area_struct *vma)
{
struct drm_file *priv = vma->vm_file->private_data;
struct drm_device *dev = priv->minor->dev;
@@ -451,7 +443,6 @@ static void drm_vm_close(struct vm_area_struct *vma)
vma->vm_start, vma->vm_end - vma->vm_start);
atomic_dec(&dev->vma_count);
- mutex_lock(&dev->struct_mutex);
list_for_each_entry_safe(pt, temp, &dev->vmalist, head) {
if (pt->vma == vma) {
list_del(&pt->head);
@@ -459,6 +450,23 @@ static void drm_vm_close(struct vm_area_struct *vma)
break;
}
}
+}
+
+/**
+ * \c close method for all virtual memory types.
+ *
+ * \param vma virtual memory area.
+ *
+ * Search the \p vma private data entry in drm_device::vmalist, unlink it, and
+ * free it.
+ */
+static void drm_vm_close(struct vm_area_struct *vma)
+{
+ struct drm_file *priv = vma->vm_file->private_data;
+ struct drm_device *dev = priv->minor->dev;
+
+ mutex_lock(&dev->struct_mutex);
+ drm_vm_close_locked(vma);
mutex_unlock(&dev->struct_mutex);
}
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 7809d23..774e1d4 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1175,6 +1175,7 @@ extern int drm_release(struct inode *inode, struct file *filp);
extern int drm_mmap(struct file *filp, struct vm_area_struct *vma);
extern int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma);
extern void drm_vm_open_locked(struct vm_area_struct *vma);
+extern void drm_vm_close_locked(struct vm_area_struct *vma);
extern resource_size_t drm_core_get_map_ofs(struct drm_local_map * map);
extern resource_size_t drm_core_get_reg_ofs(struct drm_device *dev);
extern unsigned int drm_poll(struct file *filp, struct poll_table_struct *wait);
--
1.7.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] drm: Prune GEM vma entries
2010-09-27 20:08 ` [PATCH] drm: Prune GEM vma entries Chris Wilson
@ 2010-09-27 20:08 ` Andrew Morton
2010-09-27 20:28 ` Chris Wilson
2010-10-02 0:09 ` Matt Mackall
2010-10-04 7:07 ` Paul Rolland
2 siblings, 1 reply; 9+ messages in thread
From: Andrew Morton @ 2010-09-27 20:08 UTC (permalink / raw)
To: Chris Wilson
Cc: Matt Mackall, dri-devel, linux-kernel, Dave Airlie, Jesse Barnes
That was quick, thanks.
On Mon, 27 Sep 2010 21:08:36 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Hook the GEM vm open/close ops into the generic drm vm open/close so
> that the vma entries are created and destroy appropriately.
Please update the changelog to indicate that it fixes a memory leak.
> Reported-by: Matt Mackall <mpm@selenic.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
And here please add
Cc: <stable@kernel.org>
so that earlier kernels get reliably fixed.
Thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] drm: Prune GEM vma entries
2010-09-27 20:08 ` Andrew Morton
@ 2010-09-27 20:28 ` Chris Wilson
0 siblings, 0 replies; 9+ messages in thread
From: Chris Wilson @ 2010-09-27 20:28 UTC (permalink / raw)
To: Matt Mackall, Andrew Morton
Cc: dri-devel, linux-kernel, Chris Wilson, Dave Airlie, Jesse Barnes,
stable
Hook the GEM vm open/close ops into the generic drm vm open/close so
that the private vma entries are created and destroy appropriately.
Fixes the leak of the drm_vma_entries during the lifetime of the filp.
Reported-by: Matt Mackall <mpm@selenic.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: stable@kernel.org
---
drivers/gpu/drm/drm_gem.c | 9 ++++++++-
drivers/gpu/drm/drm_vm.c | 28 ++++++++++++++++++----------
include/drm/drmP.h | 1 +
3 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index bf92d07..6fe2cd2 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -528,6 +528,10 @@ void drm_gem_vm_open(struct vm_area_struct *vma)
struct drm_gem_object *obj = vma->vm_private_data;
drm_gem_object_reference(obj);
+
+ mutex_lock(&obj->dev->struct_mutex);
+ drm_vm_open_locked(vma);
+ mutex_unlock(&obj->dev->struct_mutex);
}
EXPORT_SYMBOL(drm_gem_vm_open);
@@ -535,7 +539,10 @@ void drm_gem_vm_close(struct vm_area_struct *vma)
{
struct drm_gem_object *obj = vma->vm_private_data;
- drm_gem_object_unreference_unlocked(obj);
+ mutex_lock(&obj->dev->struct_mutex);
+ drm_vm_close_locked(vma);
+ drm_gem_object_unreference(obj);
+ mutex_unlock(&obj->dev->struct_mutex);
}
EXPORT_SYMBOL(drm_gem_vm_close);
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index fda6746..5df4506 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -433,15 +433,7 @@ static void drm_vm_open(struct vm_area_struct *vma)
mutex_unlock(&dev->struct_mutex);
}
-/**
- * \c close method for all virtual memory types.
- *
- * \param vma virtual memory area.
- *
- * Search the \p vma private data entry in drm_device::vmalist, unlink it, and
- * free it.
- */
-static void drm_vm_close(struct vm_area_struct *vma)
+void drm_vm_close_locked(struct vm_area_struct *vma)
{
struct drm_file *priv = vma->vm_file->private_data;
struct drm_device *dev = priv->minor->dev;
@@ -451,7 +443,6 @@ static void drm_vm_close(struct vm_area_struct *vma)
vma->vm_start, vma->vm_end - vma->vm_start);
atomic_dec(&dev->vma_count);
- mutex_lock(&dev->struct_mutex);
list_for_each_entry_safe(pt, temp, &dev->vmalist, head) {
if (pt->vma == vma) {
list_del(&pt->head);
@@ -459,6 +450,23 @@ static void drm_vm_close(struct vm_area_struct *vma)
break;
}
}
+}
+
+/**
+ * \c close method for all virtual memory types.
+ *
+ * \param vma virtual memory area.
+ *
+ * Search the \p vma private data entry in drm_device::vmalist, unlink it, and
+ * free it.
+ */
+static void drm_vm_close(struct vm_area_struct *vma)
+{
+ struct drm_file *priv = vma->vm_file->private_data;
+ struct drm_device *dev = priv->minor->dev;
+
+ mutex_lock(&dev->struct_mutex);
+ drm_vm_close_locked(vma);
mutex_unlock(&dev->struct_mutex);
}
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 7809d23..774e1d4 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1175,6 +1175,7 @@ extern int drm_release(struct inode *inode, struct file *filp);
extern int drm_mmap(struct file *filp, struct vm_area_struct *vma);
extern int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma);
extern void drm_vm_open_locked(struct vm_area_struct *vma);
+extern void drm_vm_close_locked(struct vm_area_struct *vma);
extern resource_size_t drm_core_get_map_ofs(struct drm_local_map * map);
extern resource_size_t drm_core_get_reg_ofs(struct drm_device *dev);
extern unsigned int drm_poll(struct file *filp, struct poll_table_struct *wait);
--
1.7.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: DRM-related kmalloc-32 memory leak in 2.6.35
2010-09-27 18:52 ` Andrew Morton
2010-09-27 20:08 ` [PATCH] drm: Prune GEM vma entries Chris Wilson
@ 2010-09-27 20:40 ` David Rientjes
1 sibling, 0 replies; 9+ messages in thread
From: David Rientjes @ 2010-09-27 20:40 UTC (permalink / raw)
To: Andrew Morton
Cc: Matt Mackall, Christoph Lameter, David Airlie, Pekka Enberg,
Linux Mailing List, dri-devel
On Mon, 27 Sep 2010, Andrew Morton wrote:
> > Still present in 2.6.35. Appears to be DRM:
> >
> > 845 drm_vm_open_locked+0x72/0x109 age=43/37572/59269 pid=2089
> > cpus=0-1
> >
> > That's after about a minute of uptime. Grows to 100k in about a day.
> >
> > dmesg bits:
> > [ 0.834653] [drm] Initialized drm 1.1.0 20060810
> > [ 0.834986] pci 0000:00:02.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16
> > [ 0.834995] pci 0000:00:02.0: setting latency timer to 64
> > [ 1.002572] mtrr: type mismatch for e0000000,10000000 old: write-back new: write-combining
> > [ 1.002580] [drm] MTRR allocation failed. Graphics performance may suffer.
> > [ 1.019880] acpi device:03: registered as cooling_device2
> > [ 1.021520] input: Video Bus as /devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/LNXVIDEO:00/input/input3
> > [ 1.021543] ACPI: Video Device [GFX0] (multi-head: yes rom: no post: no)
> > [ 1.021855] [drm] Initialized i915 1.6.0 20080730 for 0000:00:02.0 on minor 0
> >
> > This is with:
> >
> > 00:02.0 VGA compatible controller: Intel Corporation Mobile GM965/GL960
> > Integrated Graphics Controller (rev 03)
> > 00:02.1 Display controller: Intel Corporation Mobile GM965/GL960
> > Integrated Graphics Controller (rev 03)
> >
>
> Matt tells me that this (drop-dead box-killing!) bug is still present
> in current kernels. Could someone take a look please?
>
> The code seems very simple, and I couldn't spot a problem. Probably
> this means that I'm even simpler.
>
I'm wondering what reading /sys/kernel/debug/dri/.../vma says when
DRM_DEBUG_CODE is enabled, it seems to have been written for this type of
debugging.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm: Prune GEM vma entries
2010-09-27 20:08 ` [PATCH] drm: Prune GEM vma entries Chris Wilson
2010-09-27 20:08 ` Andrew Morton
@ 2010-10-02 0:09 ` Matt Mackall
2010-10-03 18:59 ` Paul Rolland
2010-10-04 7:07 ` Paul Rolland
2 siblings, 1 reply; 9+ messages in thread
From: Matt Mackall @ 2010-10-02 0:09 UTC (permalink / raw)
To: Chris Wilson, Paul Rolland
Cc: Andrew Morton, dri-devel, linux-kernel, Dave Airlie, Jesse Barnes
On Mon, 2010-09-27 at 21:08 +0100, Chris Wilson wrote:
> Hook the GEM vm open/close ops into the generic drm vm open/close so
> that the vma entries are created and destroy appropriately.
>
> Reported-by: Matt Mackall <mpm@selenic.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
All signs point to this being the correct fix, but I won't have time to
test it while I'm in Japan.
Paul, does this work for you?
> ---
> drivers/gpu/drm/drm_gem.c | 9 ++++++++-
> drivers/gpu/drm/drm_vm.c | 28 ++++++++++++++++++----------
> include/drm/drmP.h | 1 +
> 3 files changed, 27 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index bf92d07..6fe2cd2 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -528,6 +528,10 @@ void drm_gem_vm_open(struct vm_area_struct *vma)
> struct drm_gem_object *obj = vma->vm_private_data;
>
> drm_gem_object_reference(obj);
> +
> + mutex_lock(&obj->dev->struct_mutex);
> + drm_vm_open_locked(vma);
> + mutex_unlock(&obj->dev->struct_mutex);
> }
> EXPORT_SYMBOL(drm_gem_vm_open);
>
> @@ -535,7 +539,10 @@ void drm_gem_vm_close(struct vm_area_struct *vma)
> {
> struct drm_gem_object *obj = vma->vm_private_data;
>
> - drm_gem_object_unreference_unlocked(obj);
> + mutex_lock(&obj->dev->struct_mutex);
> + drm_vm_close_locked(vma);
> + drm_gem_object_unreference(obj);
> + mutex_unlock(&obj->dev->struct_mutex);
> }
> EXPORT_SYMBOL(drm_gem_vm_close);
>
> diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
> index fda6746..5df4506 100644
> --- a/drivers/gpu/drm/drm_vm.c
> +++ b/drivers/gpu/drm/drm_vm.c
> @@ -433,15 +433,7 @@ static void drm_vm_open(struct vm_area_struct *vma)
> mutex_unlock(&dev->struct_mutex);
> }
>
> -/**
> - * \c close method for all virtual memory types.
> - *
> - * \param vma virtual memory area.
> - *
> - * Search the \p vma private data entry in drm_device::vmalist, unlink it, and
> - * free it.
> - */
> -static void drm_vm_close(struct vm_area_struct *vma)
> +void drm_vm_close_locked(struct vm_area_struct *vma)
> {
> struct drm_file *priv = vma->vm_file->private_data;
> struct drm_device *dev = priv->minor->dev;
> @@ -451,7 +443,6 @@ static void drm_vm_close(struct vm_area_struct *vma)
> vma->vm_start, vma->vm_end - vma->vm_start);
> atomic_dec(&dev->vma_count);
>
> - mutex_lock(&dev->struct_mutex);
> list_for_each_entry_safe(pt, temp, &dev->vmalist, head) {
> if (pt->vma == vma) {
> list_del(&pt->head);
> @@ -459,6 +450,23 @@ static void drm_vm_close(struct vm_area_struct *vma)
> break;
> }
> }
> +}
> +
> +/**
> + * \c close method for all virtual memory types.
> + *
> + * \param vma virtual memory area.
> + *
> + * Search the \p vma private data entry in drm_device::vmalist, unlink it, and
> + * free it.
> + */
> +static void drm_vm_close(struct vm_area_struct *vma)
> +{
> + struct drm_file *priv = vma->vm_file->private_data;
> + struct drm_device *dev = priv->minor->dev;
> +
> + mutex_lock(&dev->struct_mutex);
> + drm_vm_close_locked(vma);
> mutex_unlock(&dev->struct_mutex);
> }
>
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 7809d23..774e1d4 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1175,6 +1175,7 @@ extern int drm_release(struct inode *inode, struct file *filp);
> extern int drm_mmap(struct file *filp, struct vm_area_struct *vma);
> extern int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma);
> extern void drm_vm_open_locked(struct vm_area_struct *vma);
> +extern void drm_vm_close_locked(struct vm_area_struct *vma);
> extern resource_size_t drm_core_get_map_ofs(struct drm_local_map * map);
> extern resource_size_t drm_core_get_reg_ofs(struct drm_device *dev);
> extern unsigned int drm_poll(struct file *filp, struct poll_table_struct *wait);
--
Mathematics is the supreme nostalgia of our time.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm: Prune GEM vma entries
2010-10-02 0:09 ` Matt Mackall
@ 2010-10-03 18:59 ` Paul Rolland
0 siblings, 0 replies; 9+ messages in thread
From: Paul Rolland @ 2010-10-03 18:59 UTC (permalink / raw)
To: Matt Mackall
Cc: Chris Wilson, Andrew Morton, dri-devel, linux-kernel, Dave Airlie,
Jesse Barnes
Hello,
On Fri, 01 Oct 2010 19:09:56 -0500
Matt Mackall <mpm@selenic.com> wrote:
> On Mon, 2010-09-27 at 21:08 +0100, Chris Wilson wrote:
> > Hook the GEM vm open/close ops into the generic drm vm open/close so
> > that the vma entries are created and destroy appropriately.
> >
> > Reported-by: Matt Mackall <mpm@selenic.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> All signs point to this being the correct fix, but I won't have time to
> test it while I'm in Japan.
>
> Paul, does this work for you?
I've just finished building a kernel with this patch applied.
I now need to keep it running for at least one day to make sure that the
leak is gone for good.
I'll keep you updated on that.
Regards,
Paul
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm: Prune GEM vma entries
2010-09-27 20:08 ` [PATCH] drm: Prune GEM vma entries Chris Wilson
2010-09-27 20:08 ` Andrew Morton
2010-10-02 0:09 ` Matt Mackall
@ 2010-10-04 7:07 ` Paul Rolland
2 siblings, 0 replies; 9+ messages in thread
From: Paul Rolland @ 2010-10-04 7:07 UTC (permalink / raw)
To: Chris Wilson
Cc: Matt Mackall, Andrew Morton, dri-devel, linux-kernel, Dave Airlie,
Jesse Barnes
Hello,
You can add a:
Tested-by: Paul Rolland <rol@as2917.net>
This is indeed fixing the memory leak in size-32 pool on my machine.
Thanks a lot,
Paul
On Mon, 27 Sep 2010 21:08:36 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Hook the GEM vm open/close ops into the generic drm vm open/close so
> that the vma entries are created and destroy appropriately.
>
> Reported-by: Matt Mackall <mpm@selenic.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/drm_gem.c | 9 ++++++++-
> drivers/gpu/drm/drm_vm.c | 28 ++++++++++++++++++----------
> include/drm/drmP.h | 1 +
> 3 files changed, 27 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index bf92d07..6fe2cd2 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -528,6 +528,10 @@ void drm_gem_vm_open(struct vm_area_struct *vma)
> struct drm_gem_object *obj = vma->vm_private_data;
>
> drm_gem_object_reference(obj);
> +
> + mutex_lock(&obj->dev->struct_mutex);
> + drm_vm_open_locked(vma);
> + mutex_unlock(&obj->dev->struct_mutex);
> }
> EXPORT_SYMBOL(drm_gem_vm_open);
>
> @@ -535,7 +539,10 @@ void drm_gem_vm_close(struct vm_area_struct *vma)
> {
> struct drm_gem_object *obj = vma->vm_private_data;
>
> - drm_gem_object_unreference_unlocked(obj);
> + mutex_lock(&obj->dev->struct_mutex);
> + drm_vm_close_locked(vma);
> + drm_gem_object_unreference(obj);
> + mutex_unlock(&obj->dev->struct_mutex);
> }
> EXPORT_SYMBOL(drm_gem_vm_close);
>
> diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
> index fda6746..5df4506 100644
> --- a/drivers/gpu/drm/drm_vm.c
> +++ b/drivers/gpu/drm/drm_vm.c
> @@ -433,15 +433,7 @@ static void drm_vm_open(struct vm_area_struct *vma)
> mutex_unlock(&dev->struct_mutex);
> }
>
> -/**
> - * \c close method for all virtual memory types.
> - *
> - * \param vma virtual memory area.
> - *
> - * Search the \p vma private data entry in drm_device::vmalist, unlink it, and
> - * free it.
> - */
> -static void drm_vm_close(struct vm_area_struct *vma)
> +void drm_vm_close_locked(struct vm_area_struct *vma)
> {
> struct drm_file *priv = vma->vm_file->private_data;
> struct drm_device *dev = priv->minor->dev;
> @@ -451,7 +443,6 @@ static void drm_vm_close(struct vm_area_struct *vma)
> vma->vm_start, vma->vm_end - vma->vm_start);
> atomic_dec(&dev->vma_count);
>
> - mutex_lock(&dev->struct_mutex);
> list_for_each_entry_safe(pt, temp, &dev->vmalist, head) {
> if (pt->vma == vma) {
> list_del(&pt->head);
> @@ -459,6 +450,23 @@ static void drm_vm_close(struct vm_area_struct *vma)
> break;
> }
> }
> +}
> +
> +/**
> + * \c close method for all virtual memory types.
> + *
> + * \param vma virtual memory area.
> + *
> + * Search the \p vma private data entry in drm_device::vmalist, unlink it, and
> + * free it.
> + */
> +static void drm_vm_close(struct vm_area_struct *vma)
> +{
> + struct drm_file *priv = vma->vm_file->private_data;
> + struct drm_device *dev = priv->minor->dev;
> +
> + mutex_lock(&dev->struct_mutex);
> + drm_vm_close_locked(vma);
> mutex_unlock(&dev->struct_mutex);
> }
>
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 7809d23..774e1d4 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1175,6 +1175,7 @@ extern int drm_release(struct inode *inode, struct file *filp);
> extern int drm_mmap(struct file *filp, struct vm_area_struct *vma);
> extern int drm_mmap_locked(struct file *filp, struct vm_area_struct *vma);
> extern void drm_vm_open_locked(struct vm_area_struct *vma);
> +extern void drm_vm_close_locked(struct vm_area_struct *vma);
> extern resource_size_t drm_core_get_map_ofs(struct drm_local_map * map);
> extern resource_size_t drm_core_get_reg_ofs(struct drm_device *dev);
> extern unsigned int drm_poll(struct file *filp, struct poll_table_struct *wait);
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-10-04 7:08 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <alpine.DEB.2.00.1008221615350.29062@chino.kir.corp.google.com>
[not found] ` <1282623994.10679.921.camel@calx>
[not found] ` <alpine.DEB.2.00.1008232134480.25742@chino.kir.corp.google.com>
[not found] ` <1282663241.10679.958.camel@calx>
[not found] ` <alpine.DEB.2.00.1008241036250.344@router.home>
2010-08-25 20:59 ` DRM-related kmalloc-32 memory leak in 2.6.35 Matt Mackall
2010-09-27 18:52 ` Andrew Morton
2010-09-27 20:08 ` [PATCH] drm: Prune GEM vma entries Chris Wilson
2010-09-27 20:08 ` Andrew Morton
2010-09-27 20:28 ` Chris Wilson
2010-10-02 0:09 ` Matt Mackall
2010-10-03 18:59 ` Paul Rolland
2010-10-04 7:07 ` Paul Rolland
2010-09-27 20:40 ` DRM-related kmalloc-32 memory leak in 2.6.35 David Rientjes
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).