On Tue, Jun 11, 2019 at 04:44:31PM -0300, Jason Gunthorpe wrote: > On Sat, Jun 08, 2019 at 01:54:25AM -0700, Christoph Hellwig wrote: > > FYI, I very much disagree with the direction this is moving. > > > > struct hmm_mirror literally is a trivial duplication of the > > mmu_notifiers. All these drivers should just use the mmu_notifiers > > directly for the mirroring part instead of building a thing wrapper > > that adds nothing but helping to manage the lifetime of struct hmm, > > which shouldn't exist to start with. > > Christoph: What do you think about this sketch below? > > It would replace the hmm_range/mirror/etc with a different way to > build the same locking scheme using some optional helpers linked to > the mmu notifier? > > (just a sketch, still needs a lot more thinking) I like the idea. A few nitpicks: Can we avoid having to store the mm in struct mmu_notifier? I think we could just easily pass it as a parameter to the helpers. The write lock case of mm_invlock_start_write_and_lock is probably worth factoring into separate helper? I can see cases where drivers want to just use it directly if they need to force getting the lock without the chance of a long wait.