[PATCH v3 hmm 11/12] mm/hmm: Remove confusing comment and logic from hmm_release
Jason Gunthorpe
jgg at ziepe.ca
Wed Jun 19 11:56:32 UTC 2019
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
More information about the amd-gfx
mailing list