* [PATCH] drm/i915: Free resources correctly if we cannot map status page during ctx create @ 2014-11-17 15:48 Arun Siluvery 2014-11-17 15:54 ` Daniel, Thomas 2014-11-17 19:04 ` Daniel Vetter 0 siblings, 2 replies; 8+ messages in thread From: Arun Siluvery @ 2014-11-17 15:48 UTC (permalink / raw) To: intel-gfx We are not freeing memory allocated for ringbuf and ctx if we fail to map status page so release all resources correctly. Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> --- drivers/gpu/drm/i915/intel_lrc.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index f3efdbd..a84d24b 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1777,8 +1777,10 @@ int intel_lr_context_deferred_create(struct intel_context *ctx, ring->status_page.gfx_addr = i915_gem_obj_ggtt_offset(ctx_obj); ring->status_page.page_addr = kmap(sg_page(ctx_obj->pages->sgl)); - if (ring->status_page.page_addr == NULL) - return -ENOMEM; + if (ring->status_page.page_addr == NULL) { + ret = -ENOMEM; + goto error; + } ring->status_page.obj = ctx_obj; } -- 2.1.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Free resources correctly if we cannot map status page during ctx create 2014-11-17 15:48 [PATCH] drm/i915: Free resources correctly if we cannot map status page during ctx create Arun Siluvery @ 2014-11-17 15:54 ` Daniel, Thomas 2014-11-17 15:57 ` Siluvery, Arun 2014-11-17 19:04 ` Daniel Vetter 1 sibling, 1 reply; 8+ messages in thread From: Daniel, Thomas @ 2014-11-17 15:54 UTC (permalink / raw) To: Arun Siluvery, intel-gfx@lists.freedesktop.org [-- Attachment #1.1: Type: text/plain, Size: 1893 bytes --] > -----Original Message----- > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf > Of Arun Siluvery > Sent: Monday, November 17, 2014 3:48 PM > To: intel-gfx@lists.freedesktop.org > Subject: [Intel-gfx] [PATCH] drm/i915: Free resources correctly if we cannot > map status page during ctx create > > We are not freeing memory allocated for ringbuf and ctx if we fail to map > status page so release all resources correctly. > > Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com<mailto:arun.siluvery@linux.intel.com>> > --- > drivers/gpu/drm/i915/intel_lrc.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > b/drivers/gpu/drm/i915/intel_lrc.c > index f3efdbd..a84d24b 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -1777,8 +1777,10 @@ int intel_lr_context_deferred_create(struct > intel_context *ctx, > ring->status_page.gfx_addr = > i915_gem_obj_ggtt_offset(ctx_obj); > ring->status_page.page_addr = > kmap(sg_page(ctx_obj->pages->sgl)); > - if (ring->status_page.page_addr == NULL) > - return -ENOMEM; > + if (ring->status_page.page_addr == NULL) { > + ret = -ENOMEM; > + goto error; > + } > ring->status_page.obj = ctx_obj; > } Hi Arun, I think your tree is out of date. See this patch: http://patchwork.freedesktop.org/patch/35828/ Cheers, Thomas. [-- Attachment #1.2: Type: text/html, Size: 7636 bytes --] [-- Attachment #2: Type: text/plain, Size: 159 bytes --] _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Free resources correctly if we cannot map status page during ctx create 2014-11-17 15:54 ` Daniel, Thomas @ 2014-11-17 15:57 ` Siluvery, Arun 0 siblings, 0 replies; 8+ messages in thread From: Siluvery, Arun @ 2014-11-17 15:57 UTC (permalink / raw) To: Daniel, Thomas, intel-gfx@lists.freedesktop.org On 17/11/2014 15:54, Daniel, Thomas wrote: >> -----Original Message----- > >> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf > >> Of Arun Siluvery > >> Sent: Monday, November 17, 2014 3:48 PM > >> To: intel-gfx@lists.freedesktop.org > >> Subject: [Intel-gfx] [PATCH] drm/i915: Free resources correctly if we cannot > >> map status page during ctx create > >> > >> We are not freeing memory allocated for ringbuf and ctx if we fail to map > >> status page so release all resources correctly. > >> > >> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com<mailto:arun.siluvery@linux.intel.com>> > >> --- > >> drivers/gpu/drm/i915/intel_lrc.c | 6 ++++-- > >> 1 file changed, 4 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_lrc.c > >> b/drivers/gpu/drm/i915/intel_lrc.c > >> index f3efdbd..a84d24b 100644 > >> --- a/drivers/gpu/drm/i915/intel_lrc.c > >> +++ b/drivers/gpu/drm/i915/intel_lrc.c > >> @@ -1777,8 +1777,10 @@ int intel_lr_context_deferred_create(struct > >> intel_context *ctx, > >> ring->status_page.gfx_addr = > >> i915_gem_obj_ggtt_offset(ctx_obj); > >> ring->status_page.page_addr = > >> kmap(sg_page(ctx_obj->pages->sgl)); > >> - if (ring->status_page.page_addr == NULL) > >> - return -ENOMEM; > >> + if (ring->status_page.page_addr == NULL) { > >> + ret = -ENOMEM; > >> + goto error; > >> + } > >> ring->status_page.obj = ctx_obj; > >> } > > > > > > Hi Arun, > > > > I think your tree is out of date. See this patch: > > http://patchwork.freedesktop.org/patch/35828/ > > > > Cheers, > > Thomas. > > You are right, I don't have latest changes. This patch can be ignored. regards Arun _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Free resources correctly if we cannot map status page during ctx create 2014-11-17 15:48 [PATCH] drm/i915: Free resources correctly if we cannot map status page during ctx create Arun Siluvery 2014-11-17 15:54 ` Daniel, Thomas @ 2014-11-17 19:04 ` Daniel Vetter 2014-11-18 7:44 ` Chris Wilson 1 sibling, 1 reply; 8+ messages in thread From: Daniel Vetter @ 2014-11-17 19:04 UTC (permalink / raw) To: Arun Siluvery; +Cc: intel-gfx On Mon, Nov 17, 2014 at 03:48:27PM +0000, Arun Siluvery wrote: > We are not freeing memory allocated for ringbuf and ctx if we fail > to map status page so release all resources correctly. > > Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_lrc.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index f3efdbd..a84d24b 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -1777,8 +1777,10 @@ int intel_lr_context_deferred_create(struct intel_context *ctx, > ring->status_page.gfx_addr = i915_gem_obj_ggtt_offset(ctx_obj); > ring->status_page.page_addr = > kmap(sg_page(ctx_obj->pages->sgl)); > - if (ring->status_page.page_addr == NULL) > - return -ENOMEM; > + if (ring->status_page.page_addr == NULL) { > + ret = -ENOMEM; > + goto error; > + } Since this popped up: Do we have an automated igt testcase to exercise this corner-case? We do have it for legacy (gen6/7) contexts, including exercising the shrinker logic to get rid of misplaced ctx objects to avoid ggtt fragmentation hassles ... -Daniel > ring->status_page.obj = ctx_obj; > } > > -- > 2.1.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Free resources correctly if we cannot map status page during ctx create 2014-11-17 19:04 ` Daniel Vetter @ 2014-11-18 7:44 ` Chris Wilson 2014-11-18 8:05 ` Daniel Vetter 2014-11-18 8:10 ` [PATCH] drm/i915: Drop return value from lrc_setup_hardware_status_page Daniel Vetter 0 siblings, 2 replies; 8+ messages in thread From: Chris Wilson @ 2014-11-18 7:44 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx On Mon, Nov 17, 2014 at 08:04:09PM +0100, Daniel Vetter wrote: > On Mon, Nov 17, 2014 at 03:48:27PM +0000, Arun Siluvery wrote: > > We are not freeing memory allocated for ringbuf and ctx if we fail > > to map status page so release all resources correctly. > > > > Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_lrc.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > > index f3efdbd..a84d24b 100644 > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > @@ -1777,8 +1777,10 @@ int intel_lr_context_deferred_create(struct intel_context *ctx, > > ring->status_page.gfx_addr = i915_gem_obj_ggtt_offset(ctx_obj); > > ring->status_page.page_addr = > > kmap(sg_page(ctx_obj->pages->sgl)); > > - if (ring->status_page.page_addr == NULL) > > - return -ENOMEM; > > + if (ring->status_page.page_addr == NULL) { > > + ret = -ENOMEM; > > + goto error; > > + } > > Since this popped up: Do we have an automated igt testcase to exercise > this corner-case? How? kmap() never returns NULL. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Free resources correctly if we cannot map status page during ctx create 2014-11-18 7:44 ` Chris Wilson @ 2014-11-18 8:05 ` Daniel Vetter 2014-11-18 8:10 ` [PATCH] drm/i915: Drop return value from lrc_setup_hardware_status_page Daniel Vetter 1 sibling, 0 replies; 8+ messages in thread From: Daniel Vetter @ 2014-11-18 8:05 UTC (permalink / raw) To: Chris Wilson, Daniel Vetter, Arun Siluvery, intel-gfx On Tue, Nov 18, 2014 at 07:44:40AM +0000, Chris Wilson wrote: > On Mon, Nov 17, 2014 at 08:04:09PM +0100, Daniel Vetter wrote: > > On Mon, Nov 17, 2014 at 03:48:27PM +0000, Arun Siluvery wrote: > > > We are not freeing memory allocated for ringbuf and ctx if we fail > > > to map status page so release all resources correctly. > > > > > > Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_lrc.c | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > > > index f3efdbd..a84d24b 100644 > > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > > @@ -1777,8 +1777,10 @@ int intel_lr_context_deferred_create(struct intel_context *ctx, > > > ring->status_page.gfx_addr = i915_gem_obj_ggtt_offset(ctx_obj); > > > ring->status_page.page_addr = > > > kmap(sg_page(ctx_obj->pages->sgl)); > > > - if (ring->status_page.page_addr == NULL) > > > - return -ENOMEM; > > > + if (ring->status_page.page_addr == NULL) { > > > + ret = -ENOMEM; > > > + goto error; > > > + } > > > > Since this popped up: Do we have an automated igt testcase to exercise > > this corner-case? > > How? kmap() never returns NULL. Oh dear. /me hangs head in shame I'll do a patch to correct this. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] drm/i915: Drop return value from lrc_setup_hardware_status_page 2014-11-18 7:44 ` Chris Wilson 2014-11-18 8:05 ` Daniel Vetter @ 2014-11-18 8:10 ` Daniel Vetter 2014-11-18 15:31 ` [PATCH] drm/i915: Drop return value from shuang.he 1 sibling, 1 reply; 8+ messages in thread From: Daniel Vetter @ 2014-11-18 8:10 UTC (permalink / raw) To: Intel Graphics Development; +Cc: Daniel Vetter, Daniel Vetter kmap never fails. Spotted-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Arun Siluvery <arun.siluvery@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/i915/intel_lrc.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index b7c4c9ab9012..3cf15c4da0e8 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -1706,7 +1706,7 @@ static uint32_t get_lr_context_size(struct intel_engine_cs *ring) return ret; } -static int lrc_setup_hardware_status_page(struct intel_engine_cs *ring, +static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring, struct drm_i915_gem_object *default_ctx_obj) { struct drm_i915_private *dev_priv = ring->dev->dev_private; @@ -1716,15 +1716,11 @@ static int lrc_setup_hardware_status_page(struct intel_engine_cs *ring, ring->status_page.gfx_addr = i915_gem_obj_ggtt_offset(default_ctx_obj); ring->status_page.page_addr = kmap(sg_page(default_ctx_obj->pages->sgl)); - if (ring->status_page.page_addr == NULL) - return -ENOMEM; ring->status_page.obj = default_ctx_obj; I915_WRITE(RING_HWS_PGA(ring->mmio_base), (u32)ring->status_page.gfx_addr); POSTING_READ(RING_HWS_PGA(ring->mmio_base)); - - return 0; } /** @@ -1811,13 +1807,8 @@ int intel_lr_context_deferred_create(struct intel_context *ctx, ctx->engine[ring->id].ringbuf = ringbuf; ctx->engine[ring->id].state = ctx_obj; - if (ctx == ring->default_context) { - ret = lrc_setup_hardware_status_page(ring, ctx_obj); - if (ret) { - DRM_ERROR("Failed to setup hardware status page\n"); - goto error; - } - } + if (ctx == ring->default_context) + lrc_setup_hardware_status_page(ring, ctx_obj); if (ring->id == RCS && !ctx->rcs_initialized) { if (ring->init_context) { -- 2.1.1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] drm/i915: Drop return value from 2014-11-18 8:10 ` [PATCH] drm/i915: Drop return value from lrc_setup_hardware_status_page Daniel Vetter @ 2014-11-18 15:31 ` shuang.he 0 siblings, 0 replies; 8+ messages in thread From: shuang.he @ 2014-11-18 15:31 UTC (permalink / raw) To: shuang.he, intel-gfx, daniel.vetter Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com) -------------------------------------Summary------------------------------------- Platform: baseline_drm_intel_nightly_pass_rate->patch_applied_pass_rate BYT: pass/total=290/290->290/290 PNV: pass/total=362/362->362/362 ILK: pass/total=381/381->374/381 IVB: pass/total=522/559->522/559 SNB: pass/total=444/444->444/444 HSW: pass/total=595/595->594/595 BDW: pass/total=436/436->436/436 -------------------------------------Detailed------------------------------------- test_platform: test_suite, test_case, result_with_drm_intel_nightly(count, machine_id...)...->result_with_patch_applied(count, machine_id)... ILK: Intel_gpu_tools, igt_kms_flip_bcs-wf_vblank-vs-modeset-interruptible, PASS(4, M37M26) -> DMESG_WARN(1, M26)PASS(3, M26) ILK: Intel_gpu_tools, igt_kms_flip_flip-vs-rmfb-interruptible, PASS(4, M37M26) -> DMESG_WARN(2, M26)PASS(2, M26) ILK: Intel_gpu_tools, igt_kms_flip_plain-flip, PASS(4, M37M26) -> DMESG_WARN(1, M26)PASS(3, M26) ILK: Intel_gpu_tools, igt_kms_flip_rcs-wf_vblank-vs-dpms-interruptible, PASS(4, M37M26) -> DMESG_WARN(3, M26)PASS(1, M26) ILK: Intel_gpu_tools, igt_kms_flip_vblank-vs-hang-interruptible, PASS(4, M37M26) -> DMESG_WARN(3, M26)PASS(1, M26) ILK: Intel_gpu_tools, igt_kms_flip_wf_vblank-ts-check, PASS(4, M37M26) -> DMESG_WARN(1, M26)PASS(3, M26) ILK: Intel_gpu_tools, igt_kms_flip_wf_vblank-vs-dpms, DMESG_WARN(1, M26)PASS(3, M6M26) -> DMESG_WARN(2, M26)PASS(2, M26) HSW: Intel_gpu_tools, igt_gem_reset_stats_close-pending-render, PASS(3, M19M40) -> NO_RESULT(1, M40)PASS(3, M40) _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-11-18 15:31 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-11-17 15:48 [PATCH] drm/i915: Free resources correctly if we cannot map status page during ctx create Arun Siluvery 2014-11-17 15:54 ` Daniel, Thomas 2014-11-17 15:57 ` Siluvery, Arun 2014-11-17 19:04 ` Daniel Vetter 2014-11-18 7:44 ` Chris Wilson 2014-11-18 8:05 ` Daniel Vetter 2014-11-18 8:10 ` [PATCH] drm/i915: Drop return value from lrc_setup_hardware_status_page Daniel Vetter 2014-11-18 15:31 ` [PATCH] drm/i915: Drop return value from shuang.he
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox