[PATCH] drm/vc4: Fix false positive WARN() backtrace on refcount_inc() usage

[Stefan Wahren]

Hi Daniel,


Am 24.11.2017 um 15:52 schrieb Daniel Vetter:
> On Thu, Nov 23, 2017 at 09:24:11AM +0100, Boris Brezillon wrote:
>> On Wed, 22 Nov 2017 16:13:31 -0800
>> Eric Anholt <eric at anholt.net> wrote:
>>
>>> Boris Brezillon <boris.brezillon at free-electrons.com> writes:
>>>
>>>> On Wed, 22 Nov 2017 13:16:08 -0800
>>>> Eric Anholt <eric at anholt.net> wrote:
>>>>   
>>>>> Boris Brezillon <boris.brezillon at free-electrons.com> writes:
>>>>>    
>>>>>> With CONFIG_REFCOUNT_FULL enabled, refcount_inc() complains when it's
>>>>>> passed a refcount object that has its counter set to 0. In this driver,
>>>>>> this is a valid use case since we want to increment ->usecnt only when
>>>>>> the BO object starts to be used by real HW components and this is
>>>>>> definitely not the case when the BO is created.
>>>>>>
>>>>>> Fix the problem by using refcount_inc_not_zero() instead of
>>>>>> refcount_inc() and fallback to refcount_set(1) when
>>>>>> refcount_inc_not_zero() returns false. Note that this 2-steps operation
>>>>>> is not racy here because the whole section is protected by a mutex
>>>>>> which guarantees that the counter does not change between the
>>>>>> refcount_inc_not_zero() and refcount_set() calls.
>>>>> If we're not following the model, and protecting the refcount by a
>>>>> mutex, shouldn't we just be using addition and subtraction instead of
>>>>> refcount's atomics?
>>>> Actually, this mutex is protecting the bo->madv value which has to be
>>>> checked when the counter reaches 0 (when decrementing) or 1 (when
>>>> incrementing). We just benefit from this protection here, but ideally
>>>> it would be better to have an refcount_inc_allow_zero() as suggested by
>>>> Daniel.
>>> Let me restate this to see if it makes sense: The refcount is always >=
>>> 0, this is is the only path that increases the refcount from 0 to 1, and
>>> it's (incidentally) protected by a mutex, so there's no race between the
>>> attempted increase from nonzero and the set from nonzero to 1.
>> Yep.
>>
>>> This seems fine to me as a bugfix.
>> The discussion is going on in the other thread, let's wait a bit
>> before taking a decision.
> Let's not block the bugfix on reaching perfection imo. I'd merge this as
> the minimal fix, and then apply the pretty paint once we have a clear idea
> for that.
> -Daniel

sorry but i can't find it in the DRI tree.

Regards
Stefan