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. 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
