Skip to content

Commit

Permalink
dma-buf/drivers: make reserving a shared slot mandatory v4
Browse files Browse the repository at this point in the history
Audit all the users of dma_resv_add_excl_fence() and make sure they
reserve a shared slot also when only trying to add an exclusive fence.

This is the next step towards handling the exclusive fence like a
shared one.

v2: fix missed case in amdgpu
v3: and two more radeon, rename function
v4: add one more case to TTM, fix i915 after rebase

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20220406075132.3263-2-christian.koenig@amd.com
  • Loading branch information
Christian König committed Apr 6, 2022
1 parent 20b734c commit c8d4c18
Show file tree
Hide file tree
Showing 30 changed files with 176 additions and 114 deletions.
10 changes: 5 additions & 5 deletions drivers/dma-buf/dma-resv.c
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ static inline struct dma_resv_list *dma_resv_shared_list(struct dma_resv *obj)
}

/**
* dma_resv_reserve_shared - Reserve space to add shared fences to
* dma_resv_reserve_fences - Reserve space to add shared fences to
* a dma_resv.
* @obj: reservation object
* @num_fences: number of fences we want to add
Expand All @@ -167,7 +167,7 @@ static inline struct dma_resv_list *dma_resv_shared_list(struct dma_resv *obj)
* RETURNS
* Zero for success, or -errno
*/
int dma_resv_reserve_shared(struct dma_resv *obj, unsigned int num_fences)
int dma_resv_reserve_fences(struct dma_resv *obj, unsigned int num_fences)
{
struct dma_resv_list *old, *new;
unsigned int i, j, k, max;
Expand Down Expand Up @@ -230,15 +230,15 @@ int dma_resv_reserve_shared(struct dma_resv *obj, unsigned int num_fences)

return 0;
}
EXPORT_SYMBOL(dma_resv_reserve_shared);
EXPORT_SYMBOL(dma_resv_reserve_fences);

#ifdef CONFIG_DEBUG_MUTEXES
/**
* dma_resv_reset_shared_max - reset shared fences for debugging
* @obj: the dma_resv object to reset
*
* Reset the number of pre-reserved shared slots to test that drivers do
* correct slot allocation using dma_resv_reserve_shared(). See also
* correct slot allocation using dma_resv_reserve_fences(). See also
* &dma_resv_list.shared_max.
*/
void dma_resv_reset_shared_max(struct dma_resv *obj)
Expand All @@ -260,7 +260,7 @@ EXPORT_SYMBOL(dma_resv_reset_shared_max);
* @fence: the shared fence to add
*
* Add a fence to a shared slot, @obj must be locked with dma_resv_lock(), and
* dma_resv_reserve_shared() has been called.
* dma_resv_reserve_fences() has been called.
*
* See also &dma_resv.fence for a discussion of the semantics.
*/
Expand Down
64 changes: 30 additions & 34 deletions drivers/dma-buf/st-dma-resv.c
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,16 @@ static int test_signaling(void *arg, bool shared)
goto err_free;
}

if (shared) {
r = dma_resv_reserve_shared(&resv, 1);
if (r) {
pr_err("Resv shared slot allocation failed\n");
goto err_unlock;
}
r = dma_resv_reserve_fences(&resv, 1);
if (r) {
pr_err("Resv shared slot allocation failed\n");
goto err_unlock;
}

if (shared)
dma_resv_add_shared_fence(&resv, f);
} else {
else
dma_resv_add_excl_fence(&resv, f);
}

if (dma_resv_test_signaled(&resv, shared)) {
pr_err("Resv unexpectedly signaled\n");
Expand Down Expand Up @@ -134,17 +133,16 @@ static int test_for_each(void *arg, bool shared)
goto err_free;
}

if (shared) {
r = dma_resv_reserve_shared(&resv, 1);
if (r) {
pr_err("Resv shared slot allocation failed\n");
goto err_unlock;
}
r = dma_resv_reserve_fences(&resv, 1);
if (r) {
pr_err("Resv shared slot allocation failed\n");
goto err_unlock;
}

if (shared)
dma_resv_add_shared_fence(&resv, f);
} else {
else
dma_resv_add_excl_fence(&resv, f);
}

r = -ENOENT;
dma_resv_for_each_fence(&cursor, &resv, shared, fence) {
Expand Down Expand Up @@ -206,18 +204,17 @@ static int test_for_each_unlocked(void *arg, bool shared)
goto err_free;
}

