| Message ID | 1461759565-12086-1-git-send-email-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Matthew Auld <matthew.auld@intel.com> writes: > [ text/plain ] > Prefer a goto teardown path to do all the required cleanup. > > v2: > (Joonas Lahtinen) > - remove NULL assignments > - rename goto labels > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Cc: Dave Gordon <david.s.gordon@intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Signed-off-by: Matthew Auld <matthew.auld@intel.com> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> > --- > drivers/gpu/drm/i915/i915_gem_gtt.c | 25 ++++++++++++++++--------- > 1 file changed, 16 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 0d666b3..87947ec 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -866,6 +866,7 @@ static void gen8_free_page_tables(struct drm_device *dev, > static int gen8_init_scratch(struct i915_address_space *vm) > { > struct drm_device *dev = vm->dev; > + int ret; > > vm->scratch_page = alloc_scratch_page(dev); > if (IS_ERR(vm->scratch_page)) > @@ -873,24 +874,21 @@ static int gen8_init_scratch(struct i915_address_space *vm) > > vm->scratch_pt = alloc_pt(dev); > if (IS_ERR(vm->scratch_pt)) { > - free_scratch_page(dev, vm->scratch_page); > - return PTR_ERR(vm->scratch_pt); > + ret = PTR_ERR(vm->scratch_pt); > + goto free_scratch_page; > } > > vm->scratch_pd = alloc_pd(dev); > if (IS_ERR(vm->scratch_pd)) { > - free_pt(dev, vm->scratch_pt); > - free_scratch_page(dev, vm->scratch_page); > - return PTR_ERR(vm->scratch_pd); > + ret = PTR_ERR(vm->scratch_pd); > + goto free_pt; > } > > if (USES_FULL_48BIT_PPGTT(dev)) { > vm->scratch_pdp = alloc_pdp(dev); > if (IS_ERR(vm->scratch_pdp)) { > - free_pd(dev, vm->scratch_pd); > - free_pt(dev, vm->scratch_pt); > - free_scratch_page(dev, vm->scratch_page); > - return PTR_ERR(vm->scratch_pdp); > + ret = PTR_ERR(vm->scratch_pdp); > + goto free_pd; > } > } > > @@ -900,6 +898,15 @@ static int gen8_init_scratch(struct i915_address_space *vm) > gen8_initialize_pdp(vm, vm->scratch_pdp); > > return 0; > + > +free_pd: > + free_pd(dev, vm->scratch_pd); > +free_pt: > + free_pt(dev, vm->scratch_pt); > +free_scratch_page: > + free_scratch_page(dev, vm->scratch_page); > + > + return ret; > } > > static int gen8_ppgtt_notify_vgt(struct i915_hw_ppgtt *ppgtt, bool create) > -- > 2.4.11
Patchwork <patchwork@emeril.freedesktop.org> writes: > [ text/plain ] > == Series Details == > > Series: drm/i915: tidy up gen8_init_scratch (rev2) > URL : https://patchwork.freedesktop.org/series/6315/ > State : failure > > == Summary == > > Series 6315v2 drm/i915: tidy up gen8_init_scratch > http://patchwork.freedesktop.org/api/1.0/series/6315/revisions/2/mbox/ > > Test drv_module_reload_basic: > dmesg-warn -> PASS (snb-x220t) > Test gem_exec_suspend: > Subgroup basic-s3: > fail -> PASS (snb-x220t) > Test kms_flip: > Subgroup basic-flip-vs-wf_vblank: > pass -> FAIL (snb-x220t) https://bugs.freedesktop.org/show_bug.cgi?id=94294 > > bdw-nuci7 total:200 pass:188 dwarn:0 dfail:0 fail:0 skip:12 > bdw-ultra total:200 pass:175 dwarn:0 dfail:0 fail:0 skip:25 > bsw-nuc-2 total:199 pass:158 dwarn:0 dfail:0 fail:0 skip:41 > hsw-brixbox total:200 pass:174 dwarn:0 dfail:0 fail:0 skip:26 > hsw-gt2 total:200 pass:178 dwarn:0 dfail:0 fail:1 skip:21 > ilk-hp8440p total:200 pass:139 dwarn:0 dfail:0 fail:0 skip:61 > ivb-t430s total:200 pass:169 dwarn:0 dfail:0 fail:0 skip:31 > skl-i7k-2 total:200 pass:173 dwarn:0 dfail:0 fail:0 skip:27 > skl-nuci5 total:200 pass:189 dwarn:0 dfail:0 fail:0 skip:11 > snb-dellxps total:200 pass:158 dwarn:0 dfail:0 fail:0 skip:42 > snb-x220t total:200 pass:157 dwarn:0 dfail:0 fail:2 skip:41 > > Results at /archive/results/CI_IGT_test/Patchwork_2089/ > > 4fa405ab5848b76c8568c7fb771d389a6695108c drm-intel-nightly: 2016y-04m-27d-10h-47m-35s UTC integration manifest > 6107550 drm/i915: tidy up gen8_init_scratch > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On ke, 2016-04-27 at 15:30 +0300, Mika Kuoppala wrote: > Matthew Auld <matthew.auld@intel.com> writes: > > > > > [ text/plain ] > > Prefer a goto teardown path to do all the required cleanup. > > > > v2: > > (Joonas Lahtinen) > > - remove NULL assignments > > - rename goto labels > > > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > Cc: Dave Gordon <david.s.gordon@intel.com> > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > > > --- > > drivers/gpu/drm/i915/i915_gem_gtt.c | 25 ++++++++++++++++--------- > > 1 file changed, 16 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > > index 0d666b3..87947ec 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > @@ -866,6 +866,7 @@ static void gen8_free_page_tables(struct drm_device *dev, > > static int gen8_init_scratch(struct i915_address_space *vm) > > { > > struct drm_device *dev = vm->dev; > > + int ret; > > > > vm->scratch_page = alloc_scratch_page(dev); > > if (IS_ERR(vm->scratch_page)) > > @@ -873,24 +874,21 @@ static int gen8_init_scratch(struct i915_address_space *vm) > > > > vm->scratch_pt = alloc_pt(dev); > > if (IS_ERR(vm->scratch_pt)) { > > - free_scratch_page(dev, vm->scratch_page); > > - return PTR_ERR(vm->scratch_pt); > > + ret = PTR_ERR(vm->scratch_pt); > > + goto free_scratch_page; > > } > > > > vm->scratch_pd = alloc_pd(dev); > > if (IS_ERR(vm->scratch_pd)) { > > - free_pt(dev, vm->scratch_pt); > > - free_scratch_page(dev, vm->scratch_page); > > - return PTR_ERR(vm->scratch_pd); > > + ret = PTR_ERR(vm->scratch_pd); > > + goto free_pt; > > } > > > > if (USES_FULL_48BIT_PPGTT(dev)) { > > vm->scratch_pdp = alloc_pdp(dev); > > if (IS_ERR(vm->scratch_pdp)) { > > - free_pd(dev, vm->scratch_pd); > > - free_pt(dev, vm->scratch_pt); > > - free_scratch_page(dev, vm->scratch_page); > > - return PTR_ERR(vm->scratch_pdp); > > + ret = PTR_ERR(vm->scratch_pdp); > > + goto free_pd; > > } > > } > > > > @@ -900,6 +898,15 @@ static int gen8_init_scratch(struct i915_address_space *vm) > > gen8_initialize_pdp(vm, vm->scratch_pdp); > > > > return 0; > > + > > +free_pd: > > + free_pd(dev, vm->scratch_pd); > > +free_pt: > > + free_pt(dev, vm->scratch_pt); > > +free_scratch_page: > > + free_scratch_page(dev, vm->scratch_page); > > + > > + return ret; > > } > > > > static int gen8_ppgtt_notify_vgt(struct i915_hw_ppgtt *ppgtt, bool create)
On ke, 2016-04-27 at 12:49 +0000, Patchwork wrote: > == Series Details == > > Series: drm/i915: tidy up gen8_init_scratch (rev2) > URL : https://patchwork.freedesktop.org/series/6315/ > State : failure > > == Summary == > > Series 6315v2 drm/i915: tidy up gen8_init_scratch > http://patchwork.freedesktop.org/api/1.0/series/6315/revisions/2/mbox/ > > Test drv_module_reload_basic: > dmesg-warn -> PASS (snb-x220t) > Test gem_exec_suspend: > Subgroup basic-s3: > fail -> PASS (snb-x220t) > Test kms_flip: > Subgroup basic-flip-vs-wf_vblank: > pass -> FAIL (snb-x220t) > Already reported; (kms_flip:6253) CRITICAL: Test assertion failure function check_state, file kms_flip.c:698: (kms_flip:6253) CRITICAL: Failed assertion: fabs((((double) diff.tv_usec) - usec_interflip) / usec_interflip) <= 0.005 (kms_flip:6253) CRITICAL: Last errno: 25, Inappropriate ioctl for device (kms_flip:6253) CRITICAL: inter-vblank ts jitter: 0s, 183886usec Subtest basic-flip-vs-wf_vblank failed. https://bugs.freedesktop.org/show_bug.cgi?id=94294 So merging in. Thanks for the patch. > bdw-nuci7 total:200 pass:188 dwarn:0 dfail:0 fail:0 skip:12 > bdw-ultra total:200 pass:175 dwarn:0 dfail:0 fail:0 skip:25 > bsw-nuc-2 total:199 pass:158 dwarn:0 dfail:0 fail:0 skip:41 > hsw-brixbox total:200 pass:174 dwarn:0 dfail:0 fail:0 skip:26 > hsw-gt2 total:200 pass:178 dwarn:0 dfail:0 fail:1 skip:21 > ilk-hp8440p total:200 pass:139 dwarn:0 dfail:0 fail:0 skip:61 > ivb-t430s total:200 pass:169 dwarn:0 dfail:0 fail:0 skip:31 > skl-i7k-2 total:200 pass:173 dwarn:0 dfail:0 fail:0 skip:27 > skl-nuci5 total:200 pass:189 dwarn:0 dfail:0 fail:0 skip:11 > snb-dellxps total:200 pass:158 dwarn:0 dfail:0 fail:0 skip:42 > snb-x220t total:200 pass:157 dwarn:0 dfail:0 fail:2 skip:41 > > Results at /archive/results/CI_IGT_test/Patchwork_2089/ > > 4fa405ab5848b76c8568c7fb771d389a6695108c drm-intel-nightly: 2016y-04m-27d-10h-47m-35s UTC integration manifest > 6107550 drm/i915: tidy up gen8_init_scratch > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 0d666b3..87947ec 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -866,6 +866,7 @@ static void gen8_free_page_tables(struct drm_device *dev, static int gen8_init_scratch(struct i915_address_space *vm) { struct drm_device *dev = vm->dev; + int ret; vm->scratch_page = alloc_scratch_page(dev); if (IS_ERR(vm->scratch_page)) @@ -873,24 +874,21 @@ static int gen8_init_scratch(struct i915_address_space *vm) vm->scratch_pt = alloc_pt(dev); if (IS_ERR(vm->scratch_pt)) { - free_scratch_page(dev, vm->scratch_page); - return PTR_ERR(vm->scratch_pt); + ret = PTR_ERR(vm->scratch_pt); + goto free_scratch_page; } vm->scratch_pd = alloc_pd(dev); if (IS_ERR(vm->scratch_pd)) { - free_pt(dev, vm->scratch_pt); - free_scratch_page(dev, vm->scratch_page); - return PTR_ERR(vm->scratch_pd); + ret = PTR_ERR(vm->scratch_pd); + goto free_pt; } if (USES_FULL_48BIT_PPGTT(dev)) { vm->scratch_pdp = alloc_pdp(dev); if (IS_ERR(vm->scratch_pdp)) { - free_pd(dev, vm->scratch_pd); - free_pt(dev, vm->scratch_pt); - free_scratch_page(dev, vm->scratch_page); - return PTR_ERR(vm->scratch_pdp); + ret = PTR_ERR(vm->scratch_pdp); + goto free_pd; } } @@ -900,6 +898,15 @@ static int gen8_init_scratch(struct i915_address_space *vm) gen8_initialize_pdp(vm, vm->scratch_pdp); return 0; + +free_pd: + free_pd(dev, vm->scratch_pd); +free_pt: + free_pt(dev, vm->scratch_pt); +free_scratch_page: + free_scratch_page(dev, vm->scratch_page); + + return ret; } static int gen8_ppgtt_notify_vgt(struct i915_hw_ppgtt *ppgtt, bool create)
Prefer a goto teardown path to do all the required cleanup. v2: (Joonas Lahtinen) - remove NULL assignments - rename goto labels Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Dave Gordon <david.s.gordon@intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Signed-off-by: Matthew Auld <matthew.auld@intel.com> --- drivers/gpu/drm/i915/i915_gem_gtt.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-)