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=2089cpus=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.
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);
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
so that earlier kernels get reliably fixed.
Thanks.
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);
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);
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
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);
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=2089cpus=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.
dri-devel@lists.freedesktop.org
-
Andrew Morton -
Chris Wilson -
David Rientjes -
Matt Mackall -
Paul Rolland