Skip to content

Commit cf41a8f

Browse files
mlankhorstdanvet
mlankhorst
authored and
danvet
committed
drm/i915: Finally remove obj->mm.lock.
With all callers and selftests fixed to use ww locking, we can now finally remove this lock. Signed-off-by: Maarten Lankhorst <[email protected]> Reviewed-by: Thomas Hellström <[email protected]> Signed-off-by: Daniel Vetter <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected]
1 parent 480ae79 commit cf41a8f

14 files changed

+55
-92
lines changed

‎drivers/gpu/drm/i915/gem/i915_gem_object.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,6 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
6262
const struct drm_i915_gem_object_ops *ops,
6363
struct lock_class_key *key, unsigned flags)
6464
{
65-
mutex_init(&obj->mm.lock);
66-
6765
spin_lock_init(&obj->vma.lock);
6866
INIT_LIST_HEAD(&obj->vma.list);
6967

‎drivers/gpu/drm/i915/gem/i915_gem_object.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ static inline void assert_object_held_shared(struct drm_i915_gem_object *obj)
129129
*/
130130
if (IS_ENABLED(CONFIG_LOCKDEP) &&
131131
kref_read(&obj->base.refcount) > 0)
132-
lockdep_assert_held(&obj->mm.lock);
132+
assert_object_held(obj);
133133
}
134134

135135
static inline int __i915_gem_object_lock(struct drm_i915_gem_object *obj,
@@ -358,7 +358,7 @@ int __i915_gem_object_get_pages(struct drm_i915_gem_object *obj);
358358
static inline int __must_check
359359
i915_gem_object_pin_pages(struct drm_i915_gem_object *obj)
360360
{
361-
might_lock(&obj->mm.lock);
361+
assert_object_held(obj);
362362

363363
if (atomic_inc_not_zero(&obj->mm.pages_pin_count))
364364
return 0;
@@ -404,7 +404,6 @@ i915_gem_object_unpin_pages(struct drm_i915_gem_object *obj)
404404
}
405405

406406
int __i915_gem_object_put_pages(struct drm_i915_gem_object *obj);
407-
int __i915_gem_object_put_pages_locked(struct drm_i915_gem_object *obj);
408407
void i915_gem_object_truncate(struct drm_i915_gem_object *obj);
409408
void i915_gem_object_writeback(struct drm_i915_gem_object *obj);
410409

‎drivers/gpu/drm/i915/gem/i915_gem_object_types.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,6 @@ struct drm_i915_gem_object {
216216
* Protects the pages and their use. Do not use directly, but
217217
* instead go through the pin/unpin interfaces.
218218
*/
219-
struct mutex lock;
220219
atomic_t pages_pin_count;
221220
atomic_t shrink_pin;
222221

‎drivers/gpu/drm/i915/gem/i915_gem_pages.c

Lines changed: 9 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
7070
struct list_head *list;
7171
unsigned long flags;
7272

73-
lockdep_assert_held(&obj->mm.lock);
73+
assert_object_held(obj);
7474
spin_lock_irqsave(&i915->mm.obj_lock, flags);
7575

7676
i915->mm.shrink_count++;
@@ -117,9 +117,7 @@ int __i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
117117
{
118118
int err;
119119

120-
err = mutex_lock_interruptible(&obj->mm.lock);
121-
if (err)
122-
return err;
120+
assert_object_held(obj);
123121

124122
assert_object_held_shared(obj);
125123

@@ -128,15 +126,13 @@ int __i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
128126

129127
err = ____i915_gem_object_get_pages(obj);
130128
if (err)
131-
goto unlock;
129+
return err;
132130

133131
smp_mb__before_atomic();
134132
}
135133
atomic_inc(&obj->mm.pages_pin_count);
136134

137-
unlock:
138-
mutex_unlock(&obj->mm.lock);
139-
return err;
135+
return 0;
140136
}
141137

142138
int i915_gem_object_pin_pages_unlocked(struct drm_i915_gem_object *obj)
@@ -223,7 +219,7 @@ __i915_gem_object_unset_pages(struct drm_i915_gem_object *obj)
223219
return pages;
224220
}
225221

226-
int __i915_gem_object_put_pages_locked(struct drm_i915_gem_object *obj)
222+
int __i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
227223
{
228224
struct sg_table *pages;
229225

@@ -254,21 +250,6 @@ int __i915_gem_object_put_pages_locked(struct drm_i915_gem_object *obj)
254250
return 0;
255251
}
256252

257-
int __i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
258-
{
259-
int err;
260-
261-
if (i915_gem_object_has_pinned_pages(obj))
262-
return -EBUSY;
263-
264-
/* May be called by shrinker from within get_pages() (on another bo) */
265-
mutex_lock(&obj->mm.lock);
266-
err = __i915_gem_object_put_pages_locked(obj);
267-
mutex_unlock(&obj->mm.lock);
268-
269-
return err;
270-
}
271-
272253
/* The 'mapping' part of i915_gem_object_pin_map() below */
273254
static void *i915_gem_object_map_page(struct drm_i915_gem_object *obj,
274255
enum i915_map_type type)
@@ -371,9 +352,7 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj,
371352
!i915_gem_object_type_has(obj, I915_GEM_OBJECT_HAS_IOMEM))
372353
return ERR_PTR(-ENXIO);
373354

