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 --- 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
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. -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
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);-- 2.10.2
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, Dec 05, 2016 at 08:42:49AM +0100, Christian König wrote:
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?
I'm chicken and didn't want to mass-delete code from the start. But yeah after a few kernel releases to make sure no one noticed the removal of control nodes we can garbage collected. I guess I've been bitten a few too many times when trying to remove old stuff ;-)
And BTW: Please double check the other drivers as well.
Yeah, I've done a full audit, only qxl turned up to have similar code. Already merged with Dave's irc ack. -Daniel
Christian König christian.koenig@amd.com writes:
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?
Yes, I was wondering about that, too.
Quoting from 8a357d10043c ("drm: Nerf DRM_CONTROL nodes"):
Since I don't like dead uabi, let's remove it. But since this would be a really big change I think it's better to start out small by simply not registering anything. We can garbage-collect the dead code later on, once we're sure it's really not used anywhere.
I'd too prefer compile time errors by purging ->control here. Daniel?
And BTW: Please double check the other drivers as well.
# git grep '->control' -- drivers/gpu/ drivers/gpu/drm/amd/amdgpu/amdgpu_device.c: adev->ddev->control->debugfs_root, drivers/gpu/drm/amd/amdgpu/amdgpu_device.c: adev->ddev->control); drivers/gpu/drm/amd/amdgpu/amdgpu_device.c: adev->ddev->control);
Oops.
drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c: ib_packet->control = (1 << 23) | (1 << 31) | drivers/gpu/drm/drm_drv.c: return &dev->control;
That's drm_minor_get_slot(dev, type), but grepping for DRM_MINOR_CONTROL doesn't yield anything -> dead code.
drivers/gpu/drm/gma500/psb_intel_sdvo.c: switch (sdvo->controlled_output) { drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS0; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS1; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= type; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB0; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB1; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS0; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS1; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output = 0; drivers/gpu/drm/i915/intel_sdvo.c: switch (sdvo->controlled_output) { drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS0; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS1; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= type; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB0; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB1; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS0; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS1; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output = 0; drivers/gpu/drm/msm/msm_debugfs.c: ret = late_init_minor(dev->control);
Not an oops but dead code.
drivers/gpu/drm/qxl/qxl_debugfs.c: qdev->ddev->control->debugfs_root, drivers/gpu/drm/qxl/qxl_debugfs.c: qdev->ddev->control); drivers/gpu/drm/qxl/qxl_debugfs.c: qdev->ddev->control);
Oops.
I'll send compile-only tested patches either tonight or tomorrow. Shall they cover the oopses only or the dead code as well?
Thanks,
Nicolai
Am 05.12.2016 um 09:39 schrieb Nicolai Stange:
Christian König christian.koenig@amd.com writes:
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?
Yes, I was wondering about that, too.
Quoting from 8a357d10043c ("drm: Nerf DRM_CONTROL nodes"):
Since I don't like dead uabi, let's remove it. But since this would be a really big change I think it's better to start out small by simply not registering anything. We can garbage-collect the dead code later on, once we're sure it's really not used anywhere.
I'd too prefer compile time errors by purging ->control here. Daniel?
Seconded.
And BTW: Please double check the other drivers as well.
# git grep '->control' -- drivers/gpu/ drivers/gpu/drm/amd/amdgpu/amdgpu_device.c: adev->ddev->control->debugfs_root, drivers/gpu/drm/amd/amdgpu/amdgpu_device.c: adev->ddev->control); drivers/gpu/drm/amd/amdgpu/amdgpu_device.c: adev->ddev->control);
Oops.
Yeah, that's what I expected as well but Daniel said it would only affect qxl.
drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c: ib_packet->control = (1 << 23) | (1 << 31) | drivers/gpu/drm/drm_drv.c: return &dev->control;
That's drm_minor_get_slot(dev, type), but grepping for DRM_MINOR_CONTROL doesn't yield anything -> dead code.
drivers/gpu/drm/gma500/psb_intel_sdvo.c: switch (sdvo->controlled_output) { drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS0; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS1; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= type; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB0; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB1; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS0; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS1; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output = 0; drivers/gpu/drm/i915/intel_sdvo.c: switch (sdvo->controlled_output) { drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS0; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS1; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= type; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB0; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB1; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS0; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS1; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output = 0; drivers/gpu/drm/msm/msm_debugfs.c: ret = late_init_minor(dev->control);
Not an oops but dead code.
drivers/gpu/drm/qxl/qxl_debugfs.c: qdev->ddev->control->debugfs_root, drivers/gpu/drm/qxl/qxl_debugfs.c: qdev->ddev->control); drivers/gpu/drm/qxl/qxl_debugfs.c: qdev->ddev->control);
Oops.
I'll send compile-only tested patches either tonight or tomorrow. Shall they cover the oopses only or the dead code as well?
Please start with the ops, cause we certainly will get complains about that rather fast.
Regards, Christian.
Thanks,
Nicolai
On 05/12/16 05:48 PM, Christian König wrote:
Am 05.12.2016 um 09:39 schrieb Nicolai Stange:>>
I'll send compile-only tested patches either tonight or tomorrow. Shall they cover the oopses only or the dead code as well?
Please start with the ops, cause we certainly will get complains about that rather fast.
Suspect we already did:
https://bugs.freedesktop.org/show_bug.cgi?id=98915
On Mon, Dec 05, 2016 at 09:48:02AM +0100, Christian König wrote:
Am 05.12.2016 um 09:39 schrieb Nicolai Stange:
Christian König christian.koenig@amd.com writes:
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?
Yes, I was wondering about that, too.
Quoting from 8a357d10043c ("drm: Nerf DRM_CONTROL nodes"):
Since I don't like dead uabi, let's remove it. But since this would be a really big change I think it's better to start out small by simply not registering anything. We can garbage-collect the dead code later on, once we're sure it's really not used anywhere.
I'd too prefer compile time errors by purging ->control here. Daniel?
Seconded.
We're super-late for 4.10, I think that'd would need to be postponened for 4.11. Not need to wait with creating the patches (drm-misc is always open), but wee need to plug the oopses first.
And BTW: Please double check the other drivers as well.
# git grep '->control' -- drivers/gpu/ drivers/gpu/drm/amd/amdgpu/amdgpu_device.c: adev->ddev->control->debugfs_root, drivers/gpu/drm/amd/amdgpu/amdgpu_device.c: adev->ddev->control); drivers/gpu/drm/amd/amdgpu/amdgpu_device.c: adev->ddev->control);
Oops.
Yeah, that's what I expected as well but Daniel said it would only affect qxl.
No idea why I've missed that, I guess coffee wasn't working.
drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c: ib_packet->control = (1 << 23) | (1 << 31) | drivers/gpu/drm/drm_drv.c: return &dev->control;
That's drm_minor_get_slot(dev, type), but grepping for DRM_MINOR_CONTROL doesn't yield anything -> dead code.
drivers/gpu/drm/gma500/psb_intel_sdvo.c: switch (sdvo->controlled_output) { drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS0; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS1; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= type; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB0; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB1; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS0; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS1; drivers/gpu/drm/gma500/psb_intel_sdvo.c: psb_intel_sdvo->controlled_output = 0; drivers/gpu/drm/i915/intel_sdvo.c: switch (sdvo->controlled_output) { drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS0; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= SDVO_OUTPUT_TMDS1; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= type; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB0; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= SDVO_OUTPUT_RGB1; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS0; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output |= SDVO_OUTPUT_LVDS1; drivers/gpu/drm/i915/intel_sdvo.c: intel_sdvo->controlled_output = 0; drivers/gpu/drm/msm/msm_debugfs.c: ret = late_init_minor(dev->control);
Not an oops but dead code.
drivers/gpu/drm/qxl/qxl_debugfs.c: qdev->ddev->control->debugfs_root, drivers/gpu/drm/qxl/qxl_debugfs.c: qdev->ddev->control); drivers/gpu/drm/qxl/qxl_debugfs.c: qdev->ddev->control);
Oops.
I'll send compile-only tested patches either tonight or tomorrow. Shall they cover the oopses only or the dead code as well?
Please start with the ops, cause we certainly will get complains about that rather fast.
I sent out qxl already, and yes oops patches first pls. -Daniel
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, amdgpu_debugfs_add_files() accesses ->control->debugfs_root though. This results in a NULL pointer dereference.
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 --- Applicable to next-20161202. Compile-only tested.
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index deee2db..0cb3e82 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2493,9 +2493,6 @@ int amdgpu_debugfs_add_files(struct amdgpu_device *adev, adev->debugfs_count = i; #if defined(CONFIG_DEBUG_FS) drm_debugfs_create_files(files, nfiles, - adev->ddev->control->debugfs_root, - adev->ddev->control); - drm_debugfs_create_files(files, nfiles, adev->ddev->primary->debugfs_root, adev->ddev->primary); #endif @@ -2510,9 +2507,6 @@ static void amdgpu_debugfs_remove_files(struct amdgpu_device *adev) for (i = 0; i < adev->debugfs_count; i++) { drm_debugfs_remove_files(adev->debugfs[i].files, adev->debugfs[i].num_files, - adev->ddev->control); - drm_debugfs_remove_files(adev->debugfs[i].files, - adev->debugfs[i].num_files, adev->ddev->primary); } #endif
-----Original Message----- From: Nicolai Stange [mailto:nicstange@gmail.com] Sent: Monday, December 05, 2016 3:30 PM To: Daniel Vetter Cc: Deucher, Alexander; Koenig, Christian; Michel Dänzer; linux- kernel@vger.kernel.org; dri-devel@lists.freedesktop.org; Nicolai Stange Subject: [PATCH] drm/amdgpu: don't add files at control minor debugfs directory
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, amdgpu_debugfs_add_files() accesses ->control->debugfs_root though. This results in a NULL pointer dereference.
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
Please add the bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98915 With that, Reviewed-by: Alex Deucher alexander.deucher@amd.com
Applicable to next-20161202. Compile-only tested.
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index deee2db..0cb3e82 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2493,9 +2493,6 @@ int amdgpu_debugfs_add_files(struct amdgpu_device *adev, adev->debugfs_count = i; #if defined(CONFIG_DEBUG_FS) drm_debugfs_create_files(files, nfiles,
adev->ddev->control->debugfs_root,adev->ddev->control);- drm_debugfs_create_files(files, nfiles, adev->ddev->primary->debugfs_root, adev->ddev->primary);
#endif @@ -2510,9 +2507,6 @@ static void amdgpu_debugfs_remove_files(struct amdgpu_device *adev) for (i = 0; i < adev->debugfs_count; i++) { drm_debugfs_remove_files(adev->debugfs[i].files, adev->debugfs[i].num_files,
adev->ddev->control);drm_debugfs_remove_files(adev->debugfs[i].files,adev->debugfs[i].num_files, adev->ddev->primary);#endif
2.10.2
Feel free to add a tested by from myself
Thanks for the fix
On Mon, 5 Dec 2016 at 20:33 Deucher, Alexander Alexander.Deucher@amd.com wrote:
-----Original Message----- From: Nicolai Stange [mailto:nicstange@gmail.com] Sent: Monday, December 05, 2016 3:30 PM To: Daniel Vetter Cc: Deucher, Alexander; Koenig, Christian; Michel Dänzer; linux- kernel@vger.kernel.org; dri-devel@lists.freedesktop.org; Nicolai Stange Subject: [PATCH] drm/amdgpu: don't add files at control minor debugfs directory
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, amdgpu_debugfs_add_files() accesses ->control->debugfs_root though. This results in a NULL pointer dereference.
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
Please add the bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98915 With that, Reviewed-by: Alex Deucher alexander.deucher@amd.com
Applicable to next-20161202. Compile-only tested.
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index deee2db..0cb3e82 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2493,9 +2493,6 @@ int amdgpu_debugfs_add_files(struct amdgpu_device *adev, adev->debugfs_count = i; #if defined(CONFIG_DEBUG_FS) drm_debugfs_create_files(files, nfiles,
adev->ddev->control->debugfs_root,adev->ddev->control);drm_debugfs_create_files(files, nfiles, adev->ddev->primary->debugfs_root, adev->ddev->primary);#endif @@ -2510,9 +2507,6 @@ static void amdgpu_debugfs_remove_files(struct amdgpu_device *adev) for (i = 0; i < adev->debugfs_count; i++) { drm_debugfs_remove_files(adev->debugfs[i].files, adev->debugfs[i].num_files,
adev->ddev->control);drm_debugfs_remove_files(adev->debugfs[i].files,adev->debugfs[i].num_files, adev->ddev->primary); }#endif
2.10.2
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Dec 06, 2016 at 02:01:37AM +0000, Mike Lothian wrote:
Feel free to add a tested by from myself
Thanks for the fix
On Mon, 5 Dec 2016 at 20:33 Deucher, Alexander Alexander.Deucher@amd.com wrote:
-----Original Message----- From: Nicolai Stange [mailto:nicstange@gmail.com] Sent: Monday, December 05, 2016 3:30 PM To: Daniel Vetter Cc: Deucher, Alexander; Koenig, Christian; Michel Dänzer; linux- kernel@vger.kernel.org; dri-devel@lists.freedesktop.org; Nicolai Stange Subject: [PATCH] drm/amdgpu: don't add files at control minor debugfs directory
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, amdgpu_debugfs_add_files() accesses ->control->debugfs_root though. This results in a NULL pointer dereference.
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
Please add the bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98915 With that, Reviewed-by: Alex Deucher alexander.deucher@amd.com
Applied to drm-misc, thanks for catching this and sorry for the fallout I caused. -Daniel
Applicable to next-20161202. Compile-only tested.
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index deee2db..0cb3e82 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2493,9 +2493,6 @@ int amdgpu_debugfs_add_files(struct amdgpu_device *adev, adev->debugfs_count = i; #if defined(CONFIG_DEBUG_FS) drm_debugfs_create_files(files, nfiles,
adev->ddev->control->debugfs_root,adev->ddev->control);drm_debugfs_create_files(files, nfiles, adev->ddev->primary->debugfs_root, adev->ddev->primary);#endif @@ -2510,9 +2507,6 @@ static void amdgpu_debugfs_remove_files(struct amdgpu_device *adev) for (i = 0; i < adev->debugfs_count; i++) { drm_debugfs_remove_files(adev->debugfs[i].files, adev->debugfs[i].num_files,
adev->ddev->control);drm_debugfs_remove_files(adev->debugfs[i].files,adev->debugfs[i].num_files, adev->ddev->primary); }#endif
2.10.2
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Christian König -
Daniel Vetter -
Deucher, Alexander -
Michel Dänzer -
Mike Lothian -
Nicolai Stange