bulk_move in ttm_resource manager

[Thomas Hellström]

On Wed, 2023-10-04 at 14:44 +0200, Christian König wrote:
> Am 04.10.23 um 09:17 schrieb Thomas Hellström:
> > On Wed, 2023-10-04 at 03:52 +0000, Zeng, Oak wrote:
> > > Hi Christian,
> > >   
> > > As a follow up to this thread:
> > > https://www.spinics.net/lists/dri-devel/msg410740.html, I started
> > > the
> > > work of moving the lru out of ttm_resource_manager and make it a
> > > common library for both ttm and svm. While look into the details
> > > of
> > > the bulk_move in ttm resource manager, I found a potential
> > > problem:
> > >   
> > > For simplicity, let’s say we only have one memory type and one
> > > priority, so ttm resource manager only maintains one global lru
> > > list.
> > > Let’s say this list has 10 nodes, node1 to node10.
> > >   
> > > But the lru_bulk_move is per vm. Let’s say vm1 has a bulk_move
> > > covering node range [node4, node7] and vm2 has a bulk_move
> > > covering
> > > node range [node6, node9]. Notice those two range has an overlap.
> > > Since two vm can simultaneously add nodes to lru, I think this
> > > scenario can happen.
> 
> That can't happen. See what ttm_resource_move_to_lru_tail() does when
> the BO has a bulk move associated with it.
> 
> > >   
> > > Now if we perform a bulk move for vm1, moving [node4, node7] to
> > > the
> > > tail of the lru list. The lru after this bulk move will be:
> > > node1,
> > > node2, node3,node8,node9, node10, node4, node5, node6, node7. Now
> > > notice that for vm2’s bulk_move, the first pointer  (pointing to
> > > node6) is actually after the last pointer (pointing to node9),
> > > which
> > > doesn’t make sense.
> > >   
> > > Is this a real problem? As I understand it, with this issue, we
> > > only
> > > mess up the lru list order, but there won’t be any functional
> > > problem. If it is a real problem, should we make the bulk_move
> > > global
> > > instead of per vm based?
> > >   
> > > Thanks,
> > > Oak
> > >   
> > FWIW I have a patch set that converts the TTM bulk move code to
> > using
> > sublists; a list item is either a resource or a sublist, and when
> > performing a bulk move essentially the sublist is moved. Bumping
> > resource LRU within a VM would touch only the sublist.
> 
> That sounds like my very first attempt at bulk moves which we
> abandoned 
> for various reasons.
> 
> That's easily >5years ago, but the history of that should still be on
> the mailing list if I'm not completely mistaken.

This here?

https://lists.freedesktop.org/archives/amd-gfx/2018-August/025016.html

No, in that case it's very different. Or is it an even earlier version?

/Thomas


> 
> Regards,
> Christian.
> 
> > 
> > Currently functionality and TTM API is essentially the same but
> > when
> > experimenting with LRU traversal for exhaustive WW-locking eviction
> > this concept was easier to use. Also hopefully this would reduce
> > fragility and improve understanding since a scenario like the above
> > could really never happen...
> > 
> > Let me know if I should send it out as an RFC.
> > 
> > Code is here:
> > https://gitlab.freedesktop.org/drm/xe/kernel/-/merge_requests/351/commits
> > 
> > /Thomas
> > 
> > 
> > 
> > 
> > 
>