374-
err = mutex_lock_interruptible(&obj->mm.lock);
375-
if (err)
376-
return ERR_PTR(err);
355+
assert_object_held(obj);
377356

378357
pinned = !(type & I915_MAP_OVERRIDE);
379358
type &= ~I915_MAP_OVERRIDE;
@@ -383,10 +362,8 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj,
383362
GEM_BUG_ON(i915_gem_object_has_pinned_pages(obj));
384363

385364
err = ____i915_gem_object_get_pages(obj);
386-
if (err) {
387-
ptr = ERR_PTR(err);
388-
goto out_unlock;
389-
}
365+
if (err)
366+
return ERR_PTR(err);
390367

391368
smp_mb__before_atomic();
392369
}
@@ -421,13 +398,11 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj,
421398
obj->mm.mapping = page_pack_bits(ptr, type);
422399
}
423400

424-
out_unlock:
425-
mutex_unlock(&obj->mm.lock);
426401
return ptr;
427402

428403
err_unpin:
429404
atomic_dec(&obj->mm.pages_pin_count);
430-
goto out_unlock;
405+
return ptr;
431406
}
432407

433408
void *i915_gem_object_pin_map_unlocked(struct drm_i915_gem_object *obj,

‎drivers/gpu/drm/i915/gem/i915_gem_phys.c

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -234,40 +234,22 @@ int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj, int align)
234234
if (err)
235235
return err;
236236

237-
err = mutex_lock_interruptible(&obj->mm.lock);
238-
if (err)
239-
return err;
240-
241-
if (unlikely(!i915_gem_object_has_struct_page(obj)))
242-
goto out;
243-
244-
if (obj->mm.madv != I915_MADV_WILLNEED) {
245-
err = -EFAULT;
246-
goto out;
247-
}
237+
if (obj->mm.madv != I915_MADV_WILLNEED)
238+
return -EFAULT;
248239

249-
if (i915_gem_object_has_tiling_quirk(obj)) {
250-
err = -EFAULT;
251-
goto out;
252-
}
240+
if (i915_gem_object_has_tiling_quirk(obj))
241+
return -EFAULT;
253242

254-
if (obj->mm.mapping || i915_gem_object_has_pinned_pages(obj)) {
255-
err = -EBUSY;
256-
goto out;
257-
}
243+
if (obj->mm.mapping || i915_gem_object_has_pinned_pages(obj))
244+
return -EBUSY;
258245

259246
if (unlikely(obj->mm.madv != I915_MADV_WILLNEED)) {
260247
drm_dbg(obj->base.dev,
261248
"Attempting to obtain a purgeable object\n");
262-
err = -EFAULT;
263-
goto out;
249+
return -EFAULT;
264250
}
265251

266-
err = i915_gem_object_shmem_to_phys(obj);
267-
268-
out:
269-
mutex_unlock(&obj->mm.lock);
270-
return err;
252+
return i915_gem_object_shmem_to_phys(obj);
271253
}
272254

273255
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)

‎drivers/gpu/drm/i915/gem/i915_gem_pm.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ int i915_gem_freeze_late(struct drm_i915_private *i915)
116116
*/
117117

118118
with_intel_runtime_pm(&i915->runtime_pm, wakeref)
119-
i915_gem_shrink(i915, -1UL, NULL, ~0);
119+
i915_gem_shrink(NULL, i915, -1UL, NULL, ~0);
120120
i915_gem_drain_freed_objects(i915);
121121

122122
wbinvd_on_all_cpus();

‎drivers/gpu/drm/i915/gem/i915_gem_shmem.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ static int shmem_get_pages(struct drm_i915_gem_object *obj)
9999
goto err_sg;
100100
}
101101

102-
i915_gem_shrink(i915, 2 * page_count, NULL, *s++);
102+
i915_gem_shrink(NULL, i915, 2 * page_count, NULL, *s++);
103103

