RE: [RFC 03/11] drm: introduce drm evictable LRU

["Zeng, Oak" <oak.zeng@xxxxxxxxx>]

 

 

From: Christian König <christian.koenig@xxxxxxx>
Sent: Friday, November 3, 2023 5:36 AM
To: Zeng, Oak <oak.zeng@xxxxxxxxx>; dri-devel@xxxxxxxxxxxxxxxxxxxxx; intel-xe@xxxxxxxxxxxxxxxxxxxxx
Cc: Thomas.Hellstrom@xxxxxxxxxxxxxxx; felix.kuehling@xxxxxxx; airlied@xxxxxxxxx; Welty, Brian <brian.welty@xxxxxxxxx>
Subject: Re: [RFC 03/11] drm: introduce drm evictable LRU

 

Am 03.11.23 um 05:04 schrieb Zeng, Oak:[SNIP]

I also want to have a more advanced iterator at some point where we grab

the BO lock for keeping a reference into the LRU list. Not sure how to

do this if we don't have the BO here any more.

Need to think about that further,

Don't quite get the what you want to do with the advanced iterator. But with this work, the lru entity is a base class of ttm_resource or any other resource struct in hmm/svm. Lru is decoupled from bo concept - this is why this lru can be shared with svm code which is bo-less.

This is just a crazy idea I had because TTM tends to perform bad on certain tasks.

When we start to evict something we use a callback which indicates if an eviction is valuable or not. So it can happen that we have to skip quite a bunch of BOs on the LRU until we found one which is worth evicting.

Not it can be that the first eviction doesn't make enough room to fulfill the allocation requirement, in this case we currently start over at the beginning searching for some BO to evict.

I want to avoid this by being able to have cursors into the LRU, e.g. the next BO which can't move until we have evicted the current one.

 

 

Got you now. I didn’t know this problem so I didn’t try to fix this efficiency problem in this series. Theoretically I think we can fix this issue this way: change ttm_mem_evict_first to ttm_mem_evict_first_n and add a parameter to this function to specify how much room we want to yield; then we evict the first n objects to make enough room before return, or fail if we can’t make enough room. This scheme would need the caller of ttm_mem_evict_first to tell how much room he need – I think reasonable.

BTW: How do you handle eviction here? I mean we can't call the evict callback with the spinlock held easily?

 

I was actually struggling when I refactored ttm_mem_evict_first function. I moved this function to lru manager and abstracted 3 callback functions (evict_allowable/valuable, evict_entity, evict_busy_entity) – those need to relook when hmm/svm codes come in picture. I tried not to change any logic of this function – I know people worked on this function in the past 15 years so better to be very careful.

 

So in my current implementation, spinlock is held calling the evict_entity callback. Spinlock is unlocked before calling ttm_bo_evict in the evict_entity callback and re-held if we need to move entity in lru list. See details in patch 4 and patch 10. So it keeps exactly the original call sequence but does look awkward.

 

But I think you are right. We can release the spinlock in the drm_lru_evict_first function before calling evict callback.

 

Oak

Christian.

Oak