[PATCH 2/2] drm/ttm: optimize ttm_mem_evict_first v4

Michel Dänzer michel at daenzer.net
Tue Nov 14 10:02:07 UTC 2017


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


More information about the dri-devel mailing list