diff mbox series

drm/i915/gt: Do release kernel context if breadcrumb measure fails

Message ID 20210507144251.376538-1-janusz.krzysztofik@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/gt: Do release kernel context if breadcrumb measure fails | expand

Commit Message

Janusz Krzysztofik May 7, 2021, 2:42 p.m. UTC
Commit fb5970da1b42 ("drm/i915/gt: Use the kernel_context to measure the
breadcrumb size") reordered some operations inside engine_init_common()
and added an error unwind path to that function.  In that path, a
reference to a kernel context candidate supposed to be released on error
was put, but the context, pinned when created, was not unpinned first.
Fix it by replacing intel_context_put() with destroy_pinned_context()
introduced later by commit b436a5f8b6c8 ("drm/i915/gt: Track all timelines
created using the HWSP").

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Tvrtko Ursulin May 10, 2021, 9:14 a.m. UTC | #1
On 07/05/2021 15:42, Janusz Krzysztofik wrote:
> Commit fb5970da1b42 ("drm/i915/gt: Use the kernel_context to measure the
> breadcrumb size") reordered some operations inside engine_init_common()
> and added an error unwind path to that function.  In that path, a
> reference to a kernel context candidate supposed to be released on error
> was put, but the context, pinned when created, was not unpinned first.
> Fix it by replacing intel_context_put() with destroy_pinned_context()
> introduced later by commit b436a5f8b6c8 ("drm/i915/gt: Track all timelines
> created using the HWSP").
> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 6dbdbde00f14..eba2da9679a5 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -898,7 +898,7 @@ static int engine_init_common(struct intel_engine_cs *engine)
>   	return 0;
>   
>   err_context:
> -	intel_context_put(ce);
> +	destroy_pinned_context(ce);
>   	return ret;
>   }
>   
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Found by some test or code inspection?

Regards,

Tvrtko
Janusz Krzysztofik May 10, 2021, 9:15 a.m. UTC | #2
Hi Tvrtko,

On poniedziaƂek, 10 maja 2021 11:14:46 CEST Tvrtko Ursulin wrote:
> 
> On 07/05/2021 15:42, Janusz Krzysztofik wrote:
> > Commit fb5970da1b42 ("drm/i915/gt: Use the kernel_context to measure the
> > breadcrumb size") reordered some operations inside engine_init_common()
> > and added an error unwind path to that function.  In that path, a
> > reference to a kernel context candidate supposed to be released on error
> > was put, but the context, pinned when created, was not unpinned first.
> > Fix it by replacing intel_context_put() with destroy_pinned_context()
> > introduced later by commit b436a5f8b6c8 ("drm/i915/gt: Track all timelines
> > created using the HWSP").
> > 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >   drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > index 6dbdbde00f14..eba2da9679a5 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > @@ -898,7 +898,7 @@ static int engine_init_common(struct intel_engine_cs *engine)
> >   	return 0;
> >   
> >   err_context:
> > -	intel_context_put(ce);
> > +	destroy_pinned_context(ce);
> >   	return ret;
> >   }
> >   
> > 
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Found by some test or code inspection?

Code inspection.

Thanks,
Janusz

> 
> Regards,
> 
> Tvrtko
>
Tvrtko Ursulin May 10, 2021, 9:20 a.m. UTC | #3
On 10/05/2021 10:15, Janusz Krzysztofik wrote:
> Hi Tvrtko,
> 
> On poniedziaƂek, 10 maja 2021 11:14:46 CEST Tvrtko Ursulin wrote:
>>
>> On 07/05/2021 15:42, Janusz Krzysztofik wrote:
>>> Commit fb5970da1b42 ("drm/i915/gt: Use the kernel_context to measure the
>>> breadcrumb size") reordered some operations inside engine_init_common()
>>> and added an error unwind path to that function.  In that path, a
>>> reference to a kernel context candidate supposed to be released on error
>>> was put, but the context, pinned when created, was not unpinned first.
>>> Fix it by replacing intel_context_put() with destroy_pinned_context()
>>> introduced later by commit b436a5f8b6c8 ("drm/i915/gt: Track all timelines
>>> created using the HWSP").
>>>
>>> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> ---
>>>    drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> index 6dbdbde00f14..eba2da9679a5 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> @@ -898,7 +898,7 @@ static int engine_init_common(struct intel_engine_cs *engine)
>>>    	return 0;
>>>    
>>>    err_context:
>>> -	intel_context_put(ce);
>>> +	destroy_pinned_context(ce);
>>>    	return ret;
>>>    }
>>>    
>>>
>>
>> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Found by some test or code inspection?
> 
> Code inspection.

Cool. It was all green on the CI front so I am pushing it as we speak. 
Thanks!

Regards,

Tvrtko
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 6dbdbde00f14..eba2da9679a5 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -898,7 +898,7 @@  static int engine_init_common(struct intel_engine_cs *engine)
 	return 0;
 
 err_context:
-	intel_context_put(ce);
+	destroy_pinned_context(ce);
 	return ret;
 }