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

[Boris Brezillon <boris.brezillon@free-electrons.com>]

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.

Anyway, I can also use a regular counter and protect it with the madv
mutex, it's just that I thought using the existing refcount
infrastructure would be safer than open-coding it (actually, I found
several unbalanced refcounting issues thanks to this API while
developing the feature).
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel