diff mbox

drm/radeon: fix ttm debugfs for multiple devices

Message ID 1387861117-3244-1-git-send-email-ihadzic@research.bell-labs.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ilija Hadzic Dec. 24, 2013, 4:58 a.m. UTC 
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>
---
 drivers/gpu/drm/radeon/radeon.h     |  6 ++++++
 drivers/gpu/drm/radeon/radeon_ttm.c | 43 +++++++++++++++++--------------------
 2 files changed, 26 insertions(+), 23 deletions(-)

Comments

Christian König Dec. 24, 2013, 10:48 a.m. UTC | #1 
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;
diff mbox

Patch

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;