On Thu, Apr 02, 2015 at 06:49:38PM +0530, Deepak S wrote: > > > On Monday 30 March 2015 09:13 PM, Daniel Vetter wrote: > >On Mon, Mar 30, 2015 at 08:03:58PM +0530, deepak.s@xxxxxxxxxxxxxxx wrote: > >>From: Deepak S <deepak.s@xxxxxxxxxxxxxxx> > >> > >>Cleanup idr table if any error happens after __create_hw_context() in > >>i915_gem_create_context() > >> > >>Signed-off-by: Deepak S <deepak.s@xxxxxxxxxxxxxxx> > >>--- > >> drivers/gpu/drm/i915/i915_gem_context.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >>diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > >>index f3e84c4..69bebe5 100644 > >>--- a/drivers/gpu/drm/i915/i915_gem_context.c > >>+++ b/drivers/gpu/drm/i915/i915_gem_context.c > >>@@ -287,6 +287,8 @@ err_unpin: > >> if (is_global_default_ctx && ctx->legacy_hw_ctx.rcs_state) > >> i915_gem_object_ggtt_unpin(ctx->legacy_hw_ctx.rcs_state); > >> err_destroy: > >>+ if (ctx->file_priv) > >>+ idr_remove(&ctx->file_priv->context_idr, ctx->user_handle); > >The common approach is to add a new err_idr: label at the op of the unwind > >code and make the call to idr_remove unconditional. > > > >Thanks, Daniel > > Thanks Daniel for review. > I do not think we can have a unconditional idr remove since for global ctx > i915_gem_create_context called with file_priv=NULL? Hm right, the entire control-flow in there is a bit funny. I think a much cleaner solution would be to drop the file_prive from create_context and add a new i915_gem_context_create_user which wraps create_context and the idr allocation. Doing the cleanup, conditionally, in a different function than where we do the allocation is a bit too brittle imo. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
