[1/1] drm/i915/guc: Add uc_fini_wq in gem_init unwind path
diff mbox

Message ID 1515571889-1237-1-git-send-email-sagar.a.kamble@intel.com
State New
Headers show

Commit Message

sagar.a.kamble@intel.com Jan. 10, 2018, 8:11 a.m. UTC
While moving code around for solving lockdep issue for GuC log relay,
spotted that uc_fini_wq is not being called in failure path in gem_init.
Missed in the below commit. Add it.

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Fixes: 3176ff49bc3e ("drm/i915/guc: Move GuC workqueue allocations outside of the mutex")
Cc: Michał Winiarski <michal.winiarski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Michał Winiarski Jan. 10, 2018, 9:44 a.m. UTC | #1
On Wed, Jan 10, 2018 at 01:41:29PM +0530, Sagar Arun Kamble wrote:
> While moving code around for solving lockdep issue for GuC log relay,
> spotted that uc_fini_wq is not being called in failure path in gem_init.
> Missed in the below commit. Add it.
> 
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Fixes: 3176ff49bc3e ("drm/i915/guc: Move GuC workqueue allocations outside of the mutex")
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 8bc3283..1135a77 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5283,6 +5283,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
>  	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>  	mutex_unlock(&dev_priv->drm.struct_mutex);
>  
> +	intel_uc_fini_wq(dev_priv);
> +

Ooppss.

Now that we're doing proper cleanup here, we also need to remove:

GEM_BUG_ON(!HAS_GUC(dev_priv));

from intel_uc_fini_wq - otherwise we'll hit it with enable_guc=1 on plaforms
without GuC.

With that:

Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>

-Michał

>  	if (ret != -EIO)
>  		i915_gem_cleanup_userptr(dev_priv);
>  
> -- 
> 1.9.1
>
sagar.a.kamble@intel.com Jan. 10, 2018, 10:23 a.m. UTC | #2
On 1/10/2018 3:14 PM, Michał Winiarski wrote:
> On Wed, Jan 10, 2018 at 01:41:29PM +0530, Sagar Arun Kamble wrote:
>> While moving code around for solving lockdep issue for GuC log relay,
>> spotted that uc_fini_wq is not being called in failure path in gem_init.
>> Missed in the below commit. Add it.
>>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Fixes: 3176ff49bc3e ("drm/i915/guc: Move GuC workqueue allocations outside of the mutex")
>> Cc: Michał Winiarski <michal.winiarski@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>   drivers/gpu/drm/i915/i915_gem.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index 8bc3283..1135a77 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -5283,6 +5283,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
>>   	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>>   	mutex_unlock(&dev_priv->drm.struct_mutex);
>>   
>> +	intel_uc_fini_wq(dev_priv);
>> +
> Ooppss.
>
> Now that we're doing proper cleanup here, we also need to remove:
>
> GEM_BUG_ON(!HAS_GUC(dev_priv));
>
> from intel_uc_fini_wq - otherwise we'll hit it with enable_guc=1 on plaforms
> without GuC.
Took some time for me to understand why we will not hit this on other 
uc_fini paths :)
Thanks for the review.
> With that:
>
> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
>
> -Michał
>
>>   	if (ret != -EIO)
>>   		i915_gem_cleanup_userptr(dev_priv);
>>   
>> -- 
>> 1.9.1
>>

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8bc3283..1135a77 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5283,6 +5283,8 @@  int i915_gem_init(struct drm_i915_private *dev_priv)
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 
+	intel_uc_fini_wq(dev_priv);
+
 	if (ret != -EIO)
 		i915_gem_cleanup_userptr(dev_priv);