[Intel-gfx] [PATCH v2] drm/i915/guc: Fix a memory leak where guc->execbuf_client is not freed

[Dave Gordon]

On 13/01/16 19:01, yu.dai at intel.com wrote:
> From: Alex Dai <yu.dai at intel.com>
>
> During driver unloading, the guc_client created for command submission
> needs to be released to avoid memory leak.
>
> The struct_mutex needs to be held before tearing down GuC.
>
> v1: Move i915_guc_submission_disable out of i915_guc_submission_fini and
>      take struct_mutex lock before release GuC client. (Dave Gordon)
> v2: Add the locking for failure case in guc_fw_fetch. (Dave Gordon)
>      Add i915_guc_submission_fini for failure case in intel_guc_ucode_load.
>
> Signed-off-by: Alex Dai <yu.dai at intel.com>

LGTM.

Reviewed-by: Dave Gordon <david.s.gordon at intel.com>

>
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index d20788f..3accd91 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -445,6 +445,7 @@ fail:
>
>   	direct_interrupts_to_host(dev_priv);
>   	i915_guc_submission_disable(dev);
> +	i915_guc_submission_fini(dev);
>
>   	return err;
>   }
> @@ -561,10 +562,12 @@ fail:
>   	DRM_ERROR("Failed to fetch GuC firmware from %s (error %d)\n",
>   		  guc_fw->guc_fw_path, err);
>
> +	mutex_lock(&dev->struct_mutex);
>   	obj = guc_fw->guc_fw_obj;
>   	if (obj)
>   		drm_gem_object_unreference(&obj->base);
>   	guc_fw->guc_fw_obj = NULL;
> +	mutex_unlock(&dev->struct_mutex);
>
>   	release_firmware(fw);		/* OK even if fw is NULL */
>   	guc_fw->guc_fw_fetch_status = GUC_FIRMWARE_FAIL;
> @@ -631,10 +634,11 @@ void intel_guc_ucode_fini(struct drm_device *dev)
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_guc_fw *guc_fw = &dev_priv->guc.guc_fw;
>
> +	mutex_lock(&dev->struct_mutex);
>   	direct_interrupts_to_host(dev_priv);
> +	i915_guc_submission_disable(dev);
>   	i915_guc_submission_fini(dev);
>
> -	mutex_lock(&dev->struct_mutex);
>   	if (guc_fw->guc_fw_obj)
>   		drm_gem_object_unreference(&guc_fw->guc_fw_obj->base);
>   	guc_fw->guc_fw_obj = NULL;
>