From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-1?Q?Christian_K=F6nig?= Subject: Re: [PATCH] drm/radeon: fix ttm debugfs for multiple devices Date: Tue, 24 Dec 2013 11:48:29 +0100 Message-ID: <52B9667D.1050105@vodafone.de> References: <1387861117-3244-1-git-send-email-ihadzic@research.bell-labs.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from pegasos-out.vodafone.de (pegasos-out.vodafone.de [80.84.1.38]) by gabe.freedesktop.org (Postfix) with ESMTP id 3D9BFFA27C for ; Tue, 24 Dec 2013 02:48:34 -0800 (PST) In-Reply-To: <1387861117-3244-1-git-send-email-ihadzic@research.bell-labs.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org To: Ilija Hadzic , dri-devel@lists.freedesktop.org Cc: Ilija Hadzic List-Id: dri-devel@lists.freedesktop.org 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 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;