On 6/8/19 4:41 AM, Jason Gunthorpe wrote: > On Sat, Jun 08, 2019 at 02:10:08AM -0700, Christoph Hellwig wrote: >> On Fri, Jun 07, 2019 at 05:14:52PM -0700, Ralph Campbell wrote: >>> HMM defines its own struct hmm_update which is passed to the >>> sync_cpu_device_pagetables() callback function. This is >>> sufficient when the only action is to invalidate. However, >>> a device may want to know the reason for the invalidation and >>> be able to see the new permissions on a range, update device access >>> rights or range statistics. Since sync_cpu_device_pagetables() >>> can be called from try_to_unmap(), the mmap_sem may not be held >>> and find_vma() is not safe to be called. >>> Pass the struct mmu_notifier_range to sync_cpu_device_pagetables() >>> to allow the full invalidation information to be used. >>> >>> Signed-off-by: Ralph Campbell <rcampbell@xxxxxxxxxx> >>> >>> I'm sending this out now since we are updating many of the HMM APIs >>> and I think it will be useful. >> >> This is the right thing to do. But the really right thing is to just >> kill the hmm_mirror API entirely and move to mmu_notifiers. At least >> for noveau this already is way simpler, although right now it defeats >> Jasons patch to avoid allocating the struct hmm in the fault path. >> But as said before that can be avoided by just killing struct hmm, >> which for many reasons is the right thing to do anyway. >> >> I've got a series here, which is a bit broken (epecially the last >> patch can't work as-is), but should explain where I'm trying to head: >> >> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/hmm-mirror-simplification > > At least the current hmm approach does rely on the collision retry > locking scheme in struct hmm/struct hmm_range for the pagefault side > to work right. > > So, before we can apply patch one in this series we need to fix > hmm_vma_fault() and all its varients. Otherwise the driver will be > broken. > > I'm hoping to first define what this locking should be (see other > emails to Ralph) then, ideally, see if we can extend mmu notifiers to > get it directly withouth hmm stuff. > > Then we apply your patch one and the hmm ops wrapper dies. > This all makes sense, and thanks for all this work to simplify and clarify HMM. It's going to make it a lot easier to work with, when the dust settles. thanks, -- John Hubbard NVIDIA
