On Wed, Jun 19, 2019 at 01:07:05AM -0700, Christoph Hellwig wrote: > On Wed, Jun 19, 2019 at 12:53:55AM +0000, Kuehling, Felix wrote: > > This code is derived from our old MMU notifier code. Before HMM we used > > to register a single MMU notifier per mm_struct and look up virtual > > address ranges that had been registered for mirroring via driver API > > calls. The idea was to reuse a single MMU notifier for the life time of > > the process. It would remain registered until we got a notifier_release. > > > > hmm_mirror took the place of that when we converted the code to HMM. > > > > I suppose we could destroy the mirror earlier, when we have no more > > registered virtual address ranges, and create a new one if needed later. > > I didn't write the code, but if you look at hmm_mirror it already is > a multiplexer over the mmu notifier, and the intent clearly seems that > you register one per range that you want to mirror, and not multiplex > it once again. In other words - I think each amdgpu_mn_node should > probably have its own hmm_mirror. And while the amdgpu_mn_node objects > are currently stored in an interval tree it seems like they are only > linearly iterated anyway, so a list actually seems pretty suitable. If > not we need to improve the core data structures instead of working > around them. This looks a lot like the ODP code (amdgpu_mn_node == ib_umem_odp) The interval tree is to quickly find the driver object(s) that have the virtual pages during invalidation: static int amdgpu_mn_sync_pagetables_gfx(struct hmm_mirror *mirror, const struct hmm_update *update) { it = interval_tree_iter_first(&amn->objects, start, end); while (it) { [..] amdgpu_mn_invalidate_node(node, start, end); And following the ODP model there should be a single hmm_mirror per-mm (user can fork and stuff, this is something I want to have core code help with). The hmm_mirror can either exist so long as objects exist, or it can exist until the chardev is closed - but never longer than the chardev's lifetime. Maybe we should be considering providing a mmu notifier & interval tree & lock abstraction since ODP & AMD are very similar here.. Jason