Skip to content

Commit 5f09a9c

Browse files
committed
drm/i915: Allow contexts to be unreferenced locklessly
If we move the actual cleanup of the context to a worker, we can allow the final free to be called from any context and avoid undue latency in the caller. v2: Negotiate handling the delayed contexts free by flushing the workqueue before calling i915_gem_context_fini() and performing the final free of the kernel context directly v3: Flush deferred frees before new context allocations Signed-off-by: Chris Wilson <[email protected]> Reviewed-by: Joonas Lahtinen <[email protected]> Link: http://patchwork.freedesktop.org/patch/msgid/[email protected]
1 parent 829a0af commit 5f09a9c

File tree

10 files changed

+88
-39
lines changed

10 files changed

+88
-39
lines changed

drivers/gpu/drm/i915/gvt/scheduler.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,7 @@ int intel_gvt_init_workload_scheduler(struct intel_gvt *gvt)
620620

621621
void intel_vgpu_clean_gvt_context(struct intel_vgpu *vgpu)
622622
{
623-
i915_gem_context_put_unlocked(vgpu->shadow_ctx);
623+
i915_gem_context_put(vgpu->shadow_ctx);
624624
}
625625

626626
int intel_vgpu_init_gvt_context(struct intel_vgpu *vgpu)

drivers/gpu/drm/i915/i915_drv.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,8 @@ static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
585585

586586
static void i915_gem_fini(struct drm_i915_private *dev_priv)
587587
{
588+
flush_workqueue(dev_priv->wq);
589+
588590
mutex_lock(&dev_priv->drm.struct_mutex);
589591
intel_uc_fini_hw(dev_priv);
590592
i915_gem_cleanup_engines(dev_priv);

drivers/gpu/drm/i915/i915_drv.h

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2315,6 +2315,8 @@ struct drm_i915_private {
23152315

23162316
struct {
23172317
struct list_head list;
2318+
struct llist_head free_list;
2319+
struct work_struct free_work;
23182320

23192321
/* The hw wants to have a stable context identifier for the
23202322
* lifetime of the context (for OA, PASID, faults, etc).
@@ -3545,27 +3547,6 @@ i915_gem_context_lookup(struct drm_i915_file_private *file_priv, u32 id)
35453547
return ctx;
35463548
}
35473549

3548-
static inline struct i915_gem_context *
3549-
i915_gem_context_get(struct i915_gem_context *ctx)
3550-
{
3551-
kref_get(&ctx->ref);
3552-
return ctx;
3553-
}
3554-
3555-
static inline void i915_gem_context_put(struct i915_gem_context *ctx)
3556-
{
3557-
lockdep_assert_held(&ctx->i915->drm.struct_mutex);
3558-
kref_put(&ctx->ref, i915_gem_context_free);
3559-
}
3560-
3561-
static inline void i915_gem_context_put_unlocked(struct i915_gem_context *ctx)
3562-
{
3563-
struct mutex *lock = &ctx->i915->drm.struct_mutex;
3564-
3565-
if (kref_put_mutex(&ctx->ref, i915_gem_context_free, lock))
3566-
mutex_unlock(lock);
3567-
}
3568-
35693550
static inline struct intel_timeline *
35703551
i915_gem_context_lookup_timeline(struct i915_gem_context *ctx,
35713552
struct intel_engine_cs *engine)

drivers/gpu/drm/i915/i915_gem_context.c

Lines changed: 48 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -158,13 +158,11 @@ static void vma_lut_free(struct i915_gem_context *ctx)
158158
kvfree(lut->ht);
159159
}
160160

161-
void i915_gem_context_free(struct kref *ctx_ref)
161+
static void i915_gem_context_free(struct i915_gem_context *ctx)
162162
{
163-
struct i915_gem_context *ctx = container_of(ctx_ref, typeof(*ctx), ref);
164163
int i;
165164

166165
lockdep_assert_held(&ctx->i915->drm.struct_mutex);
167-
trace_i915_context_free(ctx);
168166
GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
169167

170168
vma_lut_free(ctx);
@@ -192,6 +190,37 @@ void i915_gem_context_free(struct kref *ctx_ref)
192190
kfree(ctx);
193191
}
194192

193+
static void contexts_free(struct drm_i915_private *i915)
194+
{
195+
struct llist_node *freed = llist_del_all(&i915->contexts.free_list);
196+
struct i915_gem_context *ctx;
197+
198+
lockdep_assert_held(&i915->drm.struct_mutex);
199+
200+
llist_for_each_entry(ctx, freed, free_link)
201+
i915_gem_context_free(ctx);
202+
}
203+
204+
static void contexts_free_worker(struct work_struct *work)
205+
{
206+
struct drm_i915_private *i915 =
207+
container_of(work, typeof(*i915), contexts.free_work);
208+
209+
mutex_lock(&i915->drm.struct_mutex);
210+
contexts_free(i915);
211+
mutex_unlock(&i915->drm.struct_mutex);
212+
}
213+
214+
void i915_gem_context_release(struct kref *ref)
215+
{
216+
struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
217+
struct drm_i915_private *i915 = ctx->i915;
218+
219+
trace_i915_context_free(ctx);
220+
if (llist_add(&ctx->free_link, &i915->contexts.free_list))
221+
queue_work(i915->wq, &i915->contexts.free_work);
222+
}
223+
195224
static void context_close(struct i915_gem_context *ctx)
196225
{
197226
i915_gem_context_set_closed(ctx);
@@ -428,6 +457,8 @@ int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
428457
return 0;
429458

430459
INIT_LIST_HEAD(&dev_priv->contexts.list);
460+
INIT_WORK(&dev_priv->contexts.free_work, contexts_free_worker);
461+
init_llist_head(&dev_priv->contexts.free_list);
431462

432463
if (intel_vgpu_active(dev_priv) &&
433464
HAS_LOGICAL_RING_CONTEXTS(dev_priv)) {
@@ -505,18 +536,20 @@ void i915_gem_contexts_lost(struct drm_i915_private *dev_priv)
505536
}
506537
}
507538

508-
void i915_gem_contexts_fini(struct drm_i915_private *dev_priv)
539+
void i915_gem_contexts_fini(struct drm_i915_private *i915)
509540
{
510-
struct i915_gem_context *dctx = dev_priv->kernel_context;
511-
512-
lockdep_assert_held(&dev_priv->drm.struct_mutex);
541+
struct i915_gem_context *ctx;
513542

514-
GEM_BUG_ON(!i915_gem_context_is_kernel(dctx));
543+
lockdep_assert_held(&i915->drm.struct_mutex);
515544

516-
context_close(dctx);
517-
dev_priv->kernel_context = NULL;
545+
/* Keep the context so that we can free it immediately ourselves */
546+
ctx = i915_gem_context_get(fetch_and_zero(&i915->kernel_context));
547+
GEM_BUG_ON(!i915_gem_context_is_kernel(ctx));
548+
context_close(ctx);
549+
i915_gem_context_free(ctx);
518550

519-
ida_destroy(&dev_priv->contexts.hw_ida);
551+
/* Must free all deferred contexts (via flush_workqueue) first */
552+
ida_destroy(&i915->contexts.hw_ida);
520553
}
521554

522555
static int context_idr_cleanup(int id, void *p, void *data)
@@ -957,6 +990,10 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
957990
if (ret)
958991
return ret;
959992

993+
/* Reap stale contexts */
994+
i915_gem_retire_requests(dev_priv);
995+
contexts_free(dev_priv);
996+
960997
ctx = i915_gem_create_context(dev_priv, file_priv);
961998
mutex_unlock(&dev->struct_mutex);
962999
if (IS_ERR(ctx))

drivers/gpu/drm/i915/i915_gem_context.h

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ struct i915_gem_context {
8686

8787
/** link: place with &drm_i915_private.context_list */
8888
struct list_head link;
89+
struct llist_node free_link;
8990

9091
/**
9192
* @ref: reference count
@@ -284,7 +285,7 @@ void i915_gem_context_close(struct drm_file *file);
284285
int i915_switch_context(struct drm_i915_gem_request *req);
285286
int i915_gem_switch_to_kernel_context(struct drm_i915_private *dev_priv);
286287

287-
void i915_gem_context_free(struct kref *ctx_ref);
288+
void i915_gem_context_release(struct kref *ctx_ref);
288289
struct i915_gem_context *
289290
i915_gem_context_create_gvt(struct drm_device *dev);
290291

@@ -299,4 +300,16 @@ int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
299300
int i915_gem_context_reset_stats_ioctl(struct drm_device *dev, void *data,
300301
struct drm_file *file);
301302

303+
static inline struct i915_gem_context *
304+
i915_gem_context_get(struct i915_gem_context *ctx)
305+
{
306+
kref_get(&ctx->ref);
307+
return ctx;
308+
}
309+
310+
static inline void i915_gem_context_put(struct i915_gem_context *ctx)
311+
{
312+
kref_put(&ctx->ref, i915_gem_context_release);
313+
}
314+
302315
#endif /* !__I915_GEM_CONTEXT_H__ */

drivers/gpu/drm/i915/i915_perf.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2444,7 +2444,7 @@ static void i915_perf_destroy_locked(struct i915_perf_stream *stream)
24442444
list_del(&stream->link);
24452445

24462446
if (stream->ctx)
2447-
i915_gem_context_put_unlocked(stream->ctx);
2447+
i915_gem_context_put(stream->ctx);
24482448

24492449
kfree(stream);
24502450
}
@@ -2633,7 +2633,7 @@ i915_perf_open_ioctl_locked(struct drm_i915_private *dev_priv,
26332633
kfree(stream);
26342634
err_ctx:
26352635
if (specific_ctx)
2636-
i915_gem_context_put_unlocked(specific_ctx);
2636+
i915_gem_context_put(specific_ctx);
26372637
err:
26382638
return ret;
26392639
}

drivers/gpu/drm/i915/selftests/i915_vma.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,16 +186,20 @@ static int igt_vma_create(void *arg)
186186
goto end;
187187
}
188188

189-
list_for_each_entry_safe(ctx, cn, &contexts, link)
189+
list_for_each_entry_safe(ctx, cn, &contexts, link) {
190+
list_del_init(&ctx->link);
190191
mock_context_close(ctx);
192+
}
191193
}
192194

