* [PATCH] drm/i915: Avoid circular locking dependency when flush delayed work on gt reset
@ 2023-06-06 3:00 ` Zhanjun Dong
0 siblings, 0 replies; 8+ messages in thread
From: Zhanjun Dong @ 2023-06-06 3:00 UTC (permalink / raw)
To: intel-gfx, dri-devel; +Cc: Zhanjun Dong
This attemps to avoid circular locing dependency between flush delayed work and intel_gt_reset.
WARNING: possible circular locking dependency detected
6.4.0-rc1-drmtip_1340-g31e3463b0edb+ #1 Not tainted
------------------------------------------------------
kms_pipe_crc_ba/6415 is trying to acquire lock:
ffff88813e6cc640 ((work_completion)(&(&guc->timestamp.work)->work)){+.+.}-{0:0}, at: __flush_work+0x42/0x530
but task is already holding lock:
ffff88813e6cce90 (>->reset.mutex){+.+.}-{3:3}, at: intel_gt_reset+0x19e/0x470 [i915]
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #3 (>->reset.mutex){+.+.}-{3:3}:
lock_acquire+0xd8/0x2d0
i915_gem_shrinker_taints_mutex+0x31/0x50 [i915]
intel_gt_init_reset+0x65/0x80 [i915]
intel_gt_common_init_early+0xe1/0x170 [i915]
intel_root_gt_init_early+0x48/0x60 [i915]
i915_driver_probe+0x671/0xcb0 [i915]
i915_pci_probe+0xdc/0x210 [i915]
pci_device_probe+0x95/0x120
really_probe+0x164/0x3c0
__driver_probe_device+0x73/0x160
driver_probe_device+0x19/0xa0
__driver_attach+0xb6/0x180
bus_for_each_dev+0x77/0xd0
bus_add_driver+0x114/0x210
driver_register+0x5b/0x110
__pfx_vgem_open+0x3/0x10 [vgem]
do_one_initcall+0x57/0x270
do_init_module+0x5f/0x220
load_module+0x1ca4/0x1f00
__do_sys_finit_module+0xb4/0x130
do_syscall_64+0x3c/0x90
entry_SYSCALL_64_after_hwframe+0x72/0xdc
-> #2 (fs_reclaim){+.+.}-{0:0}:
lock_acquire+0xd8/0x2d0
fs_reclaim_acquire+0xac/0xe0
kmem_cache_alloc+0x32/0x260
i915_vma_instance+0xb2/0xc60 [i915]
i915_gem_object_ggtt_pin_ww+0x175/0x370 [i915]
vm_fault_gtt+0x22d/0xf60 [i915]
__do_fault+0x2f/0x1d0
do_pte_missing+0x4a/0xd20
__handle_mm_fault+0x5b0/0x790
handle_mm_fault+0xa2/0x230
do_user_addr_fault+0x3ea/0xa10
exc_page_fault+0x68/0x1a0
asm_exc_page_fault+0x26/0x30
-> #1 (>->reset.backoff_srcu){++++}-{0:0}:
lock_acquire+0xd8/0x2d0
_intel_gt_reset_lock+0x57/0x330 [i915]
guc_timestamp_ping+0x35/0x130 [i915]
process_one_work+0x250/0x510
worker_thread+0x4f/0x3a0
kthread+0xff/0x130
ret_from_fork+0x29/0x50
-> #0 ((work_completion)(&(&guc->timestamp.work)->work)){+.+.}-{0:0}:
check_prev_add+0x90/0xc60
__lock_acquire+0x1998/0x2590
lock_acquire+0xd8/0x2d0
__flush_work+0x74/0x530
__cancel_work_timer+0x14f/0x1f0
intel_guc_submission_reset_prepare+0x81/0x4b0 [i915]
intel_uc_reset_prepare+0x9c/0x120 [i915]
reset_prepare+0x21/0x60 [i915]
intel_gt_reset+0x1dd/0x470 [i915]
intel_gt_reset_global+0xfb/0x170 [i915]
intel_gt_handle_error+0x368/0x420 [i915]
intel_gt_debugfs_reset_store+0x5c/0xc0 [i915]
i915_wedged_set+0x29/0x40 [i915]
simple_attr_write_xsigned.constprop.0+0xb4/0x110
full_proxy_write+0x52/0x80
vfs_write+0xc5/0x4f0
ksys_write+0x64/0xe0
do_syscall_64+0x3c/0x90
entry_SYSCALL_64_after_hwframe+0x72/0xdc
other info that might help us debug this:
Chain exists of:
(work_completion)(&(&guc->timestamp.work)->work) --> fs_reclaim --> >->reset.mutex
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(>->reset.mutex);
lock(fs_reclaim);
lock(>->reset.mutex);
lock((work_completion)(&(&guc->timestamp.work)->work));
*** DEADLOCK ***
3 locks held by kms_pipe_crc_ba/6415:
#0: ffff888101541430 (sb_writers#15){.+.+}-{0:0}, at: ksys_write+0x64/0xe0
#1: ffff888136c7eab8 (&attr->mutex){+.+.}-{3:3}, at: simple_attr_write_xsigned.constprop.0+0x47/0x110
#2: ffff88813e6cce90 (>->reset.mutex){+.+.}-{3:3}, at: intel_gt_reset+0x19e/0x470 [i915]
Signed-off-by: Zhanjun Dong <zhanjun.dong@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index a0e3ef1c65d2..22390704542e 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1359,7 +1359,7 @@ static void guc_enable_busyness_worker(struct intel_guc *guc)
static void guc_cancel_busyness_worker(struct intel_guc *guc)
{
- cancel_delayed_work_sync(&guc->timestamp.work);
+ cancel_delayed_work(&guc->timestamp.work);
}
static void __reset_guc_busyness_stats(struct intel_guc *guc)
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Intel-gfx] [PATCH] drm/i915: Avoid circular locking dependency when flush delayed work on gt reset
@ 2023-06-06 3:00 ` Zhanjun Dong
0 siblings, 0 replies; 8+ messages in thread
From: Zhanjun Dong @ 2023-06-06 3:00 UTC (permalink / raw)
To: intel-gfx, dri-devel
This attemps to avoid circular locing dependency between flush delayed work and intel_gt_reset.
WARNING: possible circular locking dependency detected
6.4.0-rc1-drmtip_1340-g31e3463b0edb+ #1 Not tainted
------------------------------------------------------
kms_pipe_crc_ba/6415 is trying to acquire lock:
ffff88813e6cc640 ((work_completion)(&(&guc->timestamp.work)->work)){+.+.}-{0:0}, at: __flush_work+0x42/0x530
but task is already holding lock:
ffff88813e6cce90 (>->reset.mutex){+.+.}-{3:3}, at: intel_gt_reset+0x19e/0x470 [i915]
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #3 (>->reset.mutex){+.+.}-{3:3}:
lock_acquire+0xd8/0x2d0
i915_gem_shrinker_taints_mutex+0x31/0x50 [i915]
intel_gt_init_reset+0x65/0x80 [i915]
intel_gt_common_init_early+0xe1/0x170 [i915]
intel_root_gt_init_early+0x48/0x60 [i915]
i915_driver_probe+0x671/0xcb0 [i915]
i915_pci_probe+0xdc/0x210 [i915]
pci_device_probe+0x95/0x120
really_probe+0x164/0x3c0
__driver_probe_device+0x73/0x160
driver_probe_device+0x19/0xa0
__driver_attach+0xb6/0x180
bus_for_each_dev+0x77/0xd0
bus_add_driver+0x114/0x210
driver_register+0x5b/0x110
__pfx_vgem_open+0x3/0x10 [vgem]
do_one_initcall+0x57/0x270
do_init_module+0x5f/0x220
load_module+0x1ca4/0x1f00
__do_sys_finit_module+0xb4/0x130
do_syscall_64+0x3c/0x90
entry_SYSCALL_64_after_hwframe+0x72/0xdc
-> #2 (fs_reclaim){+.+.}-{0:0}:
lock_acquire+0xd8/0x2d0
fs_reclaim_acquire+0xac/0xe0
kmem_cache_alloc+0x32/0x260
i915_vma_instance+0xb2/0xc60 [i915]
i915_gem_object_ggtt_pin_ww+0x175/0x370 [i915]
vm_fault_gtt+0x22d/0xf60 [i915]
__do_fault+0x2f/0x1d0
do_pte_missing+0x4a/0xd20
__handle_mm_fault+0x5b0/0x790
handle_mm_fault+0xa2/0x230
do_user_addr_fault+0x3ea/0xa10
exc_page_fault+0x68/0x1a0
asm_exc_page_fault+0x26/0x30
-> #1 (>->reset.backoff_srcu){++++}-{0:0}:
lock_acquire+0xd8/0x2d0
_intel_gt_reset_lock+0x57/0x330 [i915]
guc_timestamp_ping+0x35/0x130 [i915]
process_one_work+0x250/0x510
worker_thread+0x4f/0x3a0
kthread+0xff/0x130
ret_from_fork+0x29/0x50
-> #0 ((work_completion)(&(&guc->timestamp.work)->work)){+.+.}-{0:0}:
check_prev_add+0x90/0xc60
__lock_acquire+0x1998/0x2590
lock_acquire+0xd8/0x2d0
__flush_work+0x74/0x530
__cancel_work_timer+0x14f/0x1f0
intel_guc_submission_reset_prepare+0x81/0x4b0 [i915]
intel_uc_reset_prepare+0x9c/0x120 [i915]
reset_prepare+0x21/0x60 [i915]
intel_gt_reset+0x1dd/0x470 [i915]
intel_gt_reset_global+0xfb/0x170 [i915]
intel_gt_handle_error+0x368/0x420 [i915]
intel_gt_debugfs_reset_store+0x5c/0xc0 [i915]
i915_wedged_set+0x29/0x40 [i915]
simple_attr_write_xsigned.constprop.0+0xb4/0x110
full_proxy_write+0x52/0x80
vfs_write+0xc5/0x4f0
ksys_write+0x64/0xe0
do_syscall_64+0x3c/0x90
entry_SYSCALL_64_after_hwframe+0x72/0xdc
other info that might help us debug this:
Chain exists of:
(work_completion)(&(&guc->timestamp.work)->work) --> fs_reclaim --> >->reset.mutex
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(>->reset.mutex);
lock(fs_reclaim);
lock(>->reset.mutex);
lock((work_completion)(&(&guc->timestamp.work)->work));
*** DEADLOCK ***
3 locks held by kms_pipe_crc_ba/6415:
#0: ffff888101541430 (sb_writers#15){.+.+}-{0:0}, at: ksys_write+0x64/0xe0
#1: ffff888136c7eab8 (&attr->mutex){+.+.}-{3:3}, at: simple_attr_write_xsigned.constprop.0+0x47/0x110
#2: ffff88813e6cce90 (>->reset.mutex){+.+.}-{3:3}, at: intel_gt_reset+0x19e/0x470 [i915]
Signed-off-by: Zhanjun Dong <zhanjun.dong@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index a0e3ef1c65d2..22390704542e 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1359,7 +1359,7 @@ static void guc_enable_busyness_worker(struct intel_guc *guc)
static void guc_cancel_busyness_worker(struct intel_guc *guc)
{
- cancel_delayed_work_sync(&guc->timestamp.work);
+ cancel_delayed_work(&guc->timestamp.work);
}
static void __reset_guc_busyness_stats(struct intel_guc *guc)
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Avoid circular locking dependency when flush delayed work on gt reset
2023-06-06 3:00 ` [Intel-gfx] " Zhanjun Dong
(?)
@ 2023-06-06 3:47 ` Patchwork
-1 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2023-06-06 3:47 UTC (permalink / raw)
To: Zhanjun Dong; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Avoid circular locking dependency when flush delayed work on gt reset
URL : https://patchwork.freedesktop.org/series/118898/
State : warning
== Summary ==
Error: dim checkpatch failed
0f88e7218ac1 drm/i915: Avoid circular locking dependency when flush delayed work on gt reset
-:7: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#7:
This attemps to avoid circular locing dependency between flush delayed work and intel_gt_reset.
-:7: WARNING:TYPO_SPELLING: 'attemps' may be misspelled - perhaps 'attempts'?
#7:
This attemps to avoid circular locing dependency between flush delayed work and intel_gt_reset.
^^^^^^^
total: 0 errors, 2 warnings, 0 checks, 8 lines checked
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Avoid circular locking dependency when flush delayed work on gt reset
2023-06-06 3:00 ` [Intel-gfx] " Zhanjun Dong
(?)
(?)
@ 2023-06-06 4:04 ` Patchwork
-1 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2023-06-06 4:04 UTC (permalink / raw)
To: Zhanjun Dong; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 11581 bytes --]
== Series Details ==
Series: drm/i915: Avoid circular locking dependency when flush delayed work on gt reset
URL : https://patchwork.freedesktop.org/series/118898/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_13234 -> Patchwork_118898v1
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_118898v1 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_118898v1, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/index.html
Participating hosts (38 -> 36)
------------------------------
Missing (2): fi-kbl-soraka fi-snb-2520m
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_118898v1:
### IGT changes ###
#### Possible regressions ####
* igt@i915_suspend@basic-s3-without-i915:
- bat-adlp-6: NOTRUN -> [ABORT][1]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-adlp-6/igt@i915_suspend@basic-s3-without-i915.html
#### Suppressed ####
The following results come from untrusted machines, tests, or statuses.
They do not affect the overall result.
* igt@kms_force_connector_basic@prune-stale-modes:
- {bat-adlp-11}: [SKIP][2] ([i915#4093]) -> [FAIL][3]
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13234/bat-adlp-11/igt@kms_force_connector_basic@prune-stale-modes.html
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-adlp-11/igt@kms_force_connector_basic@prune-stale-modes.html
Known issues
------------
Here are the changes found in Patchwork_118898v1 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_lmem_swapping@basic:
- bat-adln-1: NOTRUN -> [SKIP][4] ([i915#4613]) +3 similar issues
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-adln-1/igt@gem_lmem_swapping@basic.html
* igt@gem_lmem_swapping@parallel-random-engines:
- bat-rplp-1: NOTRUN -> [SKIP][5] ([i915#4613]) +3 similar issues
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-rplp-1/igt@gem_lmem_swapping@parallel-random-engines.html
* igt@gem_lmem_swapping@random-engines:
- bat-adlp-6: NOTRUN -> [SKIP][6] ([i915#4613]) +3 similar issues
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-adlp-6/igt@gem_lmem_swapping@random-engines.html
* igt@i915_pm_rps@basic-api:
- bat-adlp-6: NOTRUN -> [SKIP][7] ([i915#6621])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-adlp-6/igt@i915_pm_rps@basic-api.html
- bat-rplp-1: NOTRUN -> [SKIP][8] ([i915#6621])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-rplp-1/igt@i915_pm_rps@basic-api.html
- bat-adln-1: NOTRUN -> [SKIP][9] ([i915#6621])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-adln-1/igt@i915_pm_rps@basic-api.html
* igt@i915_selftest@live@gt_heartbeat:
- fi-apl-guc: [PASS][10] -> [DMESG-FAIL][11] ([i915#5334])
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13234/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@live@migrate:
- bat-dg2-11: [PASS][12] -> [DMESG-WARN][13] ([i915#7699])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13234/bat-dg2-11/igt@i915_selftest@live@migrate.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-dg2-11/igt@i915_selftest@live@migrate.html
* igt@i915_selftest@live@requests:
- bat-rpls-1: [PASS][14] -> [ABORT][15] ([i915#4983] / [i915#7911] / [i915#7920])
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13234/bat-rpls-1/igt@i915_selftest@live@requests.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-rpls-1/igt@i915_selftest@live@requests.html
* igt@i915_selftest@live@slpc:
- bat-rpls-2: NOTRUN -> [DMESG-WARN][16] ([i915#6367])
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-rpls-2/igt@i915_selftest@live@slpc.html
* igt@i915_suspend@basic-s2idle-without-i915:
- bat-rpls-2: NOTRUN -> [ABORT][17] ([i915#6687])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-rpls-2/igt@i915_suspend@basic-s2idle-without-i915.html
* igt@kms_chamelium_hpd@common-hpd-after-suspend:
- bat-rplp-1: NOTRUN -> [SKIP][18] ([i915#7828])
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-rplp-1/igt@kms_chamelium_hpd@common-hpd-after-suspend.html
- bat-adln-1: NOTRUN -> [SKIP][19] ([i915#7828])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-adln-1/igt@kms_chamelium_hpd@common-hpd-after-suspend.html
* igt@kms_setmode@basic-clone-single-crtc:
- bat-adlp-6: NOTRUN -> [SKIP][20] ([i915#3555] / [i915#4579])
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-adlp-6/igt@kms_setmode@basic-clone-single-crtc.html
- bat-adln-1: NOTRUN -> [SKIP][21] ([i915#3555] / [i915#4579])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-adln-1/igt@kms_setmode@basic-clone-single-crtc.html
* igt@prime_vgem@basic-fence-read:
- bat-adlp-6: NOTRUN -> [SKIP][22] ([fdo#109295] / [i915#3291] / [i915#3708]) +2 similar issues
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-adlp-6/igt@prime_vgem@basic-fence-read.html
* igt@prime_vgem@basic-read:
- bat-rplp-1: NOTRUN -> [SKIP][23] ([i915#3708]) +2 similar issues
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-rplp-1/igt@prime_vgem@basic-read.html
* igt@prime_vgem@basic-write:
- bat-adln-1: NOTRUN -> [SKIP][24] ([fdo#109295] / [i915#3291]) +2 similar issues
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-adln-1/igt@prime_vgem@basic-write.html
#### Possible fixes ####
* igt@i915_pm_backlight@basic-brightness@edp-1:
- bat-rplp-1: [ABORT][25] ([i915#7077]) -> [PASS][26]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13234/bat-rplp-1/igt@i915_pm_backlight@basic-brightness@edp-1.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-rplp-1/igt@i915_pm_backlight@basic-brightness@edp-1.html
* igt@i915_pm_rpm@basic-pci-d3-state:
- {bat-mtlp-8}: [ABORT][27] ([i915#7953]) -> [PASS][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13234/bat-mtlp-8/igt@i915_pm_rpm@basic-pci-d3-state.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-mtlp-8/igt@i915_pm_rpm@basic-pci-d3-state.html
* igt@i915_selftest@live@reset:
- bat-rpls-2: [ABORT][29] ([i915#4983] / [i915#7461] / [i915#7913] / [i915#7981] / [i915#8347]) -> [PASS][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13234/bat-rpls-2/igt@i915_selftest@live@reset.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-rpls-2/igt@i915_selftest@live@reset.html
* {igt@kms_pipe_crc_basic@compare-crc-sanitycheck-xr24@pipe-d-dp-5}:
- {bat-adlp-11}: [ABORT][31] ([i915#4423]) -> [PASS][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13234/bat-adlp-11/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-xr24@pipe-d-dp-5.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-adlp-11/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-xr24@pipe-d-dp-5.html
* igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-a-edp-1:
- bat-adlp-6: [ABORT][33] ([i915#8434]) -> [PASS][34]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13234/bat-adlp-6/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-a-edp-1.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-adlp-6/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-a-edp-1.html
* igt@kms_psr@sprite_plane_onoff:
- bat-adln-1: [ABORT][35] ([i915#8442]) -> [PASS][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13234/bat-adln-1/igt@kms_psr@sprite_plane_onoff.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/bat-adln-1/igt@kms_psr@sprite_plane_onoff.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
[i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
[i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
[i915#3546]: https://gitlab.freedesktop.org/drm/intel/issues/3546
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
[i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
[i915#4093]: https://gitlab.freedesktop.org/drm/intel/issues/4093
[i915#4423]: https://gitlab.freedesktop.org/drm/intel/issues/4423
[i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
[i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
[i915#6121]: https://gitlab.freedesktop.org/drm/intel/issues/6121
[i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
[i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
[i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
[i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
[i915#6868]: https://gitlab.freedesktop.org/drm/intel/issues/6868
[i915#7077]: https://gitlab.freedesktop.org/drm/intel/issues/7077
[i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461
[i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
[i915#7699]: https://gitlab.freedesktop.org/drm/intel/issues/7699
[i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
[i915#7911]: https://gitlab.freedesktop.org/drm/intel/issues/7911
[i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
[i915#7920]: https://gitlab.freedesktop.org/drm/intel/issues/7920
[i915#7953]: https://gitlab.freedesktop.org/drm/intel/issues/7953
[i915#7981]: https://gitlab.freedesktop.org/drm/intel/issues/7981
[i915#8347]: https://gitlab.freedesktop.org/drm/intel/issues/8347
[i915#8434]: https://gitlab.freedesktop.org/drm/intel/issues/8434
[i915#8442]: https://gitlab.freedesktop.org/drm/intel/issues/8442
Build changes
-------------
* Linux: CI_DRM_13234 -> Patchwork_118898v1
CI-20190529: 20190529
CI_DRM_13234: cb7bb5b791053c0ff10e314d24e6752795283803 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7319: 2e1bcd49944452b5f9516eecee48e1fa3ae6a636 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_118898v1: cb7bb5b791053c0ff10e314d24e6752795283803 @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
fc40d79b75a1 drm/i915: Avoid circular locking dependency when flush delayed work on gt reset
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118898v1/index.html
[-- Attachment #2: Type: text/html, Size: 13079 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Avoid circular locking dependency when flush delayed work on gt reset
2023-06-06 3:00 ` [Intel-gfx] " Zhanjun Dong
` (2 preceding siblings ...)
(?)
@ 2023-06-06 17:53 ` John Harrison
2023-06-07 17:30 ` John Harrison
-1 siblings, 1 reply; 8+ messages in thread
From: John Harrison @ 2023-06-06 17:53 UTC (permalink / raw)
To: Zhanjun Dong, intel-gfx, dri-devel
On 6/5/2023 20:00, Zhanjun Dong wrote:
> This attemps to avoid circular locing dependency between flush delayed work and intel_gt_reset.
locing -> locking
>
> WARNING: possible circular locking dependency detected
> 6.4.0-rc1-drmtip_1340-g31e3463b0edb+ #1 Not tainted
> ------------------------------------------------------
> kms_pipe_crc_ba/6415 is trying to acquire lock:
> ffff88813e6cc640 ((work_completion)(&(&guc->timestamp.work)->work)){+.+.}-{0:0}, at: __flush_work+0x42/0x530
>
> but task is already holding lock:
> ffff88813e6cce90 (>->reset.mutex){+.+.}-{3:3}, at: intel_gt_reset+0x19e/0x470 [i915]
>
> which lock already depends on the new lock.
>
> the existing dependency chain (in reverse order) is:
>
> -> #3 (>->reset.mutex){+.+.}-{3:3}:
> lock_acquire+0xd8/0x2d0
> i915_gem_shrinker_taints_mutex+0x31/0x50 [i915]
> intel_gt_init_reset+0x65/0x80 [i915]
> intel_gt_common_init_early+0xe1/0x170 [i915]
> intel_root_gt_init_early+0x48/0x60 [i915]
> i915_driver_probe+0x671/0xcb0 [i915]
> i915_pci_probe+0xdc/0x210 [i915]
> pci_device_probe+0x95/0x120
> really_probe+0x164/0x3c0
> __driver_probe_device+0x73/0x160
> driver_probe_device+0x19/0xa0
> __driver_attach+0xb6/0x180
> bus_for_each_dev+0x77/0xd0
> bus_add_driver+0x114/0x210
> driver_register+0x5b/0x110
> __pfx_vgem_open+0x3/0x10 [vgem]
> do_one_initcall+0x57/0x270
> do_init_module+0x5f/0x220
> load_module+0x1ca4/0x1f00
> __do_sys_finit_module+0xb4/0x130
> do_syscall_64+0x3c/0x90
> entry_SYSCALL_64_after_hwframe+0x72/0xdc
>
> -> #2 (fs_reclaim){+.+.}-{0:0}:
> lock_acquire+0xd8/0x2d0
> fs_reclaim_acquire+0xac/0xe0
> kmem_cache_alloc+0x32/0x260
> i915_vma_instance+0xb2/0xc60 [i915]
> i915_gem_object_ggtt_pin_ww+0x175/0x370 [i915]
> vm_fault_gtt+0x22d/0xf60 [i915]
> __do_fault+0x2f/0x1d0
> do_pte_missing+0x4a/0xd20
> __handle_mm_fault+0x5b0/0x790
> handle_mm_fault+0xa2/0x230
> do_user_addr_fault+0x3ea/0xa10
> exc_page_fault+0x68/0x1a0
> asm_exc_page_fault+0x26/0x30
>
> -> #1 (>->reset.backoff_srcu){++++}-{0:0}:
> lock_acquire+0xd8/0x2d0
> _intel_gt_reset_lock+0x57/0x330 [i915]
> guc_timestamp_ping+0x35/0x130 [i915]
> process_one_work+0x250/0x510
> worker_thread+0x4f/0x3a0
> kthread+0xff/0x130
> ret_from_fork+0x29/0x50
>
> -> #0 ((work_completion)(&(&guc->timestamp.work)->work)){+.+.}-{0:0}:
> check_prev_add+0x90/0xc60
> __lock_acquire+0x1998/0x2590
> lock_acquire+0xd8/0x2d0
> __flush_work+0x74/0x530
> __cancel_work_timer+0x14f/0x1f0
> intel_guc_submission_reset_prepare+0x81/0x4b0 [i915]
> intel_uc_reset_prepare+0x9c/0x120 [i915]
> reset_prepare+0x21/0x60 [i915]
> intel_gt_reset+0x1dd/0x470 [i915]
> intel_gt_reset_global+0xfb/0x170 [i915]
> intel_gt_handle_error+0x368/0x420 [i915]
> intel_gt_debugfs_reset_store+0x5c/0xc0 [i915]
> i915_wedged_set+0x29/0x40 [i915]
> simple_attr_write_xsigned.constprop.0+0xb4/0x110
> full_proxy_write+0x52/0x80
> vfs_write+0xc5/0x4f0
> ksys_write+0x64/0xe0
> do_syscall_64+0x3c/0x90
> entry_SYSCALL_64_after_hwframe+0x72/0xdc
>
> other info that might help us debug this:
> Chain exists of:
> (work_completion)(&(&guc->timestamp.work)->work) --> fs_reclaim --> >->reset.mutex
> Possible unsafe locking scenario:
> CPU0 CPU1
> ---- ----
> lock(>->reset.mutex);
> lock(fs_reclaim);
> lock(>->reset.mutex);
> lock((work_completion)(&(&guc->timestamp.work)->work));
>
> *** DEADLOCK ***
> 3 locks held by kms_pipe_crc_ba/6415:
> #0: ffff888101541430 (sb_writers#15){.+.+}-{0:0}, at: ksys_write+0x64/0xe0
> #1: ffff888136c7eab8 (&attr->mutex){+.+.}-{3:3}, at: simple_attr_write_xsigned.constprop.0+0x47/0x110
> #2: ffff88813e6cce90 (>->reset.mutex){+.+.}-{3:3}, at: intel_gt_reset+0x19e/0x470 [i915]
>
> Signed-off-by: Zhanjun Dong <zhanjun.dong@intel.com>
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index a0e3ef1c65d2..22390704542e 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1359,7 +1359,7 @@ static void guc_enable_busyness_worker(struct intel_guc *guc)
>
> static void guc_cancel_busyness_worker(struct intel_guc *guc)
> {
> - cancel_delayed_work_sync(&guc->timestamp.work);
> + cancel_delayed_work(&guc->timestamp.work);
I think it is worth adding a comment here to explain that it is safe to
call the non _sync variant (because of the trylock code in the worker
itself) and that the _sync variant hits circular mutex lock issues.
John.
> }
>
> static void __reset_guc_busyness_stats(struct intel_guc *guc)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Avoid circular locking dependency when flush delayed work on gt reset
2023-06-06 3:00 ` [Intel-gfx] " Zhanjun Dong
` (3 preceding siblings ...)
(?)
@ 2023-06-07 13:57 ` Andrzej Hajda
-1 siblings, 0 replies; 8+ messages in thread
From: Andrzej Hajda @ 2023-06-07 13:57 UTC (permalink / raw)
To: Zhanjun Dong, intel-gfx, dri-devel
On 06.06.2023 05:00, Zhanjun Dong wrote:
> This attemps to avoid circular locing dependency between flush delayed work and intel_gt_reset.
>
> WARNING: possible circular locking dependency detected
> 6.4.0-rc1-drmtip_1340-g31e3463b0edb+ #1 Not tainted
> ------------------------------------------------------
> kms_pipe_crc_ba/6415 is trying to acquire lock:
> ffff88813e6cc640 ((work_completion)(&(&guc->timestamp.work)->work)){+.+.}-{0:0}, at: __flush_work+0x42/0x530
>
> but task is already holding lock:
> ffff88813e6cce90 (>->reset.mutex){+.+.}-{3:3}, at: intel_gt_reset+0x19e/0x470 [i915]
>
> which lock already depends on the new lock.
>
> the existing dependency chain (in reverse order) is:
>
> -> #3 (>->reset.mutex){+.+.}-{3:3}:
> lock_acquire+0xd8/0x2d0
> i915_gem_shrinker_taints_mutex+0x31/0x50 [i915]
> intel_gt_init_reset+0x65/0x80 [i915]
> intel_gt_common_init_early+0xe1/0x170 [i915]
> intel_root_gt_init_early+0x48/0x60 [i915]
> i915_driver_probe+0x671/0xcb0 [i915]
> i915_pci_probe+0xdc/0x210 [i915]
> pci_device_probe+0x95/0x120
> really_probe+0x164/0x3c0
> __driver_probe_device+0x73/0x160
> driver_probe_device+0x19/0xa0
> __driver_attach+0xb6/0x180
> bus_for_each_dev+0x77/0xd0
> bus_add_driver+0x114/0x210
> driver_register+0x5b/0x110
> __pfx_vgem_open+0x3/0x10 [vgem]
> do_one_initcall+0x57/0x270
> do_init_module+0x5f/0x220
> load_module+0x1ca4/0x1f00
> __do_sys_finit_module+0xb4/0x130
> do_syscall_64+0x3c/0x90
> entry_SYSCALL_64_after_hwframe+0x72/0xdc
>
> -> #2 (fs_reclaim){+.+.}-{0:0}:
> lock_acquire+0xd8/0x2d0
> fs_reclaim_acquire+0xac/0xe0
> kmem_cache_alloc+0x32/0x260
> i915_vma_instance+0xb2/0xc60 [i915]
> i915_gem_object_ggtt_pin_ww+0x175/0x370 [i915]
> vm_fault_gtt+0x22d/0xf60 [i915]
> __do_fault+0x2f/0x1d0
> do_pte_missing+0x4a/0xd20
> __handle_mm_fault+0x5b0/0x790
> handle_mm_fault+0xa2/0x230
> do_user_addr_fault+0x3ea/0xa10
> exc_page_fault+0x68/0x1a0
> asm_exc_page_fault+0x26/0x30
>
> -> #1 (>->reset.backoff_srcu){++++}-{0:0}:
> lock_acquire+0xd8/0x2d0
> _intel_gt_reset_lock+0x57/0x330 [i915]
> guc_timestamp_ping+0x35/0x130 [i915]
> process_one_work+0x250/0x510
> worker_thread+0x4f/0x3a0
> kthread+0xff/0x130
> ret_from_fork+0x29/0x50
>
> -> #0 ((work_completion)(&(&guc->timestamp.work)->work)){+.+.}-{0:0}:
> check_prev_add+0x90/0xc60
> __lock_acquire+0x1998/0x2590
> lock_acquire+0xd8/0x2d0
> __flush_work+0x74/0x530
> __cancel_work_timer+0x14f/0x1f0
> intel_guc_submission_reset_prepare+0x81/0x4b0 [i915]
> intel_uc_reset_prepare+0x9c/0x120 [i915]
> reset_prepare+0x21/0x60 [i915]
> intel_gt_reset+0x1dd/0x470 [i915]
> intel_gt_reset_global+0xfb/0x170 [i915]
> intel_gt_handle_error+0x368/0x420 [i915]
> intel_gt_debugfs_reset_store+0x5c/0xc0 [i915]
> i915_wedged_set+0x29/0x40 [i915]
> simple_attr_write_xsigned.constprop.0+0xb4/0x110
> full_proxy_write+0x52/0x80
> vfs_write+0xc5/0x4f0
> ksys_write+0x64/0xe0
> do_syscall_64+0x3c/0x90
> entry_SYSCALL_64_after_hwframe+0x72/0xdc
>
> other info that might help us debug this:
> Chain exists of:
> (work_completion)(&(&guc->timestamp.work)->work) --> fs_reclaim --> >->reset.mutex
> Possible unsafe locking scenario:
> CPU0 CPU1
> ---- ----
> lock(>->reset.mutex);
> lock(fs_reclaim);
> lock(>->reset.mutex);
> lock((work_completion)(&(&guc->timestamp.work)->work));
>
> *** DEADLOCK ***
> 3 locks held by kms_pipe_crc_ba/6415:
> #0: ffff888101541430 (sb_writers#15){.+.+}-{0:0}, at: ksys_write+0x64/0xe0
> #1: ffff888136c7eab8 (&attr->mutex){+.+.}-{3:3}, at: simple_attr_write_xsigned.constprop.0+0x47/0x110
> #2: ffff88813e6cce90 (>->reset.mutex){+.+.}-{3:3}, at: intel_gt_reset+0x19e/0x470 [i915]
>
> Signed-off-by: Zhanjun Dong <zhanjun.dong@intel.com>
This unlocks multiple machines on CI, thx.
Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
Regards
Andrzej
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index a0e3ef1c65d2..22390704542e 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1359,7 +1359,7 @@ static void guc_enable_busyness_worker(struct intel_guc *guc)
>
> static void guc_cancel_busyness_worker(struct intel_guc *guc)
> {
> - cancel_delayed_work_sync(&guc->timestamp.work);
> + cancel_delayed_work(&guc->timestamp.work);
> }
>
> static void __reset_guc_busyness_stats(struct intel_guc *guc)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Avoid circular locking dependency when flush delayed work on gt reset
2023-06-06 17:53 ` [Intel-gfx] [PATCH] " John Harrison
@ 2023-06-07 17:30 ` John Harrison
0 siblings, 0 replies; 8+ messages in thread
From: John Harrison @ 2023-06-07 17:30 UTC (permalink / raw)
To: Zhanjun Dong, intel-gfx, dri-devel
On 6/6/2023 10:53, John Harrison wrote:
> On 6/5/2023 20:00, Zhanjun Dong wrote:
>> This attemps to avoid circular locing dependency between flush
>> delayed work and intel_gt_reset.
> locing -> locking
>
>
>>
>> WARNING: possible circular locking dependency detected
>> 6.4.0-rc1-drmtip_1340-g31e3463b0edb+ #1 Not tainted
>> ------------------------------------------------------
>> kms_pipe_crc_ba/6415 is trying to acquire lock:
>> ffff88813e6cc640
>> ((work_completion)(&(&guc->timestamp.work)->work)){+.+.}-{0:0}, at:
>> __flush_work+0x42/0x530
>>
>> but task is already holding lock:
>> ffff88813e6cce90 (>->reset.mutex){+.+.}-{3:3}, at:
>> intel_gt_reset+0x19e/0x470 [i915]
>>
>> which lock already depends on the new lock.
>>
>> the existing dependency chain (in reverse order) is:
>>
>> -> #3 (>->reset.mutex){+.+.}-{3:3}:
>> lock_acquire+0xd8/0x2d0
>> i915_gem_shrinker_taints_mutex+0x31/0x50 [i915]
>> intel_gt_init_reset+0x65/0x80 [i915]
>> intel_gt_common_init_early+0xe1/0x170 [i915]
>> intel_root_gt_init_early+0x48/0x60 [i915]
>> i915_driver_probe+0x671/0xcb0 [i915]
>> i915_pci_probe+0xdc/0x210 [i915]
>> pci_device_probe+0x95/0x120
>> really_probe+0x164/0x3c0
>> __driver_probe_device+0x73/0x160
>> driver_probe_device+0x19/0xa0
>> __driver_attach+0xb6/0x180
>> bus_for_each_dev+0x77/0xd0
>> bus_add_driver+0x114/0x210
>> driver_register+0x5b/0x110
>> __pfx_vgem_open+0x3/0x10 [vgem]
>> do_one_initcall+0x57/0x270
>> do_init_module+0x5f/0x220
>> load_module+0x1ca4/0x1f00
>> __do_sys_finit_module+0xb4/0x130
>> do_syscall_64+0x3c/0x90
>> entry_SYSCALL_64_after_hwframe+0x72/0xdc
>>
>> -> #2 (fs_reclaim){+.+.}-{0:0}:
>> lock_acquire+0xd8/0x2d0
>> fs_reclaim_acquire+0xac/0xe0
>> kmem_cache_alloc+0x32/0x260
>> i915_vma_instance+0xb2/0xc60 [i915]
>> i915_gem_object_ggtt_pin_ww+0x175/0x370 [i915]
>> vm_fault_gtt+0x22d/0xf60 [i915]
>> __do_fault+0x2f/0x1d0
>> do_pte_missing+0x4a/0xd20
>> __handle_mm_fault+0x5b0/0x790
>> handle_mm_fault+0xa2/0x230
>> do_user_addr_fault+0x3ea/0xa10
>> exc_page_fault+0x68/0x1a0
>> asm_exc_page_fault+0x26/0x30
>>
>> -> #1 (>->reset.backoff_srcu){++++}-{0:0}:
>> lock_acquire+0xd8/0x2d0
>> _intel_gt_reset_lock+0x57/0x330 [i915]
>> guc_timestamp_ping+0x35/0x130 [i915]
>> process_one_work+0x250/0x510
>> worker_thread+0x4f/0x3a0
>> kthread+0xff/0x130
>> ret_from_fork+0x29/0x50
>>
>> -> #0 ((work_completion)(&(&guc->timestamp.work)->work)){+.+.}-{0:0}:
>> check_prev_add+0x90/0xc60
>> __lock_acquire+0x1998/0x2590
>> lock_acquire+0xd8/0x2d0
>> __flush_work+0x74/0x530
>> __cancel_work_timer+0x14f/0x1f0
>> intel_guc_submission_reset_prepare+0x81/0x4b0 [i915]
>> intel_uc_reset_prepare+0x9c/0x120 [i915]
>> reset_prepare+0x21/0x60 [i915]
>> intel_gt_reset+0x1dd/0x470 [i915]
>> intel_gt_reset_global+0xfb/0x170 [i915]
>> intel_gt_handle_error+0x368/0x420 [i915]
>> intel_gt_debugfs_reset_store+0x5c/0xc0 [i915]
>> i915_wedged_set+0x29/0x40 [i915]
>> simple_attr_write_xsigned.constprop.0+0xb4/0x110
>> full_proxy_write+0x52/0x80
>> vfs_write+0xc5/0x4f0
>> ksys_write+0x64/0xe0
>> do_syscall_64+0x3c/0x90
>> entry_SYSCALL_64_after_hwframe+0x72/0xdc
>>
>> other info that might help us debug this:
>> Chain exists of:
>> (work_completion)(&(&guc->timestamp.work)->work) --> fs_reclaim -->
>> >->reset.mutex
>> Possible unsafe locking scenario:
>> CPU0 CPU1
>> ---- ----
>> lock(>->reset.mutex);
>> lock(fs_reclaim);
>> lock(>->reset.mutex);
>> lock((work_completion)(&(&guc->timestamp.work)->work));
>>
>> *** DEADLOCK ***
>> 3 locks held by kms_pipe_crc_ba/6415:
>> #0: ffff888101541430 (sb_writers#15){.+.+}-{0:0}, at:
>> ksys_write+0x64/0xe0
>> #1: ffff888136c7eab8 (&attr->mutex){+.+.}-{3:3}, at:
>> simple_attr_write_xsigned.constprop.0+0x47/0x110
>> #2: ffff88813e6cce90 (>->reset.mutex){+.+.}-{3:3}, at:
>> intel_gt_reset+0x19e/0x470 [i915]
>>
>> Signed-off-by: Zhanjun Dong <zhanjun.dong@intel.com>
>> ---
>> drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> index a0e3ef1c65d2..22390704542e 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> @@ -1359,7 +1359,7 @@ static void guc_enable_busyness_worker(struct
>> intel_guc *guc)
>> static void guc_cancel_busyness_worker(struct intel_guc *guc)
>> {
>> - cancel_delayed_work_sync(&guc->timestamp.work);
>> + cancel_delayed_work(&guc->timestamp.work);
> I think it is worth adding a comment here to explain that it is safe
> to call the non _sync variant (because of the trylock code in the
> worker itself) and that the _sync variant hits circular mutex lock
> issues.
>
To record the notes from direct discussion... This function needs to
take a 'bool sync' flag. The park/fini code paths still need to do a
synchronous flush to ensure that the worker is not still running during
suspend or shutdown. Only the reset path should do the asynchronous cancel.
John.
> John.
>
>
>> }
>> static void __reset_guc_busyness_stats(struct intel_guc *guc)
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] drm/i915: Avoid circular locking dependency when flush delayed work on gt reset
@ 2023-06-07 19:03 Zhanjun Dong
0 siblings, 0 replies; 8+ messages in thread
From: Zhanjun Dong @ 2023-06-07 19:03 UTC (permalink / raw)
To: intel-gfx, dri-devel; +Cc: Zhanjun Dong
This attempts to avoid circular locking dependency between flush delayed work and intel_gt_reset.
Switched from cancel_delayed_work_sync to cancel_delayed_work, the non-sync version for reset path, it is safe as the worker has the trylock code to handle the lock; Meanwhile keep the sync version for park/fini to ensure the worker is not still running during suspend or shutdown.
WARNING: possible circular locking dependency detected
6.4.0-rc1-drmtip_1340-g31e3463b0edb+ #1 Not tainted
------------------------------------------------------
kms_pipe_crc_ba/6415 is trying to acquire lock:
ffff88813e6cc640 ((work_completion)(&(&guc->timestamp.work)->work)){+.+.}-{0:0}, at: __flush_work+0x42/0x530
but task is already holding lock:
ffff88813e6cce90 (>->reset.mutex){+.+.}-{3:3}, at: intel_gt_reset+0x19e/0x470 [i915]
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #3 (>->reset.mutex){+.+.}-{3:3}:
lock_acquire+0xd8/0x2d0
i915_gem_shrinker_taints_mutex+0x31/0x50 [i915]
intel_gt_init_reset+0x65/0x80 [i915]
intel_gt_common_init_early+0xe1/0x170 [i915]
intel_root_gt_init_early+0x48/0x60 [i915]
i915_driver_probe+0x671/0xcb0 [i915]
i915_pci_probe+0xdc/0x210 [i915]
pci_device_probe+0x95/0x120
really_probe+0x164/0x3c0
__driver_probe_device+0x73/0x160
driver_probe_device+0x19/0xa0
__driver_attach+0xb6/0x180
bus_for_each_dev+0x77/0xd0
bus_add_driver+0x114/0x210
driver_register+0x5b/0x110
__pfx_vgem_open+0x3/0x10 [vgem]
do_one_initcall+0x57/0x270
do_init_module+0x5f/0x220
load_module+0x1ca4/0x1f00
__do_sys_finit_module+0xb4/0x130
do_syscall_64+0x3c/0x90
entry_SYSCALL_64_after_hwframe+0x72/0xdc
-> #2 (fs_reclaim){+.+.}-{0:0}:
lock_acquire+0xd8/0x2d0
fs_reclaim_acquire+0xac/0xe0
kmem_cache_alloc+0x32/0x260
i915_vma_instance+0xb2/0xc60 [i915]
i915_gem_object_ggtt_pin_ww+0x175/0x370 [i915]
vm_fault_gtt+0x22d/0xf60 [i915]
__do_fault+0x2f/0x1d0
do_pte_missing+0x4a/0xd20
__handle_mm_fault+0x5b0/0x790
handle_mm_fault+0xa2/0x230
do_user_addr_fault+0x3ea/0xa10
exc_page_fault+0x68/0x1a0
asm_exc_page_fault+0x26/0x30
-> #1 (>->reset.backoff_srcu){++++}-{0:0}:
lock_acquire+0xd8/0x2d0
_intel_gt_reset_lock+0x57/0x330 [i915]
guc_timestamp_ping+0x35/0x130 [i915]
process_one_work+0x250/0x510
worker_thread+0x4f/0x3a0
kthread+0xff/0x130
ret_from_fork+0x29/0x50
-> #0 ((work_completion)(&(&guc->timestamp.work)->work)){+.+.}-{0:0}:
check_prev_add+0x90/0xc60
__lock_acquire+0x1998/0x2590
lock_acquire+0xd8/0x2d0
__flush_work+0x74/0x530
__cancel_work_timer+0x14f/0x1f0
intel_guc_submission_reset_prepare+0x81/0x4b0 [i915]
intel_uc_reset_prepare+0x9c/0x120 [i915]
reset_prepare+0x21/0x60 [i915]
intel_gt_reset+0x1dd/0x470 [i915]
intel_gt_reset_global+0xfb/0x170 [i915]
intel_gt_handle_error+0x368/0x420 [i915]
intel_gt_debugfs_reset_store+0x5c/0xc0 [i915]
i915_wedged_set+0x29/0x40 [i915]
simple_attr_write_xsigned.constprop.0+0xb4/0x110
full_proxy_write+0x52/0x80
vfs_write+0xc5/0x4f0
ksys_write+0x64/0xe0
do_syscall_64+0x3c/0x90
entry_SYSCALL_64_after_hwframe+0x72/0xdc
other info that might help us debug this:
Chain exists of:
(work_completion)(&(&guc->timestamp.work)->work) --> fs_reclaim --> >->reset.mutex
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(>->reset.mutex);
lock(fs_reclaim);
lock(>->reset.mutex);
lock((work_completion)(&(&guc->timestamp.work)->work));
*** DEADLOCK ***
3 locks held by kms_pipe_crc_ba/6415:
#0: ffff888101541430 (sb_writers#15){.+.+}-{0:0}, at: ksys_write+0x64/0xe0
#1: ffff888136c7eab8 (&attr->mutex){+.+.}-{3:3}, at: simple_attr_write_xsigned.constprop.0+0x47/0x110
#2: ffff88813e6cce90 (>->reset.mutex){+.+.}-{3:3}, at: intel_gt_reset+0x19e/0x470 [i915]
Signed-off-by: Zhanjun Dong <zhanjun.dong@intel.com>
---
drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
index a0e3ef1c65d2..cca6960d3490 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
@@ -1357,9 +1357,12 @@ static void guc_enable_busyness_worker(struct intel_guc *guc)
mod_delayed_work(system_highpri_wq, &guc->timestamp.work, guc->timestamp.ping_delay);
}
-static void guc_cancel_busyness_worker(struct intel_guc *guc)
+static void guc_cancel_busyness_worker(struct intel_guc *guc, bool sync)
{
- cancel_delayed_work_sync(&guc->timestamp.work);
+ if (sync)
+ cancel_delayed_work_sync(&guc->timestamp.work);
+ else
+ cancel_delayed_work(&guc->timestamp.work);
}
static void __reset_guc_busyness_stats(struct intel_guc *guc)
@@ -1370,7 +1373,7 @@ static void __reset_guc_busyness_stats(struct intel_guc *guc)
unsigned long flags;
ktime_t unused;
- guc_cancel_busyness_worker(guc);
+ guc_cancel_busyness_worker(guc, 0);
spin_lock_irqsave(&guc->timestamp.lock, flags);
@@ -1485,7 +1488,7 @@ static int guc_init_engine_stats(struct intel_guc *guc)
static void guc_fini_engine_stats(struct intel_guc *guc)
{
- guc_cancel_busyness_worker(guc);
+ guc_cancel_busyness_worker(guc, 1);
}
void intel_guc_busyness_park(struct intel_gt *gt)
@@ -1500,7 +1503,7 @@ void intel_guc_busyness_park(struct intel_gt *gt)
* and causes an unclaimed register access warning. Cancel the worker
* synchronously here.
*/
- guc_cancel_busyness_worker(guc);
+ guc_cancel_busyness_worker(guc, 1);
/*
* Before parking, we should sample engine busyness stats if we need to.
@@ -4503,7 +4506,7 @@ int intel_guc_submission_enable(struct intel_guc *guc)
/* Note: By the time we're here, GuC may have already been reset */
void intel_guc_submission_disable(struct intel_guc *guc)
{
- guc_cancel_busyness_worker(guc);
+ guc_cancel_busyness_worker(guc, 0);
/* Semaphore interrupt disable and route to host */
guc_route_semaphores(guc, false);
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-06-07 19:03 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-06 3:00 [PATCH] drm/i915: Avoid circular locking dependency when flush delayed work on gt reset Zhanjun Dong
2023-06-06 3:00 ` [Intel-gfx] " Zhanjun Dong
2023-06-06 3:47 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2023-06-06 4:04 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2023-06-06 17:53 ` [Intel-gfx] [PATCH] " John Harrison
2023-06-07 17:30 ` John Harrison
2023-06-07 13:57 ` Andrzej Hajda
2023-06-07 19:03 Zhanjun Dong
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.