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;
MIME-Version: 1.0
References: <20220610172055.2337977-1-robdclark@gmail.com> <42e52572-726b-d94d-6523-7b42dbeecff1@kali.org>
In-Reply-To: <42e52572-726b-d94d-6523-7b42dbeecff1@kali.org>
From:   Rob Clark <robdclark@gmail.com>
Date:   Wed, 15 Jun 2022 07:59:44 -0700
Message-ID: <CAF6AEGuTw3fMaNbvkt9z3EWqjND2R3nrRu+PP=GVQjx+TW0N9w@mail.gmail.com>
Subject: Re: [PATCH] drm/msm/gem: Drop early returns in close/purge vma
To:     Steev Klimaszewski <steev@kali.org>
Cc:     dri-devel <dri-devel@lists.freedesktop.org>,
        freedreno <freedreno@lists.freedesktop.org>,
        linux-arm-msm <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>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, Jun 11, 2022 at 11:16 AM Steev Klimaszewski <steev@kali.org> wrote:
>
> 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.
> >

forgot this initially:

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

> > 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>
>