summaryrefslogtreecommitdiffstats
path: root/20200310_chris_chris_wilson_co_uk.patch
blob: 9b2dad2ed7335bfc8ad4c24362ca883ac3009e78 (plain)
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
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
534
535
536
537
538
539
540
541
542
543
544
545
546
547
548
549
550
551
552
553
554
555
556
557
558
559
From MAILER-DAEMON Thu Mar 12 13:30:18 2020
From: Chris Wilson <chris@chris-wilson.co.uk>
To: stable@vger.kernel.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>, Tvrtko Ursulin <tvrtko.ursulin@intel.com>, Jani Nikula <jani.nikula@intel.com>
Subject: [PATCH 1/5] drm/i915: Check activity on i915_vma after confirming pin_count==0
Date: Tue, 10 Mar 2020 20:40:42 +0000
Message-Id: <20200310204046.3995087-1-chris@chris-wilson.co.uk>
Sender: stable-owner@vger.kernel.org
List-ID: <stable.vger.kernel.org>
X-Mailing-List: stable@vger.kernel.org
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

Only assert that the i915_vma is now idle if and only if no other pins
are present. If another user has the i915_vma pinned, they may submit
more work to the i915_vma skipping the vm->mutex used to serialise the
unbind. We need to wait again, if we want to continue and unbind this
vma.

However, if we own the i915_vma (we hold the vm->mutex for the unbind
and the pin_count is 0), we can assert that the vma remains idle as we
unbind.

Fixes: 2850748ef876 ("drm/i915: Pull i915_vma_pin under the vm->mutex")
Closes: https://gitlab.freedesktop.org/drm/intel/issues/530
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20200123224459.38128-1-chris@chris-wilson.co.uk
(cherry picked from commit 60e94557fff1f5514c7fc4da7ddc2c7a13ffff26)
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
(cherry picked from commit e4edd4fcbf4daf9d4319bef0bfaf350cb672239a)
---
 drivers/gpu/drm/i915/i915_vma.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 01c822256b39..7c7e152cc5ff 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -1149,16 +1149,26 @@ int __i915_vma_unbind(struct i915_vma *vma)
 	if (ret)
 		return ret;
 
-	GEM_BUG_ON(i915_vma_is_active(vma));
 	if (i915_vma_is_pinned(vma)) {
 		vma_print_allocator(vma, "is pinned");
 		return -EBUSY;
 	}
 
-	GEM_BUG_ON(i915_vma_is_active(vma));
+	/*
+	 * After confirming that no one else is pinning this vma, wait for
+	 * any laggards who may have crept in during the wait (through
+	 * a residual pin skipping the vm->mutex) to complete.
+	 */
+	ret = i915_vma_sync(vma);
+	if (ret)
+		return ret;
+
 	if (!drm_mm_node_allocated(&vma->node))
 		return 0;
 
+	GEM_BUG_ON(i915_vma_is_pinned(vma));
+	GEM_BUG_ON(i915_vma_is_active(vma));
+
 	if (i915_vma_is_map_and_fenceable(vma)) {
 		/*
 		 * Check that we have flushed all writes through the GGTT
-- 
2.25.1


From MAILER-DAEMON Thu Mar 12 13:30:18 2020
From: Chris Wilson <chris@chris-wilson.co.uk>
To: stable@vger.kernel.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>, Matthew Auld <matthew.auld@intel.com>
Subject: [PATCH 2/5] drm/i915/gem: Avoid parking the vma as we unbind
Date: Tue, 10 Mar 2020 20:40:43 +0000
Message-Id: <20200310204046.3995087-2-chris@chris-wilson.co.uk>
In-Reply-To: <20200310204046.3995087-1-chris@chris-wilson.co.uk>
References: <20200310204046.3995087-1-chris@chris-wilson.co.uk>
Sender: stable-owner@vger.kernel.org
List-ID: <stable.vger.kernel.org>
X-Mailing-List: stable@vger.kernel.org
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

In order to avoid keeping a reference on the i915_vma (which is long
overdue!) we have to coordinate all the possible lifetimes and only use
the vma while we know it is alive. In this episode, we are reminded that
while idle, the closed vma are destroyed. So if the GT idles while we are
working with the vma, the vma itself becomes invalid.

First class i915_vma here we come, but in the meantime keep piling on
the straw.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191203155032.3137263-1-chris@chris-wilson.co.uk
(cherry picked from commit cb6c3d45f948f8f184687a23fea30017d01e892f)
---
 drivers/gpu/drm/i915/i915_gem.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3f07948ea4da..f7c52b437f6a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -128,18 +128,33 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 						       struct i915_vma,
 						       obj_link))) {
 		struct i915_address_space *vm = vma->vm;
+		bool awake = false;
 
-		ret = -EBUSY;
+		ret = -EAGAIN;
 		if (!i915_vm_tryopen(vm))
 			break;
 
+		/* Prevent vma being freed by i915_vma_parked as we unbind */
+		if (intel_gt_pm_get_if_awake(vm->gt)) {
+			awake = true;
+		} else {
+			if (i915_vma_is_closed(vma)) {
+				spin_unlock(&obj->vma.lock);
+				goto err_vm;
+			}
+		}
+
 		list_move_tail(&vma->obj_link, &still_in_list);
 		spin_unlock(&obj->vma.lock);
 
+		ret = -EBUSY;
 		if (flags & I915_GEM_OBJECT_UNBIND_ACTIVE ||
 		    !i915_vma_is_active(vma))
 			ret = i915_vma_unbind(vma);
 
+		if (awake)
+			intel_gt_pm_put(vm->gt);
+err_vm:
 		i915_vm_close(vm);
 		spin_lock(&obj->vma.lock);
 	}
