On Tue, 2024-05-07 at 15:36 +0200, Christian König wrote: > Am 06.05.24 um 11:46 schrieb Thomas Hellström: > > Hi Christian, Others. > > > > In order to support exhaustive eviction there are some changes that > > I > > think needs to be made to drm_exec: > > > > 1) Trylock support > > (This is for ttm_bo_vm, ttm_buffer_object_init_reserved, and also > > for > > the eviction path where I think we want to make a trylock pass > > before a > > sleeping lock pass). > > Not sure why we need this for ttm_bo_vm, but in general that sounds > like > the right approach to me as well. In current ttm_bo_vm, if the trylock fails, the mmap lock is typically dropped while we wait for the lock to become available, and then the fault is retried, using the fault retry mechanism. (This is only to avoid blocking the mmap lock unnecessarily- the comment around that code is obsolete). > > > In essence this means we can't leave any contending lock unlocked > > until > > the next sleeping lock, but rather need to relock it on > > drm_exec_retry_on_contention(), meaning that that macro also gets > > passed and returns an error code to handle -EINTR. > > Hui what? I can't really follow what you mean here. Currently drm_exec keeps a reference to the contended lock, which is not locked. And it is locked on the *next* drm_exec_lock_obj() after a restart. So the question arises what to do if the *next* lock is a trylock. We shouldn't really do a sleeping lock of the contended object at that point. So the suggestion here was to do it during drm_exec_retry_on_contention() instead. But then we face the problem that the locking of the contended object might fail with -EINTR. > > > > > 2) Snapshot unlock > > After successfully obtaining backing store, we want to unlock all > > evicted objects. So snapshot the drm_exec state when eviction > > begins, > > and unlock everything down to the snapshot after successful backing > > store allocation. Essentially the snapshot contains the number of > > locked objects and a pointer to the prelocked object. > > Interesting idea, never though about that. Not sure if that makes the > situation better or worse. > > One goal of drm_exec was to be able to keep evicted BOs locked until > the > whole submission is completed and that obviously contradicts that. We've tried both approaches on i915 downstream (non-ttm). IIRC the snapshot idea was originally brought up by Sima. Couldn't see any real difference, but I think snapshotting should be sufficient to avoid starvation. /Thomas > > Regards, > Christian. > > > > > Any concerns? > > Thanks, > > Thomas > > >