drm/msm/gem: Separate object and vma unpin

Previously the BO_PINNED state in the submit was tracking two related
but different things: (1) that the buffer object was pinned, and (2)
that the vma (mapping within a set of pagetables) was pinned.  But with
fenced vma unpin (needed so that userspace couldn't race with retire
path for releasing a vma) these two were decoupled.  The fact that the
BO_PINNED flag was already cleared meant that we leaked the bo pin count
which should have been dropped when the submit was retired.

So split this state into BO_OBJ_PINNED and BO_VMA_PINNED, so they can be
dropped independently.

Fixes: 95d1deb02a9c ("drm/msm/gem: Add fenced vma unpin")
Signed-off-by: Rob Clark <robdclark@chromium.org>
Patchwork: https://patchwork.freedesktop.org/patch/487559/
Link: https://lore.kernel.org/r/20220527172341.2151005-1-robdclark@gmail.com

diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 52fe6428a3411677a6d2e4335942dbbf08cf9bf6..916e7f418fe1a7970b04cab310ebf80744b8d938 100644 (file)
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -439,14 +439,12 @@ int msm_gem_pin_vma_locked(struct drm_gem_object *obj, struct msm_gem_vma *vma)
        return ret;
 }
 
-void msm_gem_unpin_vma_locked(struct drm_gem_object *obj, struct msm_gem_vma *vma)
+void msm_gem_unpin_locked(struct drm_gem_object *obj)
 {
        struct msm_gem_object *msm_obj = to_msm_bo(obj);
 
        GEM_WARN_ON(!msm_gem_is_locked(obj));
 
-       msm_gem_unpin_vma(vma);
-
        msm_obj->pin_count--;
        GEM_WARN_ON(msm_obj->pin_count < 0);
 
@@ -586,7 +584,8 @@ void msm_gem_unpin_iova(struct drm_gem_object *obj,
        msm_gem_lock(obj);
        vma = lookup_vma(obj, aspace);
        if (!GEM_WARN_ON(!vma)) {
-               msm_gem_unpin_vma_locked(obj, vma);
+               msm_gem_unpin_vma(vma);
+               msm_gem_unpin_locked(obj);
        }
        msm_gem_unlock(obj);
 }

diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index c75d3b879a53bfbd3f3141db26ccfeb05d9be22c..6b7d5bb3b575fa3e77a6509328f47ed2fc2d5c24 100644 (file)
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -145,7 +145,7 @@ struct msm_gem_object {
 
 uint64_t msm_gem_mmap_offset(struct drm_gem_object *obj);
 int msm_gem_pin_vma_locked(struct drm_gem_object *obj, struct msm_gem_vma *vma);
-void msm_gem_unpin_vma_locked(struct drm_gem_object *obj, struct msm_gem_vma *vma);
+void msm_gem_unpin_locked(struct drm_gem_object *obj);
 struct msm_gem_vma *msm_gem_get_vma_locked(struct drm_gem_object *obj,
                                           struct msm_gem_address_space *aspace);
 int msm_gem_get_iova(struct drm_gem_object *obj,
@@ -377,10 +377,11 @@ struct msm_gem_submit {
        } *cmd;  /* array of size nr_cmds */
        struct {
 /* make sure these don't conflict w/ MSM_SUBMIT_BO_x */
-#define BO_VALID    0x8000   /* is current addr in cmdstream correct/valid? */
-#define BO_LOCKED   0x4000   /* obj lock is held */
-#define BO_ACTIVE   0x2000   /* active refcnt is held */
-#define BO_PINNED   0x1000   /* obj is pinned and on active list */
+#define BO_VALID       0x8000  /* is current addr in cmdstream correct/valid? */
+#define BO_LOCKED      0x4000  /* obj lock is held */
+#define BO_ACTIVE      0x2000  /* active refcnt is held */
+#define BO_OBJ_PINNED  0x1000  /* obj (pages) is pinned and on active list */
+#define BO_VMA_PINNED  0x0800  /* vma (virtual address) is pinned */
                uint32_t flags;
                union {
                        struct msm_gem_object *obj;

diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index 9cd8c8708990b2d5516816973ae92a964fab59f6..286124008445dc026b1583389b5b2f5f6ac04b5a 100644 (file)
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -232,8 +232,11 @@ static void submit_cleanup_bo(struct msm_gem_submit *submit, int i,
         */
        submit->bos[i].flags &= ~cleanup_flags;
 
-       if (flags & BO_PINNED)
-               msm_gem_unpin_vma_locked(obj, submit->bos[i].vma);
+       if (flags & BO_VMA_PINNED)
+               msm_gem_unpin_vma(submit->bos[i].vma);
+
+       if (flags & BO_OBJ_PINNED)
+               msm_gem_unpin_locked(obj);
 
        if (flags & BO_ACTIVE)
                msm_gem_active_put(obj);
@@ -244,7 +247,9 @@ static void submit_cleanup_bo(struct msm_gem_submit *submit, int i,
 
 static void submit_unlock_unpin_bo(struct msm_gem_submit *submit, int i)
 {
-       submit_cleanup_bo(submit, i, BO_PINNED | BO_ACTIVE | BO_LOCKED);
+       unsigned cleanup_flags = BO_VMA_PINNED | BO_OBJ_PINNED |
+                                BO_ACTIVE | BO_LOCKED;
+       submit_cleanup_bo(submit, i, cleanup_flags);
 
        if (!(submit->bos[i].flags & BO_VALID))
                submit->bos[i].iova = 0;
@@ -377,7 +382,7 @@ static int submit_pin_objects(struct msm_gem_submit *submit)
                if (ret)
                        break;
 
-               submit->bos[i].flags |= BO_PINNED;
+               submit->bos[i].flags |= BO_OBJ_PINNED | BO_VMA_PINNED;
                submit->bos[i].vma = vma;
 
                if (vma->iova == submit->bos[i].iova) {
@@ -511,7 +516,7 @@ static void submit_cleanup(struct msm_gem_submit *submit, bool error)
        unsigned i;
 
        if (error)
-               cleanup_flags |= BO_PINNED | BO_ACTIVE;
+               cleanup_flags |= BO_VMA_PINNED | BO_OBJ_PINNED | BO_ACTIVE;
 
        for (i = 0; i < submit->nr_bos; i++) {
                struct msm_gem_object *msm_obj = submit->bos[i].obj;
@@ -529,7 +534,8 @@ void msm_submit_retire(struct msm_gem_submit *submit)
                struct drm_gem_object *obj = &submit->bos[i].obj->base;
 
                msm_gem_lock(obj);
-               submit_cleanup_bo(submit, i, BO_PINNED | BO_ACTIVE);
+               /* Note, VMA already fence-unpinned before submit: */
+               submit_cleanup_bo(submit, i, BO_OBJ_PINNED | BO_ACTIVE);
                msm_gem_unlock(obj);
                drm_gem_object_put(obj);
        }

diff --git a/drivers/gpu/drm/msm/msm_ringbuffer.c b/drivers/gpu/drm/msm/msm_ringbuffer.c
index 43066320ff8c9feeec5b092e31cb151f99f6ac13..56eecb4a72dc6d3e270316e88b20fcd37932117e 100644 (file)
--- a/drivers/gpu/drm/msm/msm_ringbuffer.c
+++ b/drivers/gpu/drm/msm/msm_ringbuffer.c
@@ -25,7 +25,7 @@ static struct dma_fence *msm_job_run(struct drm_sched_job *job)
 
                msm_gem_lock(obj);
                msm_gem_unpin_vma_fenced(submit->bos[i].vma, fctx);
-               submit->bos[i].flags &= ~BO_PINNED;
+               submit->bos[i].flags &= ~BO_VMA_PINNED;
                msm_gem_unlock(obj);
        }