Hi Christian,

Are you going to submit this change to amd-staging-drm-next?
amd-kfd-staging would pick it up from there automatically.

Regards,
  Felix


On 2018-08-15 01:57 PM, Felix Kuehling wrote:
> 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@247
> (see inline)
>
> With that fixed, this change is Reviewed-by: Felix Kuehling
> <[email protected]>
>
> 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 <[email protected]>
>> ---
>>  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;
>>  }

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to