dma_buf_detach lockdep splat
Christian König
christian.koenig at amd.com
Thu Jun 27 08:25:58 UTC 2024
Am 27.06.24 um 10:04 schrieb Daniel Vetter:
> On Wed, Jun 26, 2024 at 05:58:02PM +0200, Thomas Hellström wrote:
>> Hi!
>>
>> I'm seeing the below lockdep splat 1) with the xe driver in an imported
>> dma-buf object destruction path.
Mhm strange.
>>
>> It's not because we hold the dma_resv lock at that point, but rather
>> because we hold *another* dma_resv lock at that point, and the dma_resv
>> detach happens when the object is idle, in this case it was idle at the
>> final put(), and dma_buf_detach() is called in the putting process.
>>
>> Holding another dma-buf lock might happen as part of
>> drm_exec_unlock_all, or simply if the wider vm dma_resv was held at
>> object put time, so it's not an uncommon pattern, even if the drm_exec
>> instance can be fixed by putting all bos after unlocking them all.
>>
>> Two solutions coming to mind here:
>>
>> 1) Provide a dma_buf_detach_locked()
> This smells way too much like the endless headaches we had with
> drm_gem_object_put_locked and friends against drm_device.struct_mutex. Or
> I'm not understanding what you're doing, because I'm pretty sure you have
> to take the dma_resv lock on final put() of imported objects. Because that
> final put() is of the import wrapper, the exporter (and other importers)
> can still get at that object and so dma_resv_lock is very much needed.
>
> Or it's a completely different final put(), but I have no idea how you get
> that on an imported dma_buf.
>
>> 2) Have TTM always take the delayed delete path for imported dma-buf
>> objects.
>>
>> I'd prefer 1) since I think the correct place to call this is in the
>> TTM callback delete_mem_notify() where the bo is already locked, and I
>> figure non-TTM gem backends may come to suffer from the same problem.
>>
>> Opinions, suggestions?
> Imo 2) or trying to push the object puts outside of the dma_resv_lock.
IIRC I've stumbled over this issue before with TTM but though that I've
fixed it.
I mean no objections from my side to change drm_exec_fini() to something
like this:
drm_exec_for_each_locked_object_reverse(exec, index, obj)
dma_resv_unlock(obj->resv);
drm_exec_for_each_locked_object_reverse(exec, index, obj)
drm_gem_object_put(obj);
but in general that the last reference is dropped while holding a
different reservation object is not something special. For example that
happens all the time in TTMs eviction code.
So at least for TTM I would say we should move cleanup of imported BOs
to the worker. But not sure if that covers everything.
Regards,
Christian.
> The
> latter is imo natural, since usually you grab references, then lock. And
> this even holds for at least the slow path of lru eviction, because you
> need to drop all locks and then do a ww_mutex_lock_slow, and that requires
> that you can hold references to unlocked objects.
>
> But 2) alone is imo fine, dma_buf have become really big objects that go
> across drivers, extremely similar to struct file, and that is doing the
> delayed final put unconditionally since years too, using task_work. It's
> simply a solid design.
>
> Cheers, Sima
>
>> [1]
>> [ 99.136161] ============================================
>> [ 99.136162] WARNING: possible recursive locking detected
>> [ 99.136163] 6.10.0-rc2+ #6 Tainted: G U
>> [ 99.136165] --------------------------------------------
>> [ 99.136166] glxgears:sh0/4675 is trying to acquire lock:
>> [ 99.136167] ffff9967dcdd91a8 (reservation_ww_class_mutex){+.+.}-
>> {3:3}, at: dma_buf_detach+0x3b/0xf0
>> [ 99.136184]
>> but task is already holding lock:
>> [ 99.136186] ffff9967d8c145a8 (reservation_ww_class_mutex){+.+.}-
>> {3:3}, at: drm_exec_lock_obj+0x49/0x2b0 [drm_exec]
>> [ 99.136191]
>> other info that might help us debug this:
>> [ 99.136192] Possible unsafe locking scenario:
>>
>> [ 99.136194] CPU0
>> [ 99.136194] ----
>> [ 99.136195] lock(reservation_ww_class_mutex);
>> [ 99.136197] lock(reservation_ww_class_mutex);
>> [ 99.136199]
>> *** DEADLOCK ***
>>
>> [ 99.136199] May be due to missing lock nesting notation
>>
>> [ 99.136200] 5 locks held by glxgears:sh0/4675:
>> [ 99.136202] #0: ffff9967d8c104c8 (&xef->vm.lock){+.+.}-{3:3}, at:
>> xe_file_close+0xde/0x1c0 [xe]
>> [ 99.136272] #1: ffff9967d5bb7480 (&vm->lock){++++}-{3:3}, at:
>> xe_vm_close_and_put+0x161/0x9b0 [xe]
>> [ 99.136350] #2: ffff9967ef88a970 (&val->lock){.+.+}-{3:3}, at:
>> xe_validation_ctx_init+0x6d/0x70 [xe]
>> [ 99.136440] #3: ffffbd6a085577b8
>> (reservation_ww_class_acquire){+.+.}-{0:0}, at:
>> xe_vma_destroy_unlocked+0x7f/0xe0 [xe]
>> [ 99.136546] #4: ffff9967d8c145a8
>> (reservation_ww_class_mutex){+.+.}-{3:3}, at:
>> drm_exec_lock_obj+0x49/0x2b0 [drm_exec]
>> [ 99.136552]
>> stack backtrace:
>> [ 99.136553] CPU: 10 PID: 4675 Comm: glxgears:sh0 Tainted: G U
>> 6.10.0-rc2+ #6
>> [ 99.136555] Hardware name: ASUS System Product Name/PRIME B560M-A
>> AC, BIOS 2001 02/01/2023
>> [ 99.136557] Call Trace:
>> [ 99.136558] <TASK>
>> [ 99.136560] dump_stack_lvl+0x77/0xb0
>> [ 99.136564] __lock_acquire+0x1232/0x2160
>> [ 99.136569] lock_acquire+0xcb/0x2d0
>> [ 99.136570] ? dma_buf_detach+0x3b/0xf0
>> [ 99.136574] ? __lock_acquire+0x417/0x2160
>> [ 99.136577] __ww_mutex_lock.constprop.0+0xd0/0x13b0
>> [ 99.136580] ? dma_buf_detach+0x3b/0xf0
>> [ 99.136584] ? dma_buf_detach+0x3b/0xf0
>> [ 99.136588] ? ww_mutex_lock+0x2b/0x90
>> [ 99.136590] ww_mutex_lock+0x2b/0x90
>> [ 99.136592] dma_buf_detach+0x3b/0xf0
>> [ 99.136595] drm_prime_gem_destroy+0x2f/0x40 [drm]
>> [ 99.136638] xe_ttm_bo_destroy+0x32/0x220 [xe]
>> [ 99.136734] ? __mutex_unlock_slowpath+0x3a/0x290
>> [ 99.136738] drm_exec_unlock_all+0xa1/0xd0 [drm_exec]
>> [ 99.136741] drm_exec_fini+0x12/0xb0 [drm_exec]
>> [ 99.136743] xe_validation_ctx_fini+0x15/0x40 [xe]
>> [ 99.136848] xe_vma_destroy_unlocked+0xb1/0xe0 [xe]
>> [ 99.136954] xe_vm_close_and_put+0x41a/0x9b0 [xe]
>> [ 99.137056] ? xa_find+0xe3/0x1e0
>> [ 99.137060] xe_file_close+0x10a/0x1c0 [xe]
>> [ 99.137157] drm_file_free+0x22a/0x280 [drm]
>> [ 99.137193] drm_release_noglobal+0x22/0x70 [drm]
>> [ 99.137227] __fput+0xf1/0x2d0
>> [ 99.137231] task_work_run+0x59/0x90
>> [ 99.137235] do_exit+0x330/0xb40
>> [ 99.137238] do_group_exit+0x36/0xa0
>> [ 99.137241] get_signal+0xbd2/0xbe0
>> [ 99.137245] arch_do_signal_or_restart+0x3e/0x240
>> [ 99.137249] syscall_exit_to_user_mode+0x1e7/0x290
>> [ 99.137252] do_syscall_64+0xa1/0x180
>> [ 99.137255] ? _raw_spin_unlock+0x23/0x40
>> [ 99.137257] ? look_up_lock_class+0x6f/0x120
>> [ 99.137261] ? __lock_acquire+0x417/0x2160
>> [ 99.137264] ? lock_acquire+0xcb/0x2d0
>> [ 99.137266] ? __set_task_comm+0x28/0x1e0
>> [ 99.137268] ? find_held_lock+0x2b/0x80
>> [ 99.137271] ? __set_task_comm+0xe1/0x1e0
>> [ 99.137273] ? lock_release+0xca/0x290
>> [ 99.137277] ? __do_sys_prctl+0x245/0xab0
>> [ 99.137279] ? lockdep_hardirqs_on_prepare+0xde/0x190
>> [ 99.137281] ? syscall_exit_to_user_mode+0xb0/0x290
>> [ 99.137284] ? do_syscall_64+0xa1/0x180
>> [ 99.137286] ? cpuset_cpus_allowed+0x36/0x140
>> [ 99.137289] ? find_held_lock+0x2b/0x80
>> [ 99.137291] ? find_held_lock+0x2b/0x80
>> [ 99.137294] ? __sched_setaffinity+0x78/0x240
>> [ 99.137297] ? kfree+0xe2/0x310
>> [ 99.137301] ? kfree+0x202/0x310
>> [ 99.137303] ? __sched_setaffinity+0x78/0x240
>> [ 99.137305] ? __x64_sys_sched_setaffinity+0x69/0xb0
>> [ 99.137307] ? kfree+0xe2/0x310
>> [ 99.137310] ? lockdep_hardirqs_on_prepare+0xde/0x190
>> [ 99.137312] ? syscall_exit_to_user_mode+0xb0/0x290
>> [ 99.137315] ? do_syscall_64+0xa1/0x180
>> [ 99.137317] ? trace_hardirqs_off+0x4b/0xc0
>> [ 99.137321] ? clear_bhb_loop+0x45/0xa0
>> [ 99.137325] ? clear_bhb_loop+0x45/0xa0
>> [ 99.137327] ? clear_bhb_loop+0x45/0xa0
>> [ 99.137330] entry_SYSCALL_64_after_hwframe+0x76/0x7e
>> [ 99.137333] RIP: 0033:0x7fda70ee6169
>> [ 99.137351] Code: Unable to access opcode bytes at 0x7fda70ee613f.
>> [ 99.137352] RSP: 002b:00007fda5fdffc80 EFLAGS: 00000246 ORIG_RAX:
>> 00000000000000ca
>> [ 99.137354] RAX: fffffffffffffe00 RBX: 0000000000000000 RCX:
>> 00007fda70ee6169
>> [ 99.137356] RDX: 0000000000000000 RSI: 0000000000000189 RDI:
>> 0000564a96f45b30
>> [ 99.137358] RBP: 00007fda5fdffcb0 R08: 0000000000000000 R09:
>> 00000000ffffffff
>> [ 99.137359] R10: 0000000000000000 R11: 0000000000000246 R12:
>> 0000000000000000
>> [ 99.137360] R13: 0000000000000000 R14: 0000000000000000 R15:
>> 0000564a96f45b30
>> [ 99.137365] </TASK>
>>
More information about the dri-devel
mailing list