All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/radeon: fix ttm debugfs for multiple devices
@ 2013-12-24  4:58 Ilija Hadzic
  2013-12-24 10:48 ` Christian König
  0 siblings, 1 reply; 2+ messages in thread
From: Ilija Hadzic @ 2013-12-24  4:58 UTC (permalink / raw)
  To: dri-devel; +Cc: 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>
---
 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;
-- 
1.8.2.1

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] drm/radeon: fix ttm debugfs for multiple devices
  2013-12-24  4:58 [PATCH] drm/radeon: fix ttm debugfs for multiple devices Ilija Hadzic
@ 2013-12-24 10:48 ` Christian König
  0 siblings, 0 replies; 2+ messages in thread
From: Christian König @ 2013-12-24 10:48 UTC (permalink / raw)
  To: Ilija Hadzic, dri-devel; +Cc: Ilija Hadzic

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;

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2013-12-24 10:48 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-24  4:58 [PATCH] drm/radeon: fix ttm debugfs for multiple devices Ilija Hadzic
2013-12-24 10:48 ` Christian König

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.