* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.