[PATCH 1/4] drm/amdgpu: Optimization of critical section

[Dave Airlie]

On 13 June 2017 at 09:00, Christian König <deathsimple at vodafone.de> wrote:
> Am 12.06.2017 um 22:31 schrieb Alex Xie:
>>
>> This patch is to move a loop of unref BOs and
>> several memory free function calls out of
>> critical sections.
>>
>> Signed-off-by: Alex Xie <AlexBin.Xie at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
>> index a664987..02c138f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
>> @@ -75,9 +75,12 @@ static void amdgpu_bo_list_destroy(struct amdgpu_fpriv
>> *fpriv, int id)
>>                 /* Another user may have a reference to this list still */
>>                 mutex_lock(&list->lock);
>>                 mutex_unlock(&list->lock);
>> +               mutex_unlock(&fpriv->bo_list_lock);
>>                 amdgpu_bo_list_free(list);
>>         }
>> -       mutex_unlock(&fpriv->bo_list_lock);
>> +       else {
>> +               mutex_unlock(&fpriv->bo_list_lock);
>> +       }
>
>
> You could move the unlock of bo_list_lock even before the if.
>
> But since you pointed it out there is quite a bug in this function:
>>
>>                 /* Another user may have a reference to this list still */
>>                 mutex_lock(&list->lock);
>>                 mutex_unlock(&list->lock);
>
> Not sure if that is still up to date, but that use case used to be illegal
> with mutexes.

Oh wow, looks like this code has refcounting bugs, there should never
be a reason to lock/unlock to find another user.

Can anyone say krefs?

Dave.