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

[Eric Anholt <eric@anholt.net>]

[-- Attachment #1.1: Type: text/plain, Size: 1782 bytes --]

Boris Brezillon <boris.brezillon@free-electrons.com> writes:

> On Wed, 22 Nov 2017 13:16:08 -0800
> Eric Anholt <eric@anholt.net> wrote:
>
>> Boris Brezillon <boris.brezillon@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.

This seems fine to me as a bugfix.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel