Commit 311e03c2 authored by Rob Clark's avatar Rob Clark
Browse files

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: 95d1deb0 ("drm/msm/gem: Add fenced vma unpin")
Signed-off-by: default avatarRob Clark <robdclark@chromium.org>
Patchwork: https://patchwork.freedesktop.org/patch/487559/
Link: https://lore.kernel.org/r/20220527172341.2151005-1-robdclark@gmail.com
parent 62b5e322

drivers/gpu/drm/msm/msm_gem.c

+3 −4
Original line number Diff line number Diff line
@@ -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);

}

drivers/gpu/drm/msm/msm_gem.h

+6 −5
Original line number Diff line number Diff line
@@ -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,
@@ -380,7 +380,8 @@ struct msm_gem_submit { 
#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_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;

drivers/gpu/drm/msm/msm_gem_submit.c

+12 −6
Original line number Diff line number Diff line
@@ -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);

	}

drivers/gpu/drm/msm/msm_ringbuffer.c

+1 −1
Original line number Diff line number Diff line
@@ -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);

	}