Re: [PATCH] drm/radeon: fix ttm debugfs for multiple devices

["Christian König" <deathsimple@vodafone.de>]

Am 24.12.2013 05:58, schrieb Ilija Hadzic:
> debugfs files created in radeon_ttm_debugfs_init
> are broken when there are multiple devices in the system.
> Namely, static declaration of radeon_mem_types_list
> causes the function to overwrite the values when the
> executed for subsequent devices. Consequently, the debugfs
> access functions can get .data field that belongs to wrong
> device.
>
> This patch fixes the problem by moving the mem_types
> list into the radeon_device structure instead of using
> static declarations.
>
> Signed-off-by: Ilija Hadzic <ihadzic@research.bell-labs.com>

That fix is already queued for 3.14 by removing dynamically creating the 
debugfs file definition all together. See 
http://lists.freedesktop.org/archives/dri-devel/2013-December/050478.html.

Christian.

> ---
>   drivers/gpu/drm/radeon/radeon.h     |  6 ++++++
>   drivers/gpu/drm/radeon/radeon_ttm.c | 43 +++++++++++++++++--------------------
>   2 files changed, 26 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index b1f990d..bcb173a 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -2098,6 +2098,10 @@ struct radeon_atcs {
>   typedef uint32_t (*radeon_rreg_t)(struct radeon_device*, uint32_t);
>   typedef void (*radeon_wreg_t)(struct radeon_device*, uint32_t, uint32_t);
>   
> +#define RADEON_DEBUGFS_MEM_TYPES 2
> +#define RADEON_TTM_DEBUGFS_MEM_TYPES 2
> +#define RADEON_DEBUGFS_TOTAL_MEM_TYPES (RADEON_DEBUGFS_MEM_TYPES + RADEON_TTM_DEBUGFS_MEM_TYPES)
> +
>   struct radeon_device {
>   	struct device			*dev;
>   	struct drm_device		*ddev;
> @@ -2213,6 +2217,8 @@ struct radeon_device {
>   	/* debugfs */
>   	struct radeon_debugfs	debugfs[RADEON_DEBUGFS_MAX_COMPONENTS];
>   	unsigned 		debugfs_count;
> +	struct drm_info_list mem_types_list[RADEON_DEBUGFS_TOTAL_MEM_TYPES];
> +	char mem_types_names[RADEON_DEBUGFS_TOTAL_MEM_TYPES][32];
>   	/* virtual memory */
>   	struct radeon_vm_manager	vm_manager;
>   	struct mutex			gpu_clock_mutex;
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 051fa87..0de413b 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -832,9 +832,6 @@ int radeon_mmap(struct file *filp, struct vm_area_struct *vma)
>   	return 0;
>   }
>   
> -
> -#define RADEON_DEBUGFS_MEM_TYPES 2
> -
>   #if defined(CONFIG_DEBUG_FS)
>   static int radeon_mm_dump_table(struct seq_file *m, void *data)
>   {
> @@ -855,40 +852,40 @@ static int radeon_mm_dump_table(struct seq_file *m, void *data)
>   static int radeon_ttm_debugfs_init(struct radeon_device *rdev)
>   {
>   #if defined(CONFIG_DEBUG_FS)
> -	static struct drm_info_list radeon_mem_types_list[RADEON_DEBUGFS_MEM_TYPES+2];
> -	static char radeon_mem_types_names[RADEON_DEBUGFS_MEM_TYPES+2][32];
>   	unsigned i;
>   
>   	for (i = 0; i < RADEON_DEBUGFS_MEM_TYPES; i++) {
>   		if (i == 0)
> -			sprintf(radeon_mem_types_names[i], "radeon_vram_mm");
> +			sprintf(rdev->mem_types_names[i], "radeon_vram_mm");
>   		else
> -			sprintf(radeon_mem_types_names[i], "radeon_gtt_mm");
> -		radeon_mem_types_list[i].name = radeon_mem_types_names[i];
> -		radeon_mem_types_list[i].show = &radeon_mm_dump_table;
> -		radeon_mem_types_list[i].driver_features = 0;
> +			sprintf(rdev->mem_types_names[i], "radeon_gtt_mm");
> +		rdev->mem_types_list[i].name = rdev->mem_types_names[i];
> +		rdev->mem_types_list[i].show = &radeon_mm_dump_table;
> +		rdev->mem_types_list[i].driver_features = 0;
>   		if (i == 0)
> -			radeon_mem_types_list[i].data = rdev->mman.bdev.man[TTM_PL_VRAM].priv;
> +			rdev->mem_types_list[i].data =
> +				rdev->mman.bdev.man[TTM_PL_VRAM].priv;
>   		else
> -			radeon_mem_types_list[i].data = rdev->mman.bdev.man[TTM_PL_TT].priv;
> +			rdev->mem_types_list[i].data =
> +				rdev->mman.bdev.man[TTM_PL_TT].priv;
>   
>   	}
>   	/* Add ttm page pool to debugfs */
> -	sprintf(radeon_mem_types_names[i], "ttm_page_pool");
> -	radeon_mem_types_list[i].name = radeon_mem_types_names[i];
> -	radeon_mem_types_list[i].show = &ttm_page_alloc_debugfs;
> -	radeon_mem_types_list[i].driver_features = 0;
> -	radeon_mem_types_list[i++].data = NULL;
> +	sprintf(rdev->mem_types_names[i], "ttm_page_pool");
> +	rdev->mem_types_list[i].name = rdev->mem_types_names[i];
> +	rdev->mem_types_list[i].show = &ttm_page_alloc_debugfs;
> +	rdev->mem_types_list[i].driver_features = 0;
> +	rdev->mem_types_list[i++].data = NULL;
>   #ifdef CONFIG_SWIOTLB
>   	if (swiotlb_nr_tbl()) {
> -		sprintf(radeon_mem_types_names[i], "ttm_dma_page_pool");
> -		radeon_mem_types_list[i].name = radeon_mem_types_names[i];
> -		radeon_mem_types_list[i].show = &ttm_dma_page_alloc_debugfs;
> -		radeon_mem_types_list[i].driver_features = 0;
> -		radeon_mem_types_list[i++].data = NULL;
> +		sprintf(rdev->mem_types_names[i], "ttm_dma_page_pool");
> +		rdev->mem_types_list[i].name = rdev->mem_types_names[i];
> +		rdev->mem_types_list[i].show = &ttm_dma_page_alloc_debugfs;
> +		rdev->mem_types_list[i].driver_features = 0;
> +		rdev->mem_types_list[i++].data = NULL;
>   	}
>   #endif
> -	return radeon_debugfs_add_files(rdev, radeon_mem_types_list, i);
> +	return radeon_debugfs_add_files(rdev, rdev->mem_types_list, i);
>   
>   #endif
>   	return 0;