Subject: Re: [PATCH] drm/radeon: don't add files at control minor debugfs
 directory
To: Nicolai Stange <nicstange@gmail.com>,
        Alex Deucher <alexander.deucher@amd.com>,
        Daniel Vetter <daniel.vetter@intel.com>,
        <linux-kernel@vger.kernel.org>, <dri-devel@lists.freedesktop.org>
References: <20161203144700.2307-1-nicstange@gmail.com>
 <20161205072709.zcoja6eak2sjgd2k@phenom.ffwll.local>
From: =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
Message-ID: <5b8f4c38-c993-90af-5417-5e2e3cc38b58@amd.com>
Date: Mon, 5 Dec 2016 08:42:49 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.4.0
MIME-Version: 1.0
In-Reply-To: <20161205072709.zcoja6eak2sjgd2k@phenom.ffwll.local>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 8bit
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Dec 2016 07:43:03.7270 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR12MB1299
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 3075
Lines: 86

Am 05.12.2016 um 08:27 schrieb Daniel Vetter:
> On Sat, Dec 03, 2016 at 03:47:00PM +0100, Nicolai Stange wrote:
>> Since commit 8a357d10043c ("drm: Nerf DRM_CONTROL nodes"), a
>> struct drm_device's ->control member is always NULL.
>>
>> In the case of CONFIG_DEBUG_FS=y, radeon_debugfs_add_files() accesses
>> ->control->debugfs_root though. This results in the following Oops:
>>
>>    BUG: unable to handle kernel NULL pointer dereference at 0000000000000018
>>    IP: radeon_debugfs_add_files+0x90/0x100 [radeon]
>>    PGD 0
>>    Oops: 0000 [#1] SMP
>>    [...]
>>    Call Trace:
>>     ? work_on_cpu+0xb0/0xb0
>>     radeon_fence_driver_init+0x120/0x150 [radeon]
>>     si_init+0x122/0xd50 [radeon]
>>     ? _raw_spin_unlock_irq+0x2c/0x40
>>     ? device_pm_check_callbacks+0xb3/0xc0
>>     radeon_device_init+0x958/0xda0 [radeon]
>>     radeon_driver_load_kms+0x9a/0x210 [radeon]
>>     drm_dev_register+0xa9/0xd0 [drm]
>>     drm_get_pci_dev+0x9c/0x1e0 [drm]
>>     radeon_pci_probe+0xb8/0xe0 [radeon]
>>    [...]
>>
>> Fix this by omitting the drm_debugfs_create_files() call for the
>> control minor debugfs directory which is now non-existent anyway.
>>
>> Fixes: 8a357d10043c ("drm: Nerf DRM_CONTROL nodes")
>> Signed-off-by: Nicolai Stange <nicstange@gmail.com>
> Applied to drm-misc with Dave's irc ack, thanks for your patch.

If it's still worth it the patch is Reviewed-by: Christian K?nig 
<christian.koenig@amd.com>.

On the other hand when ->control is always NULL, why do we still have 
->control anyway?

And BTW: Please double check the other drivers as well.

Regards,
Christian.

> -Daniel
>
>> ---
>> Tested on top of next-20161202.
>> That 8a357d10043c ("drm: Nerf DRM_CONTROL nodes")
>> is in next since 20161201.
>>
>>   drivers/gpu/drm/radeon/radeon_device.c | 6 ------
>>   1 file changed, 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
>> index 60a8920..8a1df2a 100644
>> --- a/drivers/gpu/drm/radeon/radeon_device.c
>> +++ b/drivers/gpu/drm/radeon/radeon_device.c
>> @@ -1949,9 +1949,6 @@ int radeon_debugfs_add_files(struct radeon_device *rdev,
>>   	rdev->debugfs_count = i;
>>   #if defined(CONFIG_DEBUG_FS)
>>   	drm_debugfs_create_files(files, nfiles,
>> -				 rdev->ddev->control->debugfs_root,
>> -				 rdev->ddev->control);
>> -	drm_debugfs_create_files(files, nfiles,
>>   				 rdev->ddev->primary->debugfs_root,
>>   				 rdev->ddev->primary);
>>   #endif
>> @@ -1966,9 +1963,6 @@ static void radeon_debugfs_remove_files(struct radeon_device *rdev)
>>   	for (i = 0; i < rdev->debugfs_count; i++) {
>>   		drm_debugfs_remove_files(rdev->debugfs[i].files,
>>   					 rdev->debugfs[i].num_files,
>> -					 rdev->ddev->control);
>> -		drm_debugfs_remove_files(rdev->debugfs[i].files,
>> -					 rdev->debugfs[i].num_files,
>>   					 rdev->ddev->primary);
>>   	}
>>   #endif
>> -- 
>> 2.10.2
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel