On Mon, Oct 31, 2016 at 09:08:06AM +0000, Chris Wilson wrote:
> We can use the kernel's stack tracer and depot to record the allocation
> site of every drm_mm user. Then on shutdown, as well as warning that
> allocated nodes still reside with the drm_mm range manager, we can
> display who allocated them to aide tracking down the leak.
>
> v2: Move Kconfig around so it lies underneath the DRM options submenu.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Reviewed-by: Christian König <christian.koenig at amd.com>
Applied to drm-misc, thanks.
-Daniel
> ---
> drivers/gpu/drm/Kconfig | 13 +++++++++
> drivers/gpu/drm/drm_mm.c | 74
> ++++++++++++++++++++++++++++++++++++++++++++++--
> include/drm/drm_mm.h | 6 ++++
> 3 files changed, 90 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 483059a22b1b..25e8e3793d29 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -33,6 +33,19 @@ config DRM_DP_AUX_CHARDEV
> read and write values to arbitrary DPCD registers on the DP aux
> channel.
>
> +config DRM_DEBUG_MM
> + bool "Insert extra checks and debug info into the DRM range managers"
> + default n
> + depends on DRM
> + select STACKDEPOT
> + help
> + Enable allocation tracking of memory manager and leak detection on
> + shutdown.
> +
> + Recommended for driver developers only.
> +
> + If in doubt, say "N".
> +
> config DRM_KMS_HELPER
> tristate
> depends on DRM
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index 11d44a1e0ab3..89b891a4847f 100644
> --- a/drivers/gpu/drm/drm_mm.c
> +++ b/drivers/gpu/drm/drm_mm.c
> @@ -104,6 +104,66 @@ static struct drm_mm_node
> *drm_mm_search_free_in_range_generic(const struct drm_
> u64 end,
> enum drm_mm_search_flags flags);
>
> +#ifdef CONFIG_DRM_DEBUG_MM
> +#define STACKDEPTH 32
> +#define BUFSZ 4096
> +
> +static noinline void save_stack(struct drm_mm_node *node)
> +{
> + unsigned long entries[STACKDEPTH];
> + struct stack_trace trace = {
> + .entries = entries,
> + .max_entries = STACKDEPTH,
> + .skip = 1
> + };
> +
> + save_stack_trace(&trace);
> + if (trace.nr_entries != 0 &&
> + trace.entries[trace.nr_entries-1] == ULONG_MAX)
> + trace.nr_entries--;
> +
> + /* May be called under spinlock, so avoid sleeping */
> + node->stack = depot_save_stack(&trace, GFP_NOWAIT);
> +}
> +
> +static void show_leaks(struct drm_mm *mm)
> +{
> + struct drm_mm_node *node;
> + unsigned long entries[STACKDEPTH];
> + char *buf;
> +
> + buf = kmalloc(BUFSZ, GFP_KERNEL);
> + if (!buf)
> + return;
> +
> + list_for_each_entry(node, &mm->head_node.node_list, node_list) {
> + struct stack_trace trace = {
> + .entries = entries,
> + .max_entries = STACKDEPTH
> + };
> +
> + if (!node->stack) {
> + DRM_ERROR("node [%08llx + %08llx]: unknown owner\n",
> + node->start, node->size);
> + continue;
> + }
> +
> + depot_fetch_stack(node->stack, &trace);
> + snprint_stack_trace(buf, BUFSZ, &trace, 0);
> + DRM_ERROR("node [%08llx + %08llx]: inserted at\n%s",
> + node->start, node->size, buf);
> + }
> +
> + kfree(buf);
> +}
> +
> +#undef STACKDEPTH
> +#undef BUFSZ
> +#else
> +static void save_stack(struct drm_mm_node *node) { }
> +static void show_leaks(struct drm_mm *mm) { }
> +#endif
> +
> #define START(node) ((node)->start)
> #define LAST(node) ((node)->start + (node)->size - 1)
>
> @@ -228,6 +288,8 @@ static void drm_mm_insert_helper(struct drm_mm_node
> *hole_node,
> list_add(&node->hole_stack, &mm->hole_stack);
> node->hole_follows = 1;
> }
> +
> + save_stack(node);
> }
>
> /**
> @@ -293,6 +355,8 @@ int drm_mm_reserve_node(struct drm_mm *mm, struct
> drm_mm_node *node)
> node->hole_follows = 1;
> }
>
> + save_stack(node);
> +
> return 0;
> }
> EXPORT_SYMBOL(drm_mm_reserve_node);
> @@ -397,6 +461,8 @@ static void drm_mm_insert_helper_range(struct drm_mm_node
> *hole_node,
> list_add(&node->hole_stack, &mm->hole_stack);
> node->hole_follows = 1;
> }
> +
> + save_stack(node);
> }
>
> /**
> @@ -861,10 +927,12 @@ EXPORT_SYMBOL(drm_mm_init);
> * Note that it is a bug to call this function on an allocator which is not
> * clean.
> */
> -void drm_mm_takedown(struct drm_mm * mm)
> +void drm_mm_takedown(struct drm_mm *mm)
> {
> - WARN(!list_empty(&mm->head_node.node_list),
> - "Memory manager not clean during takedown.\n");
> + if (WARN(!list_empty(&mm->head_node.node_list),
> + "Memory manager not clean during takedown.\n"))
> + show_leaks(mm);
> +
> }
> EXPORT_SYMBOL(drm_mm_takedown);
>
> diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h
> index 205ddcf6d55d..41ddafe92b2f 100644
> --- a/include/drm/drm_mm.h
> +++ b/include/drm/drm_mm.h
> @@ -44,6 +44,9 @@
> #ifdef CONFIG_DEBUG_FS
> #include <linux/seq_file.h>
> #endif
> +#ifdef CONFIG_DRM_DEBUG_MM
> +#include <linux/stackdepot.h>
> +#endif
>
> enum drm_mm_search_flags {
> DRM_MM_SEARCH_DEFAULT = 0,
> @@ -74,6 +77,9 @@ struct drm_mm_node {
> u64 size;
> u64 __subtree_last;
> struct drm_mm *mm;
> +#ifdef CONFIG_DRM_DEBUG_MM
> + depot_stack_handle_t stack;
> +#endif
> };
>
> struct drm_mm {
> --
> 2.10.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
