Skip to content

Commit 20e377e

Browse files
ickle-inteltursulin
authored andcommitted
drm/i915/gt: Use i915_vm_put on ppgtt_create error paths
Now that the scratch page and page directories have a reference back to the i915_address_space, we cannot do an immediate free of the ppgtt upon error as those buffer objects will perform a later i915_vm_put in their deferred frees. The downside is that by replacing the onion unwind along the error paths, the ppgtt cleanup must handle a partially constructed vm. This includes ensuring that the vm->cleanup is set prior to the error path. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/6900 Signed-off-by: Chris Wilson <[email protected]> Fixes: 4d8151a ("drm/i915: Don't free shared locks while shared") Cc: Thomas Hellström <[email protected]> Cc: Matthew Auld <[email protected]> Cc: <[email protected]> # v5.14+ Reviewed-by: Matthew Auld <[email protected]> Signed-off-by: Matthew Auld <[email protected]> Link: https://patchwork.freedesktop.org/patch/msgid/[email protected] (cherry picked from commit c286558) Signed-off-by: Tvrtko Ursulin <[email protected]>
1 parent a50ab1b commit 20e377e

File tree

3 files changed

+41
-36
lines changed

3 files changed

+41
-36
lines changed

drivers/gpu/drm/i915/gt/gen6_ppgtt.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ static int gen6_ppgtt_init_scratch(struct gen6_ppgtt *ppgtt)
247247
i915_gem_object_put(vm->scratch[1]);
248248
err_scratch0:
249249
i915_gem_object_put(vm->scratch[0]);
250+
vm->scratch[0] = NULL;
250251
return ret;
251252
}
252253

@@ -268,9 +269,10 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm)
268269
gen6_ppgtt_free_pd(ppgtt);
269270
free_scratch(vm);
270271

271-
mutex_destroy(&ppgtt->flush);
272+
if (ppgtt->base.pd)
273+
free_pd(&ppgtt->base.vm, ppgtt->base.pd);
272274

273-
free_pd(&ppgtt->base.vm, ppgtt->base.pd);
275+
mutex_destroy(&ppgtt->flush);
274276
}
275277

276278
static void pd_vma_bind(struct i915_address_space *vm,
@@ -449,19 +451,17 @@ struct i915_ppgtt *gen6_ppgtt_create(struct intel_gt *gt)
449451

450452
err = gen6_ppgtt_init_scratch(ppgtt);
451453
if (err)
452-
goto err_free;
454+
goto err_put;
453455

454456
ppgtt->base.pd = gen6_alloc_top_pd(ppgtt);
455457
if (IS_ERR(ppgtt->base.pd)) {
456458
err = PTR_ERR(ppgtt->base.pd);
457-
goto err_scratch;
459+
goto err_put;
458460
}
459461

460462
return &ppgtt->base;
461463

462-
err_scratch:
463-
free_scratch(&ppgtt->base.vm);
464-
err_free:
465-
kfree(ppgtt);
464+
err_put:
465+
i915_vm_put(&ppgtt->base.vm);
466466
return ERR_PTR(err);
467467
}

drivers/gpu/drm/i915/gt/gen8_ppgtt.c

Lines changed: 30 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,10 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
196196
if (intel_vgpu_active(vm->i915))
197197
gen8_ppgtt_notify_vgt(ppgtt, false);
198198

199-
__gen8_ppgtt_cleanup(vm, ppgtt->pd, gen8_pd_top_count(vm), vm->top);
199+
if (ppgtt->pd)
200+
__gen8_ppgtt_cleanup(vm, ppgtt->pd,
201+
gen8_pd_top_count(vm), vm->top);
202+
200203
free_scratch(vm);
201204
}
202205

@@ -803,8 +806,10 @@ static int gen8_init_scratch(struct i915_address_space *vm)
803806
struct drm_i915_gem_object *obj;
804807

805808
obj = vm->alloc_pt_dma(vm, I915_GTT_PAGE_SIZE_4K);
806-
if (IS_ERR(obj))
809+
if (IS_ERR(obj)) {
810+
ret = PTR_ERR(obj);
807811
goto free_scratch;
812+
}
808813

