[PATCH] drm/i915/gvt: Fix unnecessary schedule timer when no vGPU exits

[Colin Xu]

Yes vgpu_lock is acquired after gvt->lock and vgpu_idr is in gvt domain 
so removing

it doesn't need vgpu_lock but need gvt->lock. And remove idr first can 
guarantee

that when vgpu destroy is on-going, other vgpu acquiring can exit asap 
so that no

race-condition with the cleanup tasks.

Thanks a lot for the fix!

Acked-by: Colin Xu <colin.xu at intel.com>

On 2020-02-29 13:54, Zhenyu Wang wrote:
>  From commit f25a49ab8ab9 ("drm/i915/gvt: Use vgpu_lock to protect per
> vgpu access") the vgpu idr destroy is moved later than vgpu resource
> destroy, then it would fail to stop timer for schedule policy clean
> which to check vgpu idr for any left vGPU. So this trys to destroy
> vgpu idr earlier.
>
> Cc: Colin Xu <colin.xu at intel.com>
> Fixes: f25a49ab8ab9 ("drm/i915/gvt: Use vgpu_lock to protect per vgpu access")
> Signed-off-by: Zhenyu Wang <zhenyuw at linux.intel.com>
> ---
>   drivers/gpu/drm/i915/gvt/vgpu.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
> index 76fa0493228f..9213b64450c3 100644
> --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> @@ -274,10 +274,17 @@ void intel_gvt_destroy_vgpu(struct intel_vgpu *vgpu)
>   	struct drm_i915_private *i915 = vgpu->gvt->dev_priv;
>   	struct intel_gvt *gvt = vgpu->gvt;
>   
> -	mutex_lock(&vgpu->vgpu_lock);
> -
>   	drm_WARN(&i915->drm, vgpu->active, "vGPU is still active!\n");
>   
> +	/*
> +	 * remove idr first so later clean can judge if need to stop
> +	 * service if no active vgpu.
> +	 */
> +	mutex_lock(&gvt->lock);
> +	idr_remove(&gvt->vgpu_idr, vgpu->id);
> +	mutex_unlock(&gvt->lock);
> +
> +	mutex_lock(&vgpu->vgpu_lock);
>   	intel_gvt_debugfs_remove_vgpu(vgpu);
>   	intel_vgpu_clean_sched_policy(vgpu);
>   	intel_vgpu_clean_submission(vgpu);
> @@ -292,7 +299,6 @@ void intel_gvt_destroy_vgpu(struct intel_vgpu *vgpu)
>   	mutex_unlock(&vgpu->vgpu_lock);
>   
>   	mutex_lock(&gvt->lock);
> -	idr_remove(&gvt->vgpu_idr, vgpu->id);
>   	if (idr_is_empty(&gvt->vgpu_idr))
>   		intel_gvt_clean_irq(gvt);
>   	intel_gvt_update_vgpu_types(gvt);

-- 
Best Regards,
Colin Xu