Am 19.05.21 um 08:27 schrieb Thomas Hellström:
On 5/18/21 6:30 PM, Christian König wrote:
Am 18.05.21 um 18:07 schrieb Thomas Hellström:
On 5/18/21 5:42 PM, Christian König wrote:
Am 18.05.21 um 17:38 schrieb Thomas Hellström:
On 5/18/21 5:28 PM, Christian König wrote:
Am 18.05.21 um 17:20 schrieb Thomas Hellström:
On 5/18/21 5:18 PM, Christian König wrote:
Am 18.05.21 um 17:15 schrieb Thomas Hellström:
On 5/18/21 10:26 AM, Thomas Hellström wrote:
We are calling the eviction_valuable driver callback at
eviction time to
determine whether we actually can evict a buffer object.
The upcoming i915 TTM backend needs the same functionality
for swapout,
and that might actually be beneficial to other drivers as well.
Add an eviction_valuable call also in the swapout path. Try
to keep the
current behaviour for all drivers by returning true if the
buffer object
is already in the TTM_PL_SYSTEM placement. We change
behaviour for the
case where a buffer object is in a TT backed placement when
swapped out,
in which case the drivers normal eviction_valuable path is run.
Finally export ttm_tt_unpopulate() and don't swap out bos
that are not populated. This allows a driver to purge a bo at
swapout time if its content is no longer valuable rather than to
have TTM swap the contents out.
Cc: Christian König <christian.koenig@xxxxxxx>
Signed-off-by: Thomas Hellström
<thomas.hellstrom@xxxxxxxxxxxxxxx>
Christian,
Here we have a ttm_tt_unpopulate() export as well at the end.
I figure you will push back on that one. What we really need
is a functionality to just drop the bo contents and end up in
system memory unpopulated. Should I perhaps add a utility
function to do that instead? like ttm_bo_purge()?
We already have that. Just call ttm_bo_validate() without any
place to put the buffer.
See how ttm_bo_pipeline_gutting() is used.
Christian.
OK, so is that reentrant from the move() or swap_notify() callback.
That sounds like a design bug to me since you should never need
to do this.
When you want to destroy the backing store of a buffer during
eviction you should just do this by returning an empty placement
from the evict_flags callback.
So this is for the functionality where the user has indicated that
the contents is no longer of value, but the buffer itself
is cached until evicted or swapped out for performance reasons. So
the above would work for eviction, but what about swapout. Could
we add some similar functionality there?
Amdgpu has the same functionality and you don't need to handle swap
at all.
Just return from the evict_flags that you want to drop the backing
store as soon as the BO leaves the GTT domain.
Hmm, the pipeline_gutting function seems ok, but overly complex if
the bo is already idle, Am I allowed to optimize it slightly for the
latter case?
Yeah, sure. We just never hat that use case so far.
One thing about the code here that makes me worried is that the
"destination" ttm_tt is allocated *after* pipeline_gutting. We're not
really allowed to fail here because that would leave the BO in a state
where codepaths (fault for example) try to access a NULL ttm_tt. While
the idle case can get away with ttm_tt_unpopulate, for the async case,
ttm_tt really needs to be pre-allocated, so that we can leave the bo
in a consistent state.
Well the original plan was to make tt allocation purely optional.
But I didn't had the time so far to actually fix that.
Christian.
/Thomas
Christian.
/Thomas
Christian.
/Thomas
Regards,
Christian.
/Thomas
Thanks,
Thomas
