On 13/11/17 10:54 AM, Christian König wrote:
> Deleted BOs with the same reservation object can be reaped even if they
> can't be reserved.
>
> v2: rebase and we still need to remove/add the BO from/to the LRU.
> v3: fix remove/add one more time, cleanup the logic a bit
> v4: we should still check if the eviction is valuable
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
[...]
> - ret = reservation_object_trylock(bo->resv) ? 0 : -EBUSY;
> - if (ret)
> - continue;
> + if (bo->resv == resv) {
> + if (list_empty(&bo->ddestroy))
> + continue;
> +
Superfluous blank line here.
> + } else {
> + locked = reservation_object_trylock(bo->resv);
> + if (!locked)
> + continue;
> + }
>
> if (place && !bdev->driver->eviction_valuable(bo,
> place)) {
> - reservation_object_unlock(bo->resv);
> - ret = -EBUSY;
> + if (locked)
> + reservation_object_unlock(bo->resv);
I think we need to always set bo = NULL before continue, otherwise we
can end up trying to evict the very last BO even though we didn't
consider it suitable for eviction (and if
bdev->driver->eviction_valuable returned false, with locked = true even
though we already unlocked bo->resv).
> - if (!ret)
> + if (&bo->lru != &man->lru[i])
I'm not sure what the purpose of this test is, can you explain?
> break;
> + else
> + bo = NULL;
> }
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
