Skip to content

Commit d0b9a9a

Browse files
committed
dma-fence: prime lockdep annotations
Two in one go: - it is allowed to call dma_fence_wait() while holding a dma_resv_lock(). This is fundamental to how eviction works with ttm, so required. - it is allowed to call dma_fence_wait() from memory reclaim contexts, specifically from shrinker callbacks (which i915 does), and from mmu notifier callbacks (which amdgpu does, and which i915 sometimes also does, and probably always should, but that's kinda a debate). Also for stuff like HMM we really need to be able to do this, or things get real dicey. Consequence is that any critical path necessary to get to a dma_fence_signal for a fence must never a) call dma_resv_lock nor b) allocate memory with GFP_KERNEL. Also by implication of dma_resv_lock(), no userspace faulting allowed. That's some supremely obnoxious limitations, which is why we need to sprinkle the right annotations to all relevant paths. The one big locking context we're leaving out here is mmu notifiers, added in commit 23b6839 Author: Daniel Vetter <[email protected]> Date: Mon Aug 26 22:14:21 2019 +0200 mm/mmu_notifiers: add a lockdep map for invalidate_range_start/end that one covers a lot of other callsites, and it's also allowed to wait on dma-fences from mmu notifiers. But there's no ready-made functions exposed to prime this, so I've left it out for now. v2: Also track against mmu notifier context. v3: kerneldoc to spec the cross-driver contract. Note that currently i915 throws in a hard-coded 10s timeout on foreign fences (not sure why that was done, but it's there), which is why that rule is worded with SHOULD instead of MUST. Also some of the mmu_notifier/shrinker rules might surprise SoC drivers, I haven't fully audited them all. Which is infeasible anyway, we'll need to run them with lockdep and dma-fence annotations and see what goes boom. v4: A spelling fix from Mika v5: #ifdef for CONFIG_MMU_NOTIFIER. Reported by 0day. Unfortunately this means lockdep enforcement is slightly inconsistent, it won't spot GFP_NOIO and GFP_NOFS allocations in the wrong spot if CONFIG_MMU_NOTIFIER is disabled in the kernel config. Oh well. v5: Note that only drivers/gpu has a reasonable (or at least historical) excuse to use dma_fence_wait() from shrinker and mmu notifier callbacks. Everyone else should either have a better memory manager model, or better hardware. This reflects discussions with Jason Gunthorpe. Cc: Jason Gunthorpe <[email protected]> Cc: Felix Kuehling <[email protected]> Cc: kernel test robot <[email protected]> Acked-by: Christian König <[email protected]> Acked-by: Dave Airlie <[email protected]> Reviewed-by: Maarten Lankhorst <[email protected]> Reviewed-by: Thomas Hellström <[email protected]> (v4) Cc: Mika Kuoppala <[email protected]> Cc: Thomas Hellstrom <[email protected]> Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: Chris Wilson <[email protected]> Cc: Maarten Lankhorst <[email protected]> Cc: Christian König <[email protected]> Signed-off-by: Daniel Vetter <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
1 parent 5fbff81 commit d0b9a9a

File tree

4 files changed

+61
-0
lines changed

4 files changed

+61
-0
lines changed

Documentation/driver-api/dma-buf.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,12 @@ DMA Fences
133133
.. kernel-doc:: drivers/dma-buf/dma-fence.c
134134
:doc: DMA fences overview
135135

136+
DMA Fence Cross-Driver Contract
137+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
138+
139+
.. kernel-doc:: drivers/dma-buf/dma-fence.c
140+
:doc: fence cross-driver contract
141+
136142
DMA Fence Signalling Annotations
137143
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
138144

drivers/dma-buf/dma-fence.c

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,52 @@ static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(1);
6464
* &dma_buf.resv pointer.
6565
*/
6666

67+
/**
68+
* DOC: fence cross-driver contract
69+
*
70+
* Since &dma_fence provide a cross driver contract, all drivers must follow the
71+
* same rules:
72+
*
73+
* * Fences must complete in a reasonable time. Fences which represent kernels
74+
* and shaders submitted by userspace, which could run forever, must be backed
75+
* up by timeout and gpu hang recovery code. Minimally that code must prevent
76+
* further command submission and force complete all in-flight fences, e.g.
77+
* when the driver or hardware do not support gpu reset, or if the gpu reset
78+
* failed for some reason. Ideally the driver supports gpu recovery which only
79+
* affects the offending userspace context, and no other userspace
80+
* submissions.
81+
*
82+
* * Drivers may have different ideas of what completion within a reasonable
83+
* time means. Some hang recovery code uses a fixed timeout, others a mix
84+
* between observing forward progress and increasingly strict timeouts.
85+
* Drivers should not try to second guess timeout handling of fences from
86+
* other drivers.
87+
*
88+
* * To ensure there's no deadlocks of dma_fence_wait() against other locks
89+
* drivers should annotate all code required to reach dma_fence_signal(),
90+
* which completes the fences, with dma_fence_begin_signalling() and
91+
* dma_fence_end_signalling().
92+
*
93+
* * Drivers are allowed to call dma_fence_wait() while holding dma_resv_lock().
94+
* This means any code required for fence completion cannot acquire a
95+
* &dma_resv lock. Note that this also pulls in the entire established
96+
* locking hierarchy around dma_resv_lock() and dma_resv_unlock().
97+
*
98+
* * Drivers are allowed to call dma_fence_wait() from their &shrinker
99+
* callbacks. This means any code required for fence completion cannot
100+
* allocate memory with GFP_KERNEL.
101+
*
102+
* * Drivers are allowed to call dma_fence_wait() from their &mmu_notifier
103+
* respectively &mmu_interval_notifier callbacks. This means any code required
104+
* for fence completeion cannot allocate memory with GFP_NOFS or GFP_NOIO.
105+
* Only GFP_ATOMIC is permissible, which might fail.
106+
*
107+
* Note that only GPU drivers have a reasonable excuse for both requiring
108+
* &mmu_interval_notifier and &shrinker callbacks at the same time as having to
109+
* track asynchronous compute work using &dma_fence. No driver outside of
110+
* drivers/gpu should ever call dma_fence_wait() in such contexts.
111+
*/
112+
67113
static const char *dma_fence_stub_get_name(struct dma_fence *fence)
68114
{
69115
return "stub";

drivers/dma-buf/dma-resv.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
#include <linux/export.h>
3737
#include <linux/mm.h>
3838
#include <linux/sched/mm.h>
39+
#include <linux/mmu_notifier.h>
3940

4041
/**
4142
* DOC: Reservation Object Overview
@@ -116,6 +117,13 @@ static int __init dma_resv_lockdep(void)
116117
if (ret == -EDEADLK)
117118
dma_resv_lock_slow(&obj, &ctx);
118119
fs_reclaim_acquire(GFP_KERNEL);
120+
#ifdef CONFIG_MMU_NOTIFIER
121+
lock_map_acquire(&__mmu_notifier_invalidate_range_start_map);
122+
__dma_fence_might_wait();
123+
lock_map_release(&__mmu_notifier_invalidate_range_start_map);
124+
#else
125+
__dma_fence_might_wait();
126+
#endif
119127
fs_reclaim_release(GFP_KERNEL);
120128
ww_mutex_unlock(&obj.lock);
121129
ww_acquire_fini(&ctx);

include/linux/dma-fence.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,7 @@ dma_fence_get_rcu_safe(struct dma_fence __rcu **fencep)
360360
#ifdef CONFIG_LOCKDEP
361361
bool dma_fence_begin_signalling(void);
362362
void dma_fence_end_signalling(bool cookie);
363+
void __dma_fence_might_wait(void);
363364
#else
364365
static inline bool dma_fence_begin_signalling(void)
365366
{

0 commit comments

Comments
 (0)