if (shared) {
r = dma_resv_reserve_shared(&resv, 1);
if (r) {
pr_err("Resv shared slot allocation failed\n");
dma_resv_unlock(&resv);
goto err_free;
}
r = dma_resv_reserve_fences(&resv, 1);
if (r) {
pr_err("Resv shared slot allocation failed\n");
dma_resv_unlock(&resv);
goto err_free;
}

if (shared)
dma_resv_add_shared_fence(&resv, f);
} else {
else
dma_resv_add_excl_fence(&resv, f);
}
dma_resv_unlock(&resv);

r = -ENOENT;
Expand Down Expand Up @@ -290,18 +287,17 @@ static int test_get_fences(void *arg, bool shared)
goto err_resv;
}

if (shared) {
r = dma_resv_reserve_shared(&resv, 1);
if (r) {
pr_err("Resv shared slot allocation failed\n");
dma_resv_unlock(&resv);
goto err_resv;
}
r = dma_resv_reserve_fences(&resv, 1);
if (r) {
pr_err("Resv shared slot allocation failed\n");
dma_resv_unlock(&resv);
goto err_resv;
}

if (shared)
dma_resv_add_shared_fence(&resv, f);
} else {
else
dma_resv_add_excl_fence(&resv, f);
}
dma_resv_unlock(&resv);

