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@xxxxxxxxxx> wrote:
Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx> writes:
On Wed, 22 Nov 2017 13:16:08 -0800
Eric Anholt <eric@xxxxxxxxxx> wrote:
Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx> 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
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