193195
end:
194196
/* Final pass to lookup all created contexts */
195197
err = create_vmas(i915, &objects, &contexts);
196198
out:
197-
list_for_each_entry_safe(ctx, cn, &contexts, link)
199+
list_for_each_entry_safe(ctx, cn, &contexts, link) {
200+
list_del_init(&ctx->link);
198201
mock_context_close(ctx);
202+
}
199203

200204
list_for_each_entry_safe(obj, on, &objects, st_link)
201205
i915_gem_object_put(obj);

drivers/gpu/drm/i915/selftests/mock_context.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,3 +86,12 @@ void mock_context_close(struct i915_gem_context *ctx)
8686

8787
i915_gem_context_put(ctx);
8888
}
89+
90+
void mock_init_contexts(struct drm_i915_private *i915)
91+
{
92+
INIT_LIST_HEAD(&i915->contexts.list);
93+
ida_init(&i915->contexts.hw_ida);
94+
95+
INIT_WORK(&i915->contexts.free_work, contexts_free_worker);
96+
init_llist_head(&i915->contexts.free_list);
97+
}

drivers/gpu/drm/i915/selftests/mock_context.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
#ifndef __MOCK_CONTEXT_H
2626
#define __MOCK_CONTEXT_H
2727

28+
void mock_init_contexts(struct drm_i915_private *i915);
29+
2830
struct i915_gem_context *
2931
mock_context(struct drm_i915_private *i915,
3032
const char *name);

drivers/gpu/drm/i915/selftests/mock_gem_device.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ static void mock_device_release(struct drm_device *dev)
5757

5858
cancel_delayed_work_sync(&i915->gt.retire_work);
5959
cancel_delayed_work_sync(&i915->gt.idle_work);
60+
flush_workqueue(i915->wq);
6061

6162
mutex_lock(&i915->drm.struct_mutex);
6263
for_each_engine(engine, i915, id)
@@ -160,7 +161,7 @@ struct drm_i915_private *mock_gem_device(void)
160161
INIT_LIST_HEAD(&i915->mm.unbound_list);
161162
INIT_LIST_HEAD(&i915->mm.bound_list);
162163

163-
ida_init(&i915->contexts.hw_ida);
164+
mock_init_contexts(i915);
164165

165166
INIT_DELAYED_WORK(&i915->gt.retire_work, mock_retire_work_handler);
166167
INIT_DELAYED_WORK(&i915->gt.idle_work, mock_idle_work_handler);

0 commit comments

Comments
 (0)