-- 
2.25.1


From MAILER-DAEMON Thu Mar 12 13:30:18 2020
From: Chris Wilson <chris@chris-wilson.co.uk>
To: stable@vger.kernel.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>, Imre Deak <imre.deak@intel.com>
Subject: [PATCH 3/5] drm/i915: Add a simple is-bound check before unbinding
Date: Tue, 10 Mar 2020 20:40:44 +0000
Message-Id: <20200310204046.3995087-3-chris@chris-wilson.co.uk>
In-Reply-To: <20200310204046.3995087-1-chris@chris-wilson.co.uk>
References: <20200310204046.3995087-1-chris@chris-wilson.co.uk>
Sender: stable-owner@vger.kernel.org
List-ID: <stable.vger.kernel.org>
X-Mailing-List: stable@vger.kernel.org
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

Only acquire the various atomic references required to unbind the vma if
we do need to unbind the vma.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191222210256.2066451-1-chris@chris-wilson.co.uk
(cherry picked from commit f5af1659d809e264d619e5f483fd8f47bced3b6a)
---
 drivers/gpu/drm/i915/i915_gem.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f7c52b437f6a..998b67e3466e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -130,6 +130,10 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 		struct i915_address_space *vm = vma->vm;
 		bool awake = false;
 
+		list_move_tail(&vma->obj_link, &still_in_list);
+		if (!i915_vma_is_bound(vma, I915_VMA_BIND_MASK))
+			continue;
+
 		ret = -EAGAIN;
 		if (!i915_vm_tryopen(vm))
 			break;
@@ -144,7 +148,6 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 			}
 		}
 
-		list_move_tail(&vma->obj_link, &still_in_list);
 		spin_unlock(&obj->vma.lock);
 
 		ret = -EBUSY;
-- 
2.25.1


From MAILER-DAEMON Thu Mar 12 13:30:18 2020
From: Chris Wilson <chris@chris-wilson.co.uk>
To: stable@vger.kernel.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>, Imre Deak <imre.deak@intel.com>
Subject: [PATCH 4/5] drm/i915: Introduce a vma.kref
Date: Tue, 10 Mar 2020 20:40:45 +0000
Message-Id: <20200310204046.3995087-4-chris@chris-wilson.co.uk>
In-Reply-To: <20200310204046.3995087-1-chris@chris-wilson.co.uk>
References: <20200310204046.3995087-1-chris@chris-wilson.co.uk>
Sender: stable-owner@vger.kernel.org
List-ID: <stable.vger.kernel.org>
X-Mailing-List: stable@vger.kernel.org
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

Start introducing a kref on i915_vma in order to protect the vma unbind
(i915_gem_object_unbind) from a parallel destruction (i915_vma_parked).
Later, we will use the refcount to manage all access and turn i915_vma
into a first class container.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Acked-by: Imre Deak <imre.deak@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191222210256.2066451-2-chris@chris-wilson.co.uk
(cherry picked from commit 76f9764cc3d538435262dea885bf69fac2415402)
---
 drivers/gpu/drm/i915/gem/i915_gem_object.c    |  2 +-
 .../gpu/drm/i915/gem/selftests/huge_pages.c   |  3 +--
 .../drm/i915/gem/selftests/i915_gem_mman.c    |  4 +--
 drivers/gpu/drm/i915/i915_gem.c               | 26 +++++++------------
 drivers/gpu/drm/i915/i915_gem_gtt.c           |  5 ++--
 drivers/gpu/drm/i915/i915_vma.c               |  9 ++++---
 drivers/gpu/drm/i915/i915_vma.h               | 25 +++++++++++++++---
 7 files changed, 44 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index a596548c07bf..b6937469ffd3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -174,7 +174,7 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
 				GEM_BUG_ON(vma->obj != obj);
 				spin_unlock(&obj->vma.lock);
 
