I applied your change to my local KFD staging branch and it through a presubmission build/test (sorry, only accessible from inside AMD): http://git.amd.com:8080/#/c/167906/ It passed, but checkpatch pointed out one issue: http://git.amd.com:8080/#/c/167906/1/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c at 247 (see inline) With that fixed, this change is Reviewed-by: Felix Kuehling <Felix.Kuehling at amd.com> Regards,  Felix On 2018-08-15 03:46 AM, Christian König wrote: > Fix quite a number of bugs here. Unfortunately only compile tested. > > v2: fix copy&paste error > > Signed-off-by: Christian König <christian.koenig at amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 103 ++++++++++------------- > 1 file changed, 46 insertions(+), 57 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c > index fa38a960ce00..887663ede781 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c > @@ -206,11 +206,9 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct amdgpu_bo *bo, > struct amdgpu_amdkfd_fence ***ef_list, > unsigned int *ef_count) > { > - struct reservation_object_list *fobj; > - struct reservation_object *resv; > - unsigned int i = 0, j = 0, k = 0, shared_count; > - unsigned int count = 0; > - struct amdgpu_amdkfd_fence **fence_list; > + struct reservation_object *resv = bo->tbo.resv; > + struct reservation_object_list *old, *new; > + unsigned int i, j, k; > > if (!ef && !ef_list) > return -EINVAL; > @@ -220,76 +218,67 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct amdgpu_bo *bo, > *ef_count = 0; > } > > - resv = bo->tbo.resv; > - fobj = reservation_object_get_list(resv); > - > - if (!fobj) > + old = reservation_object_get_list(resv); > + if (!old) > return 0; > > - preempt_disable(); > - write_seqcount_begin(&resv->seq); > + new = kmalloc(offsetof(typeof(*new), shared[old->shared_max]), > + GFP_KERNEL); > + if (!new) > + return -ENOMEM; > > - /* Go through all the shared fences in the resevation object. If > - * ef is specified and it exists in the list, remove it and reduce the > - * count. If ef is not specified, then get the count of eviction fences > - * present. > + /* Go through all the shared fences in the resevation object and sort > + * the interesting ones to the end of the list. > */ > - shared_count = fobj->shared_count; > - for (i = 0; i < shared_count; ++i) { > + for (i = 0, j = old->shared_count, k = 0; i < old->shared_count; ++i) { > struct dma_fence *f; > > - f = rcu_dereference_protected(fobj->shared[i], > + f = rcu_dereference_protected(old->shared[i], > reservation_object_held(resv)); > > - if (ef) { > - if (f->context == ef->base.context) { > - dma_fence_put(f); > - fobj->shared_count--; > - } else { > - RCU_INIT_POINTER(fobj->shared[j++], f); > - } > - } else if (to_amdgpu_amdkfd_fence(f)) > - count++; > + if ((ef && f->context == ef->base.context) || > + (!ef && to_amdgpu_amdkfd_fence(f))) > + RCU_INIT_POINTER(new->shared[--j], f); > + else > + RCU_INIT_POINTER(new->shared[k++], f); > } > - write_seqcount_end(&resv->seq); > - preempt_enable(); > - > - if (ef || !count) > - return 0; > - > - /* Alloc memory for count number of eviction fence pointers. Fill the > - * ef_list array and ef_count > - */ > - fence_list = kcalloc(count, sizeof(struct amdgpu_amdkfd_fence *), > - GFP_KERNEL); > - if (!fence_list) > - return -ENOMEM; > + new->shared_max = old->shared_max; > + new->shared_count = k; > > - preempt_disable(); > - write_seqcount_begin(&resv->seq); > + if (!ef) { > + unsigned int count = old->shared_count - j; > > - j = 0; > - for (i = 0; i < shared_count; ++i) { > - struct dma_fence *f; > - struct amdgpu_amdkfd_fence *efence; > - > - f = rcu_dereference_protected(fobj->shared[i], > - reservation_object_held(resv)); > + /* Alloc memory for count number of eviction fence pointers. Fill the checkpatch.pl reports: WARNING: line over 80 characters > + * ef_list array and ef_count > + */ > + *ef_list = kcalloc(count, sizeof(**ef_list), GFP_KERNEL); > + *ef_count = count; > > - efence = to_amdgpu_amdkfd_fence(f); > - if (efence) { > - fence_list[k++] = efence; > - fobj->shared_count--; > - } else { > - RCU_INIT_POINTER(fobj->shared[j++], f); > + if (!*ef_list) { > + kfree(new); > + return -ENOMEM; > } > } > > + /* Install the new fence list, seqcount provides the barriers */ > + preempt_disable(); > + write_seqcount_begin(&resv->seq); > + RCU_INIT_POINTER(resv->fence, new); > write_seqcount_end(&resv->seq); > preempt_enable(); > > - *ef_list = fence_list; > - *ef_count = k; > + /* Drop the references to the removed fences or move them to ef_list */ > + for (i = j, k = 0; i < old->shared_count; ++i) { > + struct dma_fence *f; > + > + f = rcu_dereference_protected(new->shared[i], > + reservation_object_held(resv)); > + if (!ef) > + (*ef_list)[k++] = to_amdgpu_amdkfd_fence(f); > + else > + dma_fence_put(f); > + } > + kfree_rcu(old, rcu); > > return 0; > }
