[PATCH v2] drm: Track drm_mm allocators and show leaks on shutdown

[Daniel Vetter]

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