Re: [PATCH] drm/i915: Clean-up idr table if context create fails.

[Deepak S <deepak.s@linux.intel.com>]

On Tuesday 07 April 2015 02:02 PM, Chris Wilson wrote:
> On Tue, Apr 07, 2015 at 10:20:15AM +0200, Daniel Vetter wrote:
>> 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@linux.intel.com wrote:
>>>>> From: Deepak S <deepak.s@linux.intel.com>
>>>>>
>>>>> Cleanup idr table if any error happens after __create_hw_context() in
>>>>> i915_gem_create_context()
>>>>>
>>>>> Signed-off-by: Deepak S <deepak.s@linux.intel.com>
>>>>> ---
>>>>>   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.
> I suggested that it look like:
> http://cgit.freedesktop.org/~ickle/linux-2.6/tree/drivers/gpu/drm/i915/i915_gem_context.c#n179
> -Chris

Thanks Chris and Daniel, I will submit cleaned up patches


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx