On Mon, Jun 30, 2014 at 05:40:42PM +0200, Joerg Roedel wrote: > On Mon, Jun 30, 2014 at 02:41:24PM +0000, Gabbay, Oded wrote: > > I did face some problems regarding the amd IOMMU v2 driver, which > > changed its behavior (see commit "iommu/amd: Implement > > mmu_notifier_release call-back") to use mmu_notifier_release and did > > some "bad things" inside that > > notifier (primarily, but not only, deleting the object which held the > > mmu_notifier object itself, which you mustn't do because of the > > locking). > > > > I'm thinking of changing that driver's behavior to use this new > > mechanism instead of using mmu_notifier_release. Does that seem > > acceptable ? Another solution will be to add a new mmu_notifier call, > > but we already ruled that out ;) > > The mmu_notifier_release() function is exactly what this new notifier > aims to do. Unless there is a very compelling reason to duplicate this > functionality I stronly NACK this approach. > >
No this patch does not duplicate it. Current user of mmu_notifier rely on file close code path to call mmu_notifier_unregister. New code like AMD IOMMUv2 or HMM can not rely on that. Thus they need a way to call the mmu_notifer_unregister (which can not be done from inside the the release call back). If you look at current code the release callback is use to kill secondary translation but not to free associated resources. All the associated resources are free later on after the release callback (well it depends if the file is close before the process is kill). So this patch aims to provide a callback to code outside of the mmu_notifier realms, a place where it is safe to cleanup the mmu_notifier and associated resources. Cheers, Jérôme Glisse _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