r = dma_resv_get_fences(&resv, shared, &i, &fences);
Expand Down
4 changes: 2 additions & 2 deletions drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
Original file line number Diff line number Diff line change
Expand Up @@ -1233,7 +1233,7 @@ static int init_kfd_vm(struct amdgpu_vm *vm, void **process_info,
AMDGPU_FENCE_OWNER_KFD, false);
if (ret)
goto wait_pd_fail;
ret = dma_resv_reserve_shared(vm->root.bo->tbo.base.resv, 1);
ret = dma_resv_reserve_fences(vm->root.bo->tbo.base.resv, 1);
if (ret)
goto reserve_shared_fail;
amdgpu_bo_fence(vm->root.bo,
Expand Down Expand Up @@ -2571,7 +2571,7 @@ int amdgpu_amdkfd_add_gws_to_process(void *info, void *gws, struct kgd_mem **mem
* Add process eviction fence to bo so they can
* evict each other.
*/
ret = dma_resv_reserve_shared(gws_bo->tbo.base.resv, 1);
ret = dma_resv_reserve_fences(gws_bo->tbo.base.resv, 1);
if (ret)
goto reserve_shared_fail;
amdgpu_bo_fence(gws_bo, &process_info->eviction_fence->base, true);
Expand Down
8 changes: 8 additions & 0 deletions drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
Original file line number Diff line number Diff line change
Expand Up @@ -1388,6 +1388,14 @@ void amdgpu_bo_fence(struct amdgpu_bo *bo, struct dma_fence *fence,
bool shared)
{
struct dma_resv *resv = bo->tbo.base.resv;
int r;

r = dma_resv_reserve_fences(resv, 1);
if (r) {
/* As last resort on OOM we block for the fence */
dma_fence_wait(fence, false);
return;
}

if (shared)
dma_resv_add_shared_fence(resv, fence);
Expand Down
4 changes: 2 additions & 2 deletions drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
Original file line number Diff line number Diff line change
Expand Up @@ -2926,7 +2926,7 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm)
if (r)
goto error_free_root;

r = dma_resv_reserve_shared(root_bo->tbo.base.resv, 1);
r = dma_resv_reserve_fences(root_bo->tbo.base.resv, 1);
if (r)
goto error_unreserve;

Expand Down Expand Up @@ -3369,7 +3369,7 @@ bool amdgpu_vm_handle_fault(struct amdgpu_device *adev, u32 pasid,
value = 0;
}

r = dma_resv_reserve_shared(root->tbo.base.resv, 1);
r = dma_resv_reserve_fences(root->tbo.base.resv, 1);
if (r) {
pr_debug("failed %d to reserve fence slot\n", r);
goto error_unlock;
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/amd/amdkfd/kfd_svm.c
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ svm_range_vram_node_new(struct amdgpu_device *adev, struct svm_range *prange,
goto reserve_bo_failed;
}

r = dma_resv_reserve_shared(bo->tbo.base.resv, 1);
r = dma_resv_reserve_fences(bo->tbo.base.resv, 1);
if (r) {
pr_debug("failed %d to reserve bo\n", r);
amdgpu_bo_unreserve(bo);
Expand Down
8 changes: 3 additions & 5 deletions drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,9 @@ static int submit_fence_sync(struct etnaviv_gem_submit *submit)
struct etnaviv_gem_submit_bo *bo = &submit->bos[i];
struct dma_resv *robj = bo->obj->base.resv;

if (!(bo->flags & ETNA_SUBMIT_BO_WRITE)) {
ret = dma_resv_reserve_shared(robj, 1);
if (ret)
return ret;
}
ret = dma_resv_reserve_fences(robj, 1);
if (ret)
return ret;

if (submit->flags & ETNA_SUBMIT_NO_IMPLICIT)
continue;
Expand Down
3 changes: 2 additions & 1 deletion drivers/gpu/drm/i915/gem/i915_gem_clflush.c
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ bool i915_gem_clflush_object(struct drm_i915_gem_object *obj,
trace_i915_gem_object_clflush(obj);

clflush = NULL;
if (!(flags & I915_CLFLUSH_SYNC))
if (!(flags & I915_CLFLUSH_SYNC) &&
dma_resv_reserve_fences(obj->base.resv, 1) == 0)
clflush = clflush_work_create(obj);
if (clflush) {
i915_sw_fence_await_reservation(&clflush->base.chain,
Expand Down
10 changes: 4 additions & 6 deletions drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -998,11 +998,9 @@ static int eb_validate_vmas(struct i915_execbuffer *eb)
}
}

if (!(ev->flags & EXEC_OBJECT_WRITE)) {
err = dma_resv_reserve_shared(vma->obj->base.resv, 1);
if (err)
return err;
}
err = dma_resv_reserve_fences(vma->obj->base.resv, 1);
if (err)
return err;

GEM_BUG_ON(drm_mm_node_allocated(&vma->node) &&
eb_vma_misplaced(&eb->exec[i], vma, ev->flags));
Expand Down Expand Up @@ -2303,7 +2301,7 @@ static int eb_parse(struct i915_execbuffer *eb)
if (IS_ERR(batch))
return PTR_ERR(batch);

err = dma_resv_reserve_shared(shadow->obj->base.resv, 1);
err = dma_resv_reserve_fences(shadow->obj->base.resv, 1);
if (err)
return err;

Expand Down
6 changes: 5 additions & 1 deletion drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,11 @@ int i915_gem_obj_copy_ttm(struct drm_i915_gem_object *dst,
assert_object_held(src);
i915_deps_init(&deps, GFP_KERNEL | __GFP_NORETRY | __GFP_NOWARN);

ret = dma_resv_reserve_shared(src_bo->base.resv, 1);
ret = dma_resv_reserve_fences(src_bo->base.resv, 1);
if (ret)
return ret;

ret = dma_resv_reserve_fences(dst_bo->base.resv, 1);
if (ret)
return ret;

Expand Down
5 changes: 4 additions & 1 deletion drivers/gpu/drm/i915/gem/selftests/i915_gem_migrate.c
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,10 @@ static int __igt_lmem_pages_migrate(struct intel_gt *gt,
i915_gem_object_is_lmem(obj),
0xdeadbeaf, &rq);
if (rq) {
dma_resv_add_excl_fence(obj->base.resv, &rq->fence);
err = dma_resv_reserve_fences(obj->base.resv, 1);
if (!err)
dma_resv_add_excl_fence(obj->base.resv,
&rq->fence);
i915_gem_object_set_moving_fence(obj, &rq->fence);
i915_request_put(rq);
}
Expand Down
10 changes: 8 additions & 2 deletions drivers/gpu/drm/i915/i915_vma.c
Original file line number Diff line number Diff line change
Expand Up @@ -1819,14 +1819,20 @@ int _i915_vma_move_to_active(struct i915_vma *vma,
intel_frontbuffer_put(front);
}

if (!(flags & __EXEC_OBJECT_NO_RESERVE)) {
err = dma_resv_reserve_fences(vma->obj->base.resv, 1);
if (unlikely(err))
return err;
}

if (fence) {
dma_resv_add_excl_fence(vma->obj->base.resv, fence);
obj->write_domain = I915_GEM_DOMAIN_RENDER;
obj->read_domains = 0;
}
} else {
if (!(flags & __EXEC_OBJECT_NO_RESERVE)) {
err = dma_resv_reserve_shared(vma->obj->base.resv, 1);
err = dma_resv_reserve_fences(vma->obj->base.resv, 1);
if (unlikely(err))
return err;
}
Expand Down Expand Up @@ -2044,7 +2050,7 @@ int i915_vma_unbind_async(struct i915_vma *vma, bool trylock_vm)
if (!obj->mm.rsgt)
return -EBUSY;

err = dma_resv_reserve_shared(obj->base.resv, 1);
err = dma_resv_reserve_fences(obj->base.resv, 1);
if (err)
return -EBUSY;

Expand Down
7 changes: 7 additions & 0 deletions drivers/gpu/drm/i915/selftests/intel_memory_region.c
Original file line number Diff line number Diff line change
Expand Up @@ -1043,6 +1043,13 @@ static int igt_lmem_write_cpu(void *arg)
}

i915_gem_object_lock(obj, NULL);

err = dma_resv_reserve_fences(obj->base.resv, 1);
if (err) {
i915_gem_object_unlock(obj);
goto out_put;
}

/* Put the pages into a known state -- from the gpu for added fun */
intel_engine_pm_get(engine);
err = intel_context_migrate_clear(engine->gt->migrate.context, NULL,
Expand Down
10 changes: 4 additions & 6 deletions drivers/gpu/drm/lima/lima_gem.c
Original file line number Diff line number Diff line change
Expand Up @@ -257,13 +257,11 @@ int lima_gem_get_info(struct drm_file *file, u32 handle, u32 *va, u64 *offset)
static int lima_gem_sync_bo(struct lima_sched_task *task, struct lima_bo *bo,
bool write, bool explicit)
{
int err = 0;
int err;

if (!write) {
err = dma_resv_reserve_shared(lima_bo_resv(bo), 1);
if (err)
return err;
}
err = dma_resv_reserve_fences(lima_bo_resv(bo), 1);
if (err)
return err;

/* explicit sync use user passed dep fence */
if (explicit)
Expand Down
18 changes: 8 additions & 10 deletions drivers/gpu/drm/msm/msm_gem_submit.c
Original file line number Diff line number Diff line change
Expand Up @@ -320,16 +320,14 @@ static int submit_fence_sync(struct msm_gem_submit *submit, bool no_implicit)
struct drm_gem_object *obj = &submit->bos[i].obj->base;
bool write = submit->bos[i].flags & MSM_SUBMIT_BO_WRITE;

if (!write) {
/* NOTE: _reserve_shared() must happen before
* _add_shared_fence(), which makes this a slightly
* strange place to call it. OTOH this is a
* convenient can-fail point to hook it in.
*/
ret = dma_resv_reserve_shared(obj->resv, 1);
if (ret)
return ret;
}
/* NOTE: _reserve_shared() must happen before
* _add_shared_fence(), which makes this a slightly
* strange place to call it. OTOH this is a
* convenient can-fail point to hook it in.
*/
ret = dma_resv_reserve_fences(obj->resv, 1);
if (ret)
return ret;

/* exclusive fences must be ordered */
if (no_implicit && !write)
Expand Down
8 changes: 3 additions & 5 deletions drivers/gpu/drm/nouveau/nouveau_fence.c
Original file line number Diff line number Diff line change
Expand Up @@ -346,11 +346,9 @@ nouveau_fence_sync(struct nouveau_bo *nvbo, struct nouveau_channel *chan,
struct dma_resv *resv = nvbo->bo.base.resv;
int i, ret;

if (!exclusive) {
ret = dma_resv_reserve_shared(resv, 1);
if (ret)
return ret;
}
ret = dma_resv_reserve_fences(resv, 1);
if (ret)
return ret;

/* Waiting for the exclusive fence first causes performance regressions
* under some circumstances. So manually wait for the shared ones first.
Expand Down
4 changes: 4 additions & 0 deletions drivers/gpu/drm/panfrost/panfrost_job.c
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,10 @@ static int panfrost_acquire_object_fences(struct drm_gem_object **bos,
int i, ret;

for (i = 0; i < bo_count; i++) {
ret = dma_resv_reserve_fences(bos[i]->resv, 1);
if (ret)
return ret;

/* panfrost always uses write mode in its current uapi */
ret = drm_sched_job_add_implicit_dependencies(job, bos[i],
true);
Expand Down
Loading

0 comments on commit c8d4c18

Please sign in to comment.