Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <42e52572-726b-d94d-6523-7b42dbeecff1@kali.org>
Date:   Sat, 11 Jun 2022 13:16:32 -0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.10.0
Subject: Re: [PATCH] drm/msm/gem: Drop early returns in close/purge vma
Content-Language: en-US
To:     Rob Clark <robdclark@gmail.com>, dri-devel@lists.freedesktop.org
Cc:     freedreno@lists.freedesktop.org, linux-arm-msm@vger.kernel.org,
        Rob Clark <robdclark@chromium.org>,
        Abhinav Kumar <quic_abhinavk@quicinc.com>,
        Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
        Sean Paul <sean@poorly.run>, David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        open list <linux-kernel@vger.kernel.org>
References: <20220610172055.2337977-1-robdclark@gmail.com>
From:   Steev Klimaszewski <steev@kali.org>
In-Reply-To: <20220610172055.2337977-1-robdclark@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Rob,

On 6/10/22 12:20 PM, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
>
> Keep the warn, but drop the early return.  If we do manage to hit this
> sort of issue, skipping the cleanup just makes things worse (dangling
> drm_mm_nodes when the msm_gem_vma is freed, etc).  Whereas the worst
> that happens if we tear down a mapping the GPU is accessing is that we
> get GPU iova faults, but otherwise the world keeps spinning.
>
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>   drivers/gpu/drm/msm/msm_gem_vma.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c b/drivers/gpu/drm/msm/msm_gem_vma.c
> index 3c1dc9241831..c471aebcdbab 100644
> --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> @@ -62,8 +62,7 @@ void msm_gem_purge_vma(struct msm_gem_address_space *aspace,
>   	unsigned size = vma->node.size;
>   
>   	/* Print a message if we try to purge a vma in use */
> -	if (GEM_WARN_ON(msm_gem_vma_inuse(vma)))
> -		return;
> +	GEM_WARN_ON(msm_gem_vma_inuse(vma));
>   
>   	/* Don't do anything if the memory isn't mapped */
>   	if (!vma->mapped)
> @@ -128,8 +127,7 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace,
>   void msm_gem_close_vma(struct msm_gem_address_space *aspace,
>   		struct msm_gem_vma *vma)
>   {
> -	if (GEM_WARN_ON(msm_gem_vma_inuse(vma) || vma->mapped))
> -		return;
> +	GEM_WARN_ON(msm_gem_vma_inuse(vma) || vma->mapped);
>   
>   	spin_lock(&aspace->lock);
>   	if (vma->iova)

I've seen the splat on the Lenovo Yoga C630 here, and have tested this 
patch, and as described, the splat still happens, but the system is 
still able to be used.

Tested-by: Steev Klimaszewski <steev@kali.org>