* [PATCH] drm/i915: In render_state_init reset obj in teardown path @ 2016-04-26 9:21 Matthew Auld 2016-04-26 10:04 ` Dave Gordon 2016-04-26 11:24 ` ✗ Fi.CI.BAT: failure for " Patchwork 0 siblings, 2 replies; 4+ messages in thread From: Matthew Auld @ 2016-04-26 9:21 UTC (permalink / raw) To: intel-gfx The teardown path in render_state_init leaves so->obj != NULL. Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Signed-off-by: Matthew Auld <matthew.auld@intel.com> --- drivers/gpu/drm/i915/i915_gem_render_state.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c index 71611bf..10f3cf0 100644 --- a/drivers/gpu/drm/i915/i915_gem_render_state.c +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c @@ -70,6 +70,7 @@ static int render_state_init(struct render_state *so, struct drm_device *dev) free_gem: drm_gem_object_unreference(&so->obj->base); + so->obj = NULL; return ret; } -- 2.4.11 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: In render_state_init reset obj in teardown path 2016-04-26 9:21 [PATCH] drm/i915: In render_state_init reset obj in teardown path Matthew Auld @ 2016-04-26 10:04 ` Dave Gordon 2016-04-27 13:51 ` Joonas Lahtinen 2016-04-26 11:24 ` ✗ Fi.CI.BAT: failure for " Patchwork 1 sibling, 1 reply; 4+ messages in thread From: Dave Gordon @ 2016-04-26 10:04 UTC (permalink / raw) To: intel-gfx On 26/04/16 10:21, Matthew Auld wrote: > The teardown path in render_state_init leaves so->obj != NULL. > > Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > --- > drivers/gpu/drm/i915/i915_gem_render_state.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c > index 71611bf..10f3cf0 100644 > --- a/drivers/gpu/drm/i915/i915_gem_render_state.c > +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c > @@ -70,6 +70,7 @@ static int render_state_init(struct render_state *so, struct drm_device *dev) > > free_gem: > drm_gem_object_unreference(&so->obj->base); > + so->obj = NULL; > return ret; > } It doesn't actually matter, because 'so' is pointing to a local object a few frames up the callstack. But I don't think this function is entitled to assume that; it should leave everything in a consistent state so there aren't any dangling pointers to objects that have been freed lying around - someday the argument may turn into a per-engine static or some other long-lived thing. So, Reviewed-by: Dave Gordon <david.s.gordon@intel.com> _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: In render_state_init reset obj in teardown path 2016-04-26 10:04 ` Dave Gordon @ 2016-04-27 13:51 ` Joonas Lahtinen 0 siblings, 0 replies; 4+ messages in thread From: Joonas Lahtinen @ 2016-04-27 13:51 UTC (permalink / raw) To: Matthew Auld; +Cc: intel-gfx On ti, 2016-04-26 at 11:04 +0100, Dave Gordon wrote: > On 26/04/16 10:21, Matthew Auld wrote: > > > > The teardown path in render_state_init leaves so->obj != NULL. > > > > Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > > --- > > drivers/gpu/drm/i915/i915_gem_render_state.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c > > index 71611bf..10f3cf0 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_render_state.c > > +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c > > @@ -70,6 +70,7 @@ static int render_state_init(struct render_state *so, struct drm_device *dev) > > > > free_gem: > > drm_gem_object_unreference(&so->obj->base); > > + so->obj = NULL; This is object_init style function, if it fails, the contents of "so" is expected to be uninitialized, and should only be freed or attempted to re-initialize by caller, never inspected, so no need for this. See gen8_ppgtt_init for an example, it would be rather cubersome to undo all assignments on error. In short, I do not see this as a necessary step. Regards, Joonas > > return ret; > > } > It doesn't actually matter, because 'so' is pointing to a local object a > few frames up the callstack. But I don't think this function is entitled > to assume that; it should leave everything in a consistent state so > there aren't any dangling pointers to objects that have been freed lying > around - someday the argument may turn into a per-engine static or some > other long-lived thing. So, > > Reviewed-by: Dave Gordon <david.s.gordon@intel.com> > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Joonas Lahtinen Open Source Technology Center Intel Corporation _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 4+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: In render_state_init reset obj in teardown path 2016-04-26 9:21 [PATCH] drm/i915: In render_state_init reset obj in teardown path Matthew Auld 2016-04-26 10:04 ` Dave Gordon @ 2016-04-26 11:24 ` Patchwork 1 sibling, 0 replies; 4+ messages in thread From: Patchwork @ 2016-04-26 11:24 UTC (permalink / raw) To: Matthew Auld; +Cc: intel-gfx == Series Details == Series: drm/i915: In render_state_init reset obj in teardown path URL : https://patchwork.freedesktop.org/series/6314/ State : failure == Summary == Series 6314v1 drm/i915: In render_state_init reset obj in teardown path http://patchwork.freedesktop.org/api/1.0/series/6314/revisions/1/mbox/ Test kms_frontbuffer_tracking: Subgroup basic: pass -> FAIL (hsw-gt2) Test kms_pipe_crc_basic: Subgroup read-crc-pipe-a: pass -> SKIP (hsw-brixbox) 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 byt-nuc total:199 pass:158 dwarn:0 dfail:0 fail:0 skip:41 hsw-brixbox total:200 pass:173 dwarn:0 dfail:0 fail:0 skip:27 hsw-gt2 total:200 pass:177 dwarn:0 dfail:0 fail:2 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:158 dwarn:0 dfail:0 fail:1 skip:41 Results at /archive/results/CI_IGT_test/Patchwork_2069/ f028bc7dba69ef340ff5139e06fb332a62bc8455 drm-intel-nightly: 2016y-04m-26d-10h-05m-40s UTC integration manifest 09b3bbe drm/i915: In render_state_init reset obj in teardown path _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-04-27 13:50 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-04-26 9:21 [PATCH] drm/i915: In render_state_init reset obj in teardown path Matthew Auld 2016-04-26 10:04 ` Dave Gordon 2016-04-27 13:51 ` Joonas Lahtinen 2016-04-26 11:24 ` ✗ Fi.CI.BAT: failure for " Patchwork
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox