[Intel-gfx] [PATCH 1/2] drm: Track drm_mm allocators and show leaks on shutdown

[Christian König]

Am 29.10.2016 um 20:42 schrieb Chris Wilson:
> We can use the kernel's stack tracer and depot to record the allocation
> site of every drm_mm user and then on shutdown as well as warning that
> allocated nodes still reside with the drm_mm range manager, we can also
> display who allocated them to aide tracking down the leak.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

Thanks, exactly what I need to debug the leaks in amdgpu as well.

Patch is Reviewed-by: Christian König <christian.koenig at amd.com>.

Regards,
Christian.

> ---
>   drivers/gpu/drm/Kconfig  | 12 ++++++++
>   drivers/gpu/drm/drm_mm.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++--
>   include/drm/drm_mm.h     |  6 ++++
>   3 files changed, 89 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 483059a22b1b..e339ac9ca8cf 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -300,3 +300,15 @@ config DRM_SAVAGE
>   	  chipset. If M is selected the module will be called savage.
>   
>   endif # DRM_LEGACY
> +
> +config DRM_DEBUG_MM
> +	bool "Insert extra checks for tracking the memory manager"
> +	default n
> +	depends on DRM
> +	select STACKDEPOT
> +	help
> +	  Enable tracking of memory manager and leak detection.
> +
> +	  Recommended for driver developers only.
> +
> +	  If in doubt, say "N".
> diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
> index 11d44a1e0ab3..f7d25c02297d 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);
>   
> +#if 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 {