104104
/*
105105
* We've tried hard to allocate the memory by reaping

‎drivers/gpu/drm/i915/gem/i915_gem_shrinker.c

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,8 @@ static void try_to_writeback(struct drm_i915_gem_object *obj,
9494
* The number of pages of backing storage actually released.
9595
*/
9696
unsigned long
97-
i915_gem_shrink(struct drm_i915_private *i915,
97+
i915_gem_shrink(struct i915_gem_ww_ctx *ww,
98+
struct drm_i915_private *i915,
9899
unsigned long target,
99100
unsigned long *nr_scanned,
100101
unsigned int shrink)
@@ -113,6 +114,7 @@ i915_gem_shrink(struct drm_i915_private *i915,
113114
intel_wakeref_t wakeref = 0;
114115
unsigned long count = 0;
115116
unsigned long scanned = 0;
117+
int err;
116118

117119
trace_i915_gem_shrink(i915, target, shrink);
118120

@@ -200,25 +202,40 @@ i915_gem_shrink(struct drm_i915_private *i915,
200202

201203
spin_unlock_irqrestore(&i915->mm.obj_lock, flags);
202204

203-
if (unsafe_drop_pages(obj, shrink) &&
204-
mutex_trylock(&obj->mm.lock)) {
205+
err = 0;
206+
if (unsafe_drop_pages(obj, shrink)) {
205207
/* May arrive from get_pages on another bo */
206-
if (!__i915_gem_object_put_pages_locked(obj)) {
208+
if (!ww) {
209+
if (!i915_gem_object_trylock(obj))
210+
goto skip;
211+
} else {
212+
err = i915_gem_object_lock(obj, ww);
213+
if (err)
214+
goto skip;
215+
}
216+
217+
if (!__i915_gem_object_put_pages(obj)) {
207218
try_to_writeback(obj, shrink);
208219
count += obj->base.size >> PAGE_SHIFT;
209220
}
210-
mutex_unlock(&obj->mm.lock);
221+
if (!ww)
222+
i915_gem_object_unlock(obj);
211223
}
212224

213225
dma_resv_prune(obj->base.resv);
214226

215227
scanned += obj->base.size >> PAGE_SHIFT;
228+
skip:
216229
i915_gem_object_put(obj);
217230

218231
spin_lock_irqsave(&i915->mm.obj_lock, flags);
232+
if (err)
233+
break;
219234
}
220235
list_splice_tail(&still_in_list, phase->list);
221236
spin_unlock_irqrestore(&i915->mm.obj_lock, flags);
237+
if (err)
238+
return err;
222239
}
223240

224241
if (shrink & I915_SHRINK_BOUND)
@@ -249,7 +266,7 @@ unsigned long i915_gem_shrink_all(struct drm_i915_private *i915)
249266
unsigned long freed = 0;
250267

251268
with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
252-
freed = i915_gem_shrink(i915, -1UL, NULL,
269+
freed = i915_gem_shrink(NULL, i915, -1UL, NULL,
253270
I915_SHRINK_BOUND |
254271
I915_SHRINK_UNBOUND);
255272
}
@@ -295,7 +312,7 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
295312

296313
sc->nr_scanned = 0;
297314

298-
freed = i915_gem_shrink(i915,
315+
freed = i915_gem_shrink(NULL, i915,
299316
sc->nr_to_scan,
300317
&sc->nr_scanned,
301318
I915_SHRINK_BOUND |
@@ -304,7 +321,7 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
304321
intel_wakeref_t wakeref;
305322

306323
with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
307-
freed += i915_gem_shrink(i915,
324+
freed += i915_gem_shrink(NULL, i915,
308325
sc->nr_to_scan - sc->nr_scanned,
309326
&sc->nr_scanned,
310327
I915_SHRINK_ACTIVE |
@@ -329,7 +346,7 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
329346

330347
freed_pages = 0;
331348
with_intel_runtime_pm(&i915->runtime_pm, wakeref)
332-
freed_pages += i915_gem_shrink(i915, -1UL, NULL,
349+
freed_pages += i915_gem_shrink(NULL, i915, -1UL, NULL,
333350
I915_SHRINK_BOUND |
334351
I915_SHRINK_UNBOUND |
335352
I915_SHRINK_WRITEBACK);
@@ -367,7 +384,7 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
367384
intel_wakeref_t wakeref;
368385

369386
with_intel_runtime_pm(&i915->runtime_pm, wakeref)
370-
freed_pages += i915_gem_shrink(i915, -1UL, NULL,
387+
freed_pages += i915_gem_shrink(NULL, i915, -1UL, NULL,
371388
I915_SHRINK_BOUND |
372389
I915_SHRINK_UNBOUND |
373390
I915_SHRINK_VMAPS);

‎drivers/gpu/drm/i915/gem/i915_gem_shrinker.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,12 @@
99
#include <linux/bits.h>
1010

1111
struct drm_i915_private;
12+
struct i915_gem_ww_ctx;
1213
struct mutex;
1314

1415
/* i915_gem_shrinker.c */
15-
unsigned long i915_gem_shrink(struct drm_i915_private *i915,
16+
unsigned long i915_gem_shrink(struct i915_gem_ww_ctx *ww,
17+
struct drm_i915_private *i915,
1618
unsigned long target,
1719
unsigned long *nr_scanned,
1820
unsigned flags);

‎drivers/gpu/drm/i915/gem/i915_gem_tiling.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,6 @@ i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
265265
* pages to prevent them being swapped out and causing corruption
266266
* due to the change in swizzling.
267267
*/
268-
mutex_lock(&obj->mm.lock);
269268
if (i915_gem_object_has_pages(obj) &&
270269
obj->mm.madv == I915_MADV_WILLNEED &&
271270
i915->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
@@ -280,7 +279,6 @@ i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
280279
i915_gem_object_set_tiling_quirk(obj);
281280
}
282281
}
283-
mutex_unlock(&obj->mm.lock);
284282

285283
spin_lock(&obj->vma.lock);
286284
for_each_ggtt_vma(vma, obj) {

0 commit comments

Comments
 (0)