809814
ret = map_pt_dma(vm, obj);
810815
if (ret) {
@@ -823,7 +828,8 @@ static int gen8_init_scratch(struct i915_address_space *vm)
823828
free_scratch:
824829
while (i--)
825830
i915_gem_object_put(vm->scratch[i]);
826-
return -ENOMEM;
831+
vm->scratch[0] = NULL;
832+
return ret;
827833
}
828834

829835
static int gen8_preallocate_top_level_pdp(struct i915_ppgtt *ppgtt)
@@ -901,6 +907,7 @@ gen8_alloc_top_pd(struct i915_address_space *vm)
901907
struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt *gt,
902908
unsigned long lmem_pt_obj_flags)
903909
{
910+
struct i915_page_directory *pd;
904911
struct i915_ppgtt *ppgtt;
905912
int err;
906913

@@ -946,21 +953,7 @@ struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt *gt,
946953
ppgtt->vm.alloc_scratch_dma = alloc_pt_dma;
947954
}
948955

949-
err = gen8_init_scratch(&ppgtt->vm);
950-
if (err)
951-
goto err_free;
952-
953-
ppgtt->pd = gen8_alloc_top_pd(&ppgtt->vm);
954-
if (IS_ERR(ppgtt->pd)) {
955-
err = PTR_ERR(ppgtt->pd);
956-
goto err_free_scratch;
957-
}
958-
959-
if (!i915_vm_is_4lvl(&ppgtt->vm)) {
960-
err = gen8_preallocate_top_level_pdp(ppgtt);
961-
if (err)
962-
goto err_free_pd;
963-
}
956+
ppgtt->vm.pte_encode = gen8_pte_encode;
964957

965958
ppgtt->vm.bind_async_flags = I915_VMA_LOCAL_BIND;
966959
ppgtt->vm.insert_entries = gen8_ppgtt_insert;
@@ -971,22 +964,31 @@ struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt *gt,
971964
ppgtt->vm.allocate_va_range = gen8_ppgtt_alloc;
972965
ppgtt->vm.clear_range = gen8_ppgtt_clear;
973966
ppgtt->vm.foreach = gen8_ppgtt_foreach;
967+
ppgtt->vm.cleanup = gen8_ppgtt_cleanup;
974968

975-
ppgtt->vm.pte_encode = gen8_pte_encode;
969+
err = gen8_init_scratch(&ppgtt->vm);
970+
if (err)
971+
goto err_put;
972+
973+
pd = gen8_alloc_top_pd(&ppgtt->vm);
974+
if (IS_ERR(pd)) {
975+
err = PTR_ERR(pd);
976+
goto err_put;
977+
}
978+
ppgtt->pd = pd;
979+
980+
if (!i915_vm_is_4lvl(&ppgtt->vm)) {
981+
err = gen8_preallocate_top_level_pdp(ppgtt);
982+
if (err)
983+
goto err_put;
984+
}
976985

977986
if (intel_vgpu_active(gt->i915))
978987
gen8_ppgtt_notify_vgt(ppgtt, true);
979988

980-
ppgtt->vm.cleanup = gen8_ppgtt_cleanup;
981-
982989
return ppgtt;
983990

984-
err_free_pd:
985-
__gen8_ppgtt_cleanup(&ppgtt->vm, ppgtt->pd,
986-
gen8_pd_top_count(&ppgtt->vm), ppgtt->vm.top);
987-
err_free_scratch:
988-
free_scratch(&ppgtt->vm);
989-
err_free:
990-
kfree(ppgtt);
991+
err_put:
992+
i915_vm_put(&ppgtt->vm);
991993
return ERR_PTR(err);
992994
}

drivers/gpu/drm/i915/gt/intel_gtt.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,9 @@ void free_scratch(struct i915_address_space *vm)
405405
{
406406
int i;
407407

408+
if (!vm->scratch[0])
409+
return;
410+
408411
for (i = 0; i <= vm->top; i++)
409412
i915_gem_object_put(vm->scratch[i]);
410413
}

0 commit comments

Comments
 (0)