1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
|
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Date: Tue, 7 Jul 2020 12:25:11 +0200
Subject: [PATCH 10/10] drm/i915: Don't disable interrupts and pretend a lock
as been acquired in __timeline_mark_lock().
This is a revert of commits
d67739268cf0e ("drm/i915/gt: Mark up the nested engine-pm timeline lock as irqsafe")
6c69a45445af9 ("drm/i915/gt: Mark context->active_count as protected by timeline->mutex")
The existing code leads to a different behaviour depending on wheather
lockdep is enabled or not. Any following lock that is acquired without
disabling interrupts (but needs to) will not be noticed by lockdep.
This it not just a lockdep annotation but is used but an actual mutex_t
that is properly used as a lock but in case of __timeline_mark_lock()
lockdep is only told that it is acquired but no lock has been acquired.
It appears that its purporse is just satisfy the lockdep_assert_held()
check in intel_context_mark_active(). The other problem with disabling
interrupts is that on PREEMPT_RT interrupts are also disabled which
leads to problems for instance later during memory allocation.
Add an argument to intel_context_mark_active() which is true if the lock
must have been acquired, false if other magic is involved and the lock
is not needed. Use the `false' argument only from within
switch_to_kernel_context() and remove __timeline_mark_lock().
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
drivers/gpu/drm/i915/gt/intel_context.h | 6 ++-
drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c | 2 -
drivers/gpu/drm/i915/gt/intel_engine_pm.c | 38 -----------------------
drivers/gpu/drm/i915/i915_request.c | 7 ++--
drivers/gpu/drm/i915/i915_request.h | 3 +
5 files changed, 12 insertions(+), 44 deletions(-)
--- a/drivers/gpu/drm/i915/gt/intel_context.h
+++ b/drivers/gpu/drm/i915/gt/intel_context.h
@@ -161,9 +161,11 @@ static inline void intel_context_enter(s
ce->ops->enter(ce);
}
-static inline void intel_context_mark_active(struct intel_context *ce)
+static inline void intel_context_mark_active(struct intel_context *ce,
+ bool timeline_mutex_needed)
{
- lockdep_assert_held(&ce->timeline->mutex);
+ if (timeline_mutex_needed)
+ lockdep_assert_held(&ce->timeline->mutex);
++ce->active_count;
}
--- a/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_heartbeat.c
@@ -42,7 +42,7 @@ heartbeat_create(struct intel_context *c
struct i915_request *rq;
intel_context_enter(ce);
- rq = __i915_request_create(ce, gfp);
+ rq = __i915_request_create(ce, gfp, true);
intel_context_exit(ce);
return rq;
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -80,39 +80,6 @@ static int __engine_unpark(struct intel_
return 0;
}
-#if IS_ENABLED(CONFIG_LOCKDEP)
-
-static unsigned long __timeline_mark_lock(struct intel_context *ce)
-{
- unsigned long flags;
-
- local_irq_save(flags);
- mutex_acquire(&ce->timeline->mutex.dep_map, 2, 0, _THIS_IP_);
-
- return flags;
-}
-
-static void __timeline_mark_unlock(struct intel_context *ce,
- unsigned long flags)
-{
- mutex_release(&ce->timeline->mutex.dep_map, _THIS_IP_);
- local_irq_restore(flags);
-}
-
-#else
-
-static unsigned long __timeline_mark_lock(struct intel_context *ce)
-{
- return 0;
-}
-
-static void __timeline_mark_unlock(struct intel_context *ce,
- unsigned long flags)
-{
-}
-
-#endif /* !IS_ENABLED(CONFIG_LOCKDEP) */
-
static void duration(struct dma_fence *fence, struct dma_fence_cb *cb)
{
struct i915_request *rq = to_request(fence);
@@ -159,7 +126,6 @@ static bool switch_to_kernel_context(str
{
struct intel_context *ce = engine->kernel_context;
struct i915_request *rq;
- unsigned long flags;
bool result = true;
/* GPU is pointing to the void, as good as in the kernel context. */
@@ -201,10 +167,9 @@ static bool switch_to_kernel_context(str
* engine->wakeref.count, we may see the request completion and retire
* it causing an underflow of the engine->wakeref.
*/
- flags = __timeline_mark_lock(ce);
GEM_BUG_ON(atomic_read(&ce->timeline->active_count) < 0);
- rq = __i915_request_create(ce, GFP_NOWAIT);
+ rq = __i915_request_create(ce, GFP_NOWAIT, false);
if (IS_ERR(rq))
/* Context switch failed, hope for the best! Maybe reset? */
goto out_unlock;
@@ -233,7 +198,6 @@ static bool switch_to_kernel_context(str
result = false;
out_unlock:
- __timeline_mark_unlock(ce, flags);
return result;
}
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -833,7 +833,8 @@ static void __i915_request_ctor(void *ar
}
struct i915_request *
-__i915_request_create(struct intel_context *ce, gfp_t gfp)
+__i915_request_create(struct intel_context *ce, gfp_t gfp,
+ bool timeline_mutex_needed)
{
struct intel_timeline *tl = ce->timeline;
struct i915_request *rq;
@@ -957,7 +958,7 @@ struct i915_request *
rq->infix = rq->ring->emit; /* end of header; start of user payload */
- intel_context_mark_active(ce);
+ intel_context_mark_active(ce, timeline_mutex_needed);
list_add_tail_rcu(&rq->link, &tl->requests);
return rq;
@@ -993,7 +994,7 @@ i915_request_create(struct intel_context
i915_request_retire(rq);
intel_context_enter(ce);
- rq = __i915_request_create(ce, GFP_KERNEL);
+ rq = __i915_request_create(ce, GFP_KERNEL, true);
intel_context_exit(ce); /* active reference transferred to request */
if (IS_ERR(rq))
goto err_unlock;
--- a/drivers/gpu/drm/i915/i915_request.h
+++ b/drivers/gpu/drm/i915/i915_request.h
@@ -320,7 +320,8 @@ static inline bool dma_fence_is_i915(con
struct kmem_cache *i915_request_slab_cache(void);
struct i915_request * __must_check
-__i915_request_create(struct intel_context *ce, gfp_t gfp);
+__i915_request_create(struct intel_context *ce, gfp_t gfp,
+ bool timeline_mutex_needed);
struct i915_request * __must_check
i915_request_create(struct intel_context *ce);
|
