Re: [Intel-gfx] [PATCH 3/3] drm/i915: Use fence_write() from rpm resume
On ma, 2016-10-10 at 22:58 +0100, Chris Wilson wrote:
> During rpm resume we restore the fences, but we do not have the
> protection of struct_mutex. This rules out updating the activity
> tracking on the fences, and requires us to rely on the rpm as the
> serialisation barrier instead.
>
> [ 350.298052] [drm:intel_runtime_resume [i915]] Resuming device
> [ 350.308606]
> [ 350.310520] ===
> [ 350.315560] [ INFO: suspicious RCU usage. ]
> [ 350.320554] 4.8.0-rc8-bsw-rapl+ #3133 Tainted: G U W
> [ 350.327208] ---
> [ 350.331977] ../drivers/gpu/drm/i915/i915_gem_request.h:371 suspicious
> rcu_dereference_protected() usage!
> [ 350.342619]
> [ 350.342619] other info that might help us debug this:
> [ 350.342619]
> [ 350.351593]
> [ 350.351593] rcu_scheduler_active = 1, debug_locks = 0
> [ 350.358952] 3 locks held by Xorg/320:
> [ 350.363077] #0: (>mode_config.mutex){+.+.+.}, at:
> [] drm_modeset_lock_all+0x3c/0xd0 [drm]
> [ 350.375162] #1: (crtc_ww_class_acquire){+.+.+.}, at:
> [] drm_modeset_lock_all+0x46/0xd0 [drm]
> [ 350.387022] #2: (crtc_ww_class_mutex){+.+.+.}, at: []
> drm_modeset_lock+0x36/0x110 [drm]
> [ 350.398236]
> [ 350.398236] stack backtrace:
> [ 350.403196] CPU: 1 PID: 320 Comm: Xorg Tainted: G U W
> 4.8.0-rc8-bsw-rapl+ #3133
> [ 350.412457] Hardware name: Intel Corporation CHERRYVIEW C0
> PLATFORM/Braswell CRB, BIOS BRAS.X64.X088.R00.1510270350 10/27/2015
> [ 350.425212] 8801680a78c8 81332187
> 88016c5c5000
> [ 350.433611] 0001 8801680a78f8 810ca6da
> 88016cc8b0f0
> [ 350.442012] 88016cc8 88016cc8 880177ad
> 8801680a7948
> [ 350.450409] Call Trace:
> [ 350.453165] [] dump_stack+0x67/0x90
> [ 350.458931] [] lockdep_rcu_suspicious+0xea/0x120
> [ 350.466002] [] fence_update+0xbd/0x670 [i915]
> [ 350.472766] [] i915_gem_restore_fences+0x52/0x70 [i915]
> [ 350.480496] [] vlv_resume_prepare+0x72/0x570 [i915]
> [ 350.487839] [] intel_runtime_resume+0x102/0x210 [i915]
> [ 350.495442] [] pci_pm_runtime_resume+0x7f/0xb0
> [ 350.502274] [] ? pci_restore_standard_config+0x40/0x40
> [ 350.509883] [] __rpm_callback+0x35/0x70
> [ 350.516037] [] ? pci_restore_standard_config+0x40/0x40
> [ 350.523646] [] rpm_callback+0x24/0x80
> [ 350.529604] [] ? pci_restore_standard_config+0x40/0x40
> [ 350.537212] [] rpm_resume+0x4ad/0x740
> [ 350.543161] [] __pm_runtime_resume+0x51/0x80
> [ 350.549824] [] intel_runtime_pm_get+0x28/0x90 [i915]
> [ 350.557265] [] intel_display_power_get+0x23/0x50 [i915]
> [ 350.565001] [] intel_atomic_commit_tail+0xdfd/0x10b0
> [i915]
> [ 350.573106] [] ?
> drm_atomic_helper_swap_state+0x159/0x300 [drm_kms_helper]
> [ 350.582659] [] ? _raw_spin_unlock+0x31/0x50
> [ 350.589205] [] ?
> drm_atomic_helper_swap_state+0x159/0x300 [drm_kms_helper]
> [ 350.598787] [] intel_atomic_commit+0x3b5/0x500 [i915]
> [ 350.606319] [] ?
> drm_atomic_set_crtc_for_connector+0xcc/0x100 [drm]
> [ 350.615209] [] drm_atomic_commit+0x49/0x50 [drm]
> [ 350.622242] [] drm_atomic_helper_set_config+0x88/0xc0
> [drm_kms_helper]
> [ 350.631419] [] drm_mode_set_config_internal+0x6c/0x120
> [drm]
> [ 350.639623] [] drm_mode_setcrtc+0x22c/0x4d0 [drm]
> [ 350.646760] [] drm_ioctl+0x209/0x460 [drm]
> [ 350.653217] [] ? drm_mode_getcrtc+0x150/0x150 [drm]
> [ 350.660536] [] ? __lock_is_held+0x4a/0x70
> [ 350.666885] [] do_vfs_ioctl+0x93/0x6b0
> [ 350.672939] [] ? __fget+0x113/0x200
> [ 350.678797] [] ? __fget+0x5/0x200
> [ 350.684361] [] SyS_ioctl+0x44/0x80
> [ 350.690030] [] do_syscall_64+0x5b/0x120
> [ 350.696184] [] entry_SYSCALL64_slow_path+0x25/0x25
>
> Note we also have to remember the lesson from commit 4fc788f5ee3d
> ("drm/i915: Flush delayed fence releases after reset") where we have to
> flush any changes to the fence on restore.
>
> v2: Replace call to release user mmaps with an assertion that they have
> already been zapped.
>
> Fixes: 49ef5294cda2 ("drm/i915: Move fence tracking from object to vma")
> Reported-by: Ville Syrjälä
> Signed-off-by: Chris Wilson
> Cc: Ville Syrjälä
> Cc: Joonas Lahtinen
> Cc: Mika Kuoppala
Reviewed-by: Joonas Lahtinen
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Use fence_write() from rpm resume
During rpm resume we restore the fences, but we do not have the
protection of struct_mutex. This rules out updating the activity
tracking on the fences, and requires us to rely on the rpm as the
serialisation barrier instead.
[ 350.298052] [drm:intel_runtime_resume [i915]] Resuming device
[ 350.308606]
[ 350.310520] ===
[ 350.315560] [ INFO: suspicious RCU usage. ]
[ 350.320554] 4.8.0-rc8-bsw-rapl+ #3133 Tainted: G U W
[ 350.327208] ---
[ 350.331977] ../drivers/gpu/drm/i915/i915_gem_request.h:371 suspicious
rcu_dereference_protected() usage!
[ 350.342619]
[ 350.342619] other info that might help us debug this:
[ 350.342619]
[ 350.351593]
[ 350.351593] rcu_scheduler_active = 1, debug_locks = 0
[ 350.358952] 3 locks held by Xorg/320:
[ 350.363077] #0: (>mode_config.mutex){+.+.+.}, at:
[] drm_modeset_lock_all+0x3c/0xd0 [drm]
[ 350.375162] #1: (crtc_ww_class_acquire){+.+.+.}, at: []
drm_modeset_lock_all+0x46/0xd0 [drm]
[ 350.387022] #2: (crtc_ww_class_mutex){+.+.+.}, at: []
drm_modeset_lock+0x36/0x110 [drm]
[ 350.398236]
[ 350.398236] stack backtrace:
[ 350.403196] CPU: 1 PID: 320 Comm: Xorg Tainted: G U W
4.8.0-rc8-bsw-rapl+ #3133
[ 350.412457] Hardware name: Intel Corporation CHERRYVIEW C0 PLATFORM/Braswell
CRB, BIOS BRAS.X64.X088.R00.1510270350 10/27/2015
[ 350.425212] 8801680a78c8 81332187
88016c5c5000
[ 350.433611] 0001 8801680a78f8 810ca6da
88016cc8b0f0
[ 350.442012] 88016cc8 88016cc8 880177ad
8801680a7948
[ 350.450409] Call Trace:
[ 350.453165] [] dump_stack+0x67/0x90
[ 350.458931] [] lockdep_rcu_suspicious+0xea/0x120
[ 350.466002] [] fence_update+0xbd/0x670 [i915]
[ 350.472766] [] i915_gem_restore_fences+0x52/0x70 [i915]
[ 350.480496] [] vlv_resume_prepare+0x72/0x570 [i915]
[ 350.487839] [] intel_runtime_resume+0x102/0x210 [i915]
[ 350.495442] [] pci_pm_runtime_resume+0x7f/0xb0
[ 350.502274] [] ? pci_restore_standard_config+0x40/0x40
[ 350.509883] [] __rpm_callback+0x35/0x70
[ 350.516037] [] ? pci_restore_standard_config+0x40/0x40
[ 350.523646] [] rpm_callback+0x24/0x80
[ 350.529604] [] ? pci_restore_standard_config+0x40/0x40
[ 350.537212] [] rpm_resume+0x4ad/0x740
[ 350.543161] [] __pm_runtime_resume+0x51/0x80
[ 350.549824] [] intel_runtime_pm_get+0x28/0x90 [i915]
[ 350.557265] [] intel_display_power_get+0x23/0x50 [i915]
[ 350.565001] [] intel_atomic_commit_tail+0xdfd/0x10b0
[i915]
[ 350.573106] [] ? drm_atomic_helper_swap_state+0x159/0x300
[drm_kms_helper]
[ 350.582659] [] ? _raw_spin_unlock+0x31/0x50
[ 350.589205] [] ? drm_atomic_helper_swap_state+0x159/0x300
[drm_kms_helper]
[ 350.598787] [] intel_atomic_commit+0x3b5/0x500 [i915]
[ 350.606319] [] ?
drm_atomic_set_crtc_for_connector+0xcc/0x100 [drm]
[ 350.615209] [] drm_atomic_commit+0x49/0x50 [drm]
[ 350.622242] [] drm_atomic_helper_set_config+0x88/0xc0
[drm_kms_helper]
[ 350.631419] [] drm_mode_set_config_internal+0x6c/0x120
[drm]
[ 350.639623] [] drm_mode_setcrtc+0x22c/0x4d0 [drm]
[ 350.646760] [] drm_ioctl+0x209/0x460 [drm]
[ 350.653217] [] ? drm_mode_getcrtc+0x150/0x150 [drm]
[ 350.660536] [] ? __lock_is_held+0x4a/0x70
[ 350.666885] [] do_vfs_ioctl+0x93/0x6b0
[ 350.672939] [] ? __fget+0x113/0x200
[ 350.678797] [] ? __fget+0x5/0x200
[ 350.684361] [] SyS_ioctl+0x44/0x80
[ 350.690030] [] do_syscall_64+0x5b/0x120
[ 350.696184] [] entry_SYSCALL64_slow_path+0x25/0x25
Note we also have to remember the lesson from commit 4fc788f5ee3d
("drm/i915: Flush delayed fence releases after reset") where we have to
flush any changes to the fence on restore.
v2: Replace call to release user mmaps with an assertion that they have
already been zapped.
Fixes: 49ef5294cda2 ("drm/i915: Move fence tracking from object to vma")
Reported-by: Ville Syrjälä
Signed-off-by: Chris Wilson
Cc: Ville Syrjälä
Cc: Joonas Lahtinen
Cc: Mika Kuoppala
---
drivers/gpu/drm/i915/i915_gem_fence.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_fence.c
b/drivers/gpu/drm/i915/i915_gem_fence.c
index 8df1fa7234e8..2c7ba0ee127c 100644
--- a/drivers/gpu/drm/i915/i915_gem_fence.c
+++ b/drivers/gpu/drm/i915/i915_gem_fence.c
@@ -290,6 +290,8 @@ i915_vma_put_fence(struct i915_vma *vma)
{
struct drm_i915_fence_reg *fence = vma->fence;
+ assert_rpm_wakelock_held(to_i915(vma->vm->dev));
+
if (!fence)
return 0;
@@ -341,6 +343,8 @@ i915_vma_get_fence(struct i915_vma *vma)
struct drm_i915_fence_reg *fence;
struct i915_vma *set = i915_gem_object_is_tiled(vma->obj) ? vma : NULL;
+ assert_rpm_wakelock_held(to_i915(vma->vm->dev));
+
/*
