[PATCH] drm/amdgpu: Fix blocking in RCU critical section(v2)

[axie]

Hi Emil,

Sorry for late reply. I was so busy last week and this week.

Your logic looks correct, smarter and shorter. I feel relatively more 
difficult to understand the logic, with two "!" and one "||" in the same 
if statement.

Thanks for the advice always.

On 2017-07-20 02:29 PM, Emil Velikov wrote:
> Hi Alex,
>
> On 20 July 2017 at 03:46, Alex Xie <AlexBin.Xie at amd.com> wrote:
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
>> @@ -198,12 +198,18 @@ amdgpu_bo_list_get(struct amdgpu_fpriv *fpriv, int id)
>>          result = idr_find(&fpriv->bo_list_handles, id);
>>
>>          if (result) {
>> -               if (kref_get_unless_zero(&result->refcount))
>> +               if (kref_get_unless_zero(&result->refcount)) {
>> +                       rcu_read_unlock();
>>                          mutex_lock(&result->lock);
>> -               else
>> +               }
>> +               else {
>> +                       rcu_read_unlock();
>>                          result = NULL;
>> +               }
>> +       }
>> +       else {
>> +               rcu_read_unlock();
>>          }
>> -       rcu_read_unlock();
>>
>>          return result;
> A drive-by suggestion - feel free to ignore.
>
> The "return early" approach seems great IMHO. The code will be
> shorter, indentation - less, no {}/else to track plus overall it seems
> clearer.
> Namely:
>
>     result = idr_find(&fpriv->bo_list_handles, id);
>
>     if (!result || !kref_get_unless_zero(&result->refcount)) {
>           rcu_read_unlock();
>           return NULL;
>     }
>
>     rcu_read_unlock();
>     mutex_lock(&result->lock);
>     return result;
> }
>
> HTH
> Emil