-				i915_vma_destroy(vma);
+				__i915_vma_put(vma);
 
 				spin_lock(&obj->vma.lock);
 			}
diff --git a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
index 688c49a24f32..bd1e2c12de63 100644
--- a/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
+++ b/drivers/gpu/drm/i915/gem/selftests/huge_pages.c
@@ -1110,8 +1110,7 @@ static int __igt_write_huge(struct intel_context *ce,
 out_vma_unpin:
 	i915_vma_unpin(vma);
 out_vma_close:
-	i915_vma_destroy(vma);
-
+	__i915_vma_put(vma);
 	return err;
 }
 
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 29b2077b73d2..d226e55df8b2 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -161,7 +161,7 @@ static int check_partial_mapping(struct drm_i915_gem_object *obj,
 	kunmap(p);
 
 out:
-	i915_vma_destroy(vma);
+	__i915_vma_put(vma);
 	return err;
 }
 
@@ -255,7 +255,7 @@ static int check_partial_mappings(struct drm_i915_gem_object *obj,
 		if (err)
 			return err;
 
-		i915_vma_destroy(vma);
+		__i915_vma_put(vma);
 
 		if (igt_timeout(end_time,
 				"%s: timed out after tiling=%d stride=%d\n",
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 998b67e3466e..67a8e7408e67 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -128,7 +128,6 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 						       struct i915_vma,
 						       obj_link))) {
 		struct i915_address_space *vm = vma->vm;
-		bool awake = false;
 
 		list_move_tail(&vma->obj_link, &still_in_list);
 		if (!i915_vma_is_bound(vma, I915_VMA_BIND_MASK))
@@ -139,25 +138,18 @@ int i915_gem_object_unbind(struct drm_i915_gem_object *obj,
 			break;
 
 		/* Prevent vma being freed by i915_vma_parked as we unbind */
-		if (intel_gt_pm_get_if_awake(vm->gt)) {
-			awake = true;
-		} else {
-			if (i915_vma_is_closed(vma)) {
-				spin_unlock(&obj->vma.lock);
-				goto err_vm;
-			}
-		}
-
+		vma = __i915_vma_get(vma);
 		spin_unlock(&obj->vma.lock);
 
-		ret = -EBUSY;
-		if (flags & I915_GEM_OBJECT_UNBIND_ACTIVE ||
-		    !i915_vma_is_active(vma))
-			ret = i915_vma_unbind(vma);
+		if (vma) {
+			ret = -EBUSY;
+			if (flags & I915_GEM_OBJECT_UNBIND_ACTIVE ||
+			    !i915_vma_is_active(vma))
+				ret = i915_vma_unbind(vma);
+
+			__i915_vma_put(vma);
+		}
 
-		if (awake)
-			intel_gt_pm_put(vm->gt);
-err_vm:
 		i915_vm_close(vm);
 		spin_lock(&obj->vma.lock);
 	}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 44727806dfd7..dd2c20f7d4d2 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -522,7 +522,7 @@ void __i915_vm_close(struct i915_address_space *vm)
 
 		atomic_and(~I915_VMA_PIN_MASK, &vma->flags);
 		WARN_ON(__i915_vma_unbind(vma));
-		i915_vma_destroy(vma);
+		__i915_vma_put(vma);
 
 		i915_gem_object_put(obj);
 	}
