On Sat, Jun 15, 2019 at 07:21:06AM -0700, Christoph Hellwig wrote: > On Thu, Jun 13, 2019 at 09:44:49PM -0300, Jason Gunthorpe wrote: > > From: Jason Gunthorpe <jgg@xxxxxxxxxxxx> > > > > hmm_release() is called exactly once per hmm. ops->release() cannot > > accidentally trigger any action that would recurse back onto > > hmm->mirrors_sem. > > In linux-next amdgpu actually calls hmm_mirror_unregister from its > release function. That whole release function looks rather sketchy, > but we probably need to sort that out first. Does it? I see this: static void amdgpu_hmm_mirror_release(struct hmm_mirror *mirror) { struct amdgpu_mn *amn = container_of(mirror, struct amdgpu_mn, mirror); INIT_WORK(&amn->work, amdgpu_mn_destroy); schedule_work(&amn->work); } static struct hmm_mirror_ops amdgpu_hmm_mirror_ops[] = { [AMDGPU_MN_TYPE_GFX] = { .sync_cpu_device_pagetables = amdgpu_mn_sync_pagetables_gfx, .release = amdgpu_hmm_mirror_release }, [AMDGPU_MN_TYPE_HSA] = { .sync_cpu_device_pagetables = amdgpu_mn_sync_pagetables_hsa, .release = amdgpu_hmm_mirror_release }, }; Am I looking at the wrong thing? Looks like it calls it through a work queue should should be OK.. Though very strange that amdgpu only destroys the mirror via release, that cannot be right. Jason