| Message ID | seffzgr3ptmwyrk4rmq4hn5lx5lljm6zoan4w752dovrai4ajg@slooah72lixx (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] drm/i915/selftests: avoid using uninitialized context | expand |
I sent an earlier version of this patch by mistake (it includes indentation errors) - I apologize for that, will fix this in v3 later. Sorry for the noise. Krzysztof On 2025-01-28 at 12:53:44 +0000, Krzysztof Karas wrote: > There is an error path in igt_ppgtt_alloc(), which leads to ww > object being passed down to i915_gem_ww_ctx_fini() without > initialization. Correct that by putting ppgtt->vm and returning > early. > > Fixes: 480ae79537b2 ("drm/i915/selftests: Prepare gtt tests for obj->mm.lock removal") > Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com> >
Hi Krzysztof > + if (!ppgtt->vm.allocate_va_range) { > + i915_vm_put(&ppgtt->vm); > + return 0; > + } I don't know if it feels more in line with kernel style, but consider changing it to a label before second `i915_vm_put` at end of function plus goto instead of creating new cleanup section. Otherwise looks alright. Reviewed-by: Mikolaj Wasiak <mikolaj.wasiak@intel.com> Mikolaj
Hi Mikolaj, > > + if (!ppgtt->vm.allocate_va_range) { > > + i915_vm_put(&ppgtt->vm); > > + return 0; > > + } > > I don't know if it feels more in line with kernel style, but consider > changing it to a label before second `i915_vm_put` at end of function > plus goto instead of creating new cleanup section. I do not mind, we can place a label as well, as long as we avoid using that uninitialized struct. I will include your suggestion in v3. > Otherwise looks alright. > Reviewed-by: Mikolaj Wasiak <mikolaj.wasiak@intel.com> Thanks for reviewing! Krzysztof
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c index 5816d515203a..d94a5b88f442 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c @@ -167,8 +167,10 @@ static int igt_ppgtt_alloc(void *arg) if (IS_ERR(ppgtt)) return PTR_ERR(ppgtt); - if (!ppgtt->vm.allocate_va_range) - goto err_ppgtt_cleanup; + if (!ppgtt->vm.allocate_va_range) { + i915_vm_put(&ppgtt->vm); + return 0; + } /* * While we only allocate the page tables here and so we could
There is an error path in igt_ppgtt_alloc(), which leads to ww object being passed down to i915_gem_ww_ctx_fini() without initialization. Correct that by putting ppgtt->vm and returning early. Fixes: 480ae79537b2 ("drm/i915/selftests: Prepare gtt tests for obj->mm.lock removal") Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com> --- Changelog: * v1 -> v2: Avoid calling i915_gem_ww_ctx_fini() with zeroed context by returning early (Sebastian). drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)