@@ -1790,7 +1790,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
 {
 	struct gen6_ppgtt *ppgtt = to_gen6_ppgtt(i915_vm_to_ppgtt(vm));
 
-	i915_vma_destroy(ppgtt->vma);
+	__i915_vma_put(ppgtt->vma);
 
 	gen6_ppgtt_free_pd(ppgtt);
 	free_scratch(vm);
@@ -1878,6 +1878,7 @@ static struct i915_vma *pd_vma_create(struct gen6_ppgtt *ppgtt, int size)
 
 	i915_active_init(&vma->active, NULL, NULL);
 
+	kref_init(&vma->ref);
 	mutex_init(&vma->pages_mutex);
 	vma->vm = i915_vm_get(&ggtt->vm);
 	vma->ops = &pd_vma_ops;
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 7c7e152cc5ff..5309872442bc 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -112,6 +112,7 @@ vma_create(struct drm_i915_gem_object *obj,
 	if (vma == NULL)
 		return ERR_PTR(-ENOMEM);
 
+	kref_init(&vma->ref);
 	mutex_init(&vma->pages_mutex);
 	vma->vm = i915_vm_get(vm);
 	vma->ops = &vm->vma_ops;
@@ -978,8 +979,10 @@ void i915_vma_reopen(struct i915_vma *vma)
 		__i915_vma_remove_closed(vma);
 }
 
-void i915_vma_destroy(struct i915_vma *vma)
+void i915_vma_release(struct kref *ref)
 {
+	struct i915_vma *vma = container_of(ref, typeof(*vma), ref);
+
 	if (drm_mm_node_allocated(&vma->node)) {
 		mutex_lock(&vma->vm->mutex);
 		atomic_and(~I915_VMA_PIN_MASK, &vma->flags);
@@ -1027,7 +1030,7 @@ void i915_vma_parked(struct intel_gt *gt)
 		spin_unlock_irq(&gt->closed_lock);
 
 		if (obj) {
-			i915_vma_destroy(vma);
+			__i915_vma_put(vma);
 			i915_gem_object_put(obj);
 		}
 
@@ -1202,7 +1205,7 @@ int __i915_vma_unbind(struct i915_vma *vma)
 	i915_vma_detach(vma);
 	vma_unbind_pages(vma);
 
-	drm_mm_remove_node(&vma->node); /* pairs with i915_vma_destroy() */
+	drm_mm_remove_node(&vma->node); /* pairs with i915_vma_release() */
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index 465932813bc5..ce1db908ad69 100644
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -51,14 +51,19 @@ enum i915_cache_level;
  */
 struct i915_vma {
 	struct drm_mm_node node;
-	struct drm_i915_gem_object *obj;
+
 	struct i915_address_space *vm;
 	const struct i915_vma_ops *ops;
-	struct i915_fence_reg *fence;
+
+	struct drm_i915_gem_object *obj;
 	struct dma_resv *resv; /** Alias of obj->resv */
+
 	struct sg_table *pages;
 	void __iomem *iomap;
 	void *private; /* owned by creator */
+
+	struct i915_fence_reg *fence;
+
 	u64 size;
 	u64 display_alignment;
 	struct i915_page_sizes page_sizes;
@@ -71,6 +76,7 @@ struct i915_vma {
 	 * handles (but same file) for execbuf, i.e. the number of aliases
 	 * that exist in the ctx->handle_vmas LUT for this vma.
 	 */
+	struct kref ref;
 	atomic_t open_count;
 	atomic_t flags;
 	/**
@@ -333,7 +339,20 @@ int __must_check i915_vma_unbind(struct i915_vma *vma);
 void i915_vma_unlink_ctx(struct i915_vma *vma);
 void i915_vma_close(struct i915_vma *vma);
 void i915_vma_reopen(struct i915_vma *vma);
-void i915_vma_destroy(struct i915_vma *vma);
+
+static inline struct i915_vma *__i915_vma_get(struct i915_vma *vma)
+{
+	if (kref_get_unless_zero(&vma->ref))
+		return vma;
+
+	return NULL;
+}
+
+void i915_vma_release(struct kref *ref);
+static inline void __i915_vma_put(struct i915_vma *vma)
+{
+	kref_put(&vma->ref, i915_vma_release);
+}
 
 #define assert_vma_held(vma) dma_resv_assert_held((vma)->resv)
 
-- 
2.25.1


From MAILER-DAEMON Thu Mar 12 13:30:18 2020
From: Chris Wilson <chris@chris-wilson.co.uk>
To: stable@vger.kernel.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>, Kenneth Graunke <kenneth@whitecape.org>, Tvrtko Ursulin <tvrtko.ursulin@intel.com>, Matthew Auld <matthew.auld@intel.com>
Subject: [PATCH 5/5] drm/i915: Serialise i915_active_acquire() with __active_retire()
Date: Tue, 10 Mar 2020 20:40:46 +0000
Message-Id: <20200310204046.3995087-5-chris@chris-wilson.co.uk>
In-Reply-To: <20200310204046.3995087-1-chris@chris-wilson.co.uk>
References: <20200310204046.3995087-1-chris@chris-wilson.co.uk>
Sender: stable-owner@vger.kernel.org
List-ID: <stable.vger.kernel.org>
X-Mailing-List: stable@vger.kernel.org
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

As __active_retire() does it's final atomic_dec() under the
ref->tree_lock spinlock, in order to prevent ourselves from reusing the
ref->cache and ref->tree as they are being destroyed, we need to
serialise with the retirement during i915_active_acquire().

[  +0.000005] kernel BUG at drivers/gpu/drm/i915/i915_active.c:157!
[  +0.000011] invalid opcode: 0000 [#1] SMP
[  +0.000004] CPU: 7 PID: 188 Comm: kworker/u16:4 Not tainted 5.4.0-rc8-03070-gac5e57322614 #89
[  +0.000002] Hardware name: Razer Razer Blade Stealth 13 Late 2019/LY320, BIOS 1.02 09/10/2019
[  +0.000082] Workqueue: events_unbound active_work [i915]
[  +0.000059] RIP: 0010:__active_retire+0x115/0x120 [i915]
[  +0.000003] Code: 75 28 48 8b 3d 8c 6e 1a 00 48 89 ee e8 e4 5f a5 c0 48 8b 44 24 10 65 48 33 04 25 28 00 00 00 75 0f 48 83 c4 18 5b 5d 41 5c c3 <0f> 0b 0f 0b 0f 0b e8 a0 90 87 c0 0f 1f 44 00 00 48 8b 3d 54 6e 1a
[  +0.000002] RSP: 0018:ffffb833003f7e48 EFLAGS: 00010286
[  +0.000003] RAX: ffff8d6e8d726d00 RBX: ffff8d6f9db4e840 RCX: 0000000000000000
[  +0.000001] RDX: ffffffff82605930 RSI: ffff8d6f9adc4908 RDI: ffff8d6e96cefe28
[  +0.000002] RBP: ffff8d6e96cefe00 R08: 0000000000000000 R09: ffff8d6f9ffe9a50
[  +0.000002] R10: 0000000000000048 R11: 0000000000000018 R12: ffff8d6f9adc4930
[  +0.000001] R13: ffff8d6f9e04fb00 R14: 0000000000000000 R15: ffff8d6f9adc4988
[  +0.000002] FS:  0000000000000000(0000) GS:ffff8d6f9ffc0000(0000) knlGS:0000000000000000
[  +0.000002] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  +0.000002] CR2: 000055eb5a34cf10 CR3: 000000018d609002 CR4: 0000000000760ee0
[  +0.000002] PKRU: 55555554
[  +0.000001] Call Trace:
[  +0.000010]  process_one_work+0x1aa/0x350
[  +0.000004]  worker_thread+0x4d/0x3a0
[  +0.000004]  kthread+0xfb/0x130
[  +0.000004]  ? process_one_work+0x350/0x350
[  +0.000003]  ? kthread_park+0x90/0x90
[  +0.000005]  ret_from_fork+0x1f/0x40

Reported-by: Kenneth Graunke <kenneth@whitecape.org>
Fixes: c9ad602feabe ("drm/i915: Split i915_active.mutex into an irq-safe spinlock for the rbtree")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Kenneth Graunke <kenneth@whitecape.org>
Cc: Matthew Auld <matthew.auld@intel.com>
Tested-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20191205183332.801237-1-chris@chris-wilson.co.uk
(cherry picked from commit bbca083de291a03ffe1a1eb0832a0d74f8b64898)
---
 drivers/gpu/drm/i915/i915_active.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c
index a19e7d89bc8a..378b52d1ab74 100644
--- a/drivers/gpu/drm/i915/i915_active.c
+++ b/drivers/gpu/drm/i915/i915_active.c
@@ -91,10 +91,9 @@ static void debug_active_init(struct i915_active *ref)
 
 static void debug_active_activate(struct i915_active *ref)
 {
-	spin_lock_irq(&ref->tree_lock);
+	lockdep_assert_held(&ref->tree_lock);
 	if (!atomic_read(&ref->count)) /* before the first inc */
 		debug_object_activate(ref, &active_debug_desc);
-	spin_unlock_irq(&ref->tree_lock);
 }
 
 static void debug_active_deactivate(struct i915_active *ref)
@@ -407,8 +406,10 @@ int i915_active_acquire(struct i915_active *ref)
 	if (!atomic_read(&ref->count) && ref->active)
 		err = ref->active(ref);
 	if (!err) {
+		spin_lock_irq(&ref->tree_lock); /* vs __active_retire() */
 		debug_active_activate(ref);
 		atomic_inc(&ref->count);
+		spin_unlock_irq(&ref->tree_lock);
 	}
 
 	mutex_unlock(&ref->mutex);
-- 
2.25.1