On Tue, Apr 14, 2020 at 07:02:29PM +0200, Jean-Philippe Brucker wrote:
> The new allocation scheme introduced by commit 2c7933f53f6b
> ("mm/mmu_notifiers: add a get/put scheme for the registration") provides
> a convenient way for users to attach notifier data to an mm. However, it
> would be even better to create this notifier data atomically.
>
> Since the alloc_notifier() callback only takes an mm argument at the
> moment, some users have to perform the allocation in two times.
> alloc_notifier() initially creates an incomplete structure, which is
> then finalized using more context once mmu_notifier_get() returns. This
> second step requires extra care to order memory accesses against live
> invalidation.
>
> The IOMMU SVA module, which attaches an mm to multiple devices,
> exemplifies this situation. In essence it does:
>
> mmu_notifier_get()
> alloc_notifier()
> A = kzalloc()
> /* MMU notifier is published */
> A->ctx = ctx; // (1)
> device->A = A;
> list_add_rcu(device, A->devices); // (2)
>
> The invalidate notifier, which may start running before A is fully
> initialized, does the following:
>
> io_mm_invalidate(A)
> list_for_each_entry_rcu(device, A->devices)
> device->invalidate(A->ctx)
This could probably also have been reliably fixed by not having A->ctx
be allocated memory, but inlined into the notifier struct
But I can't think of a down side to not add a params either.
Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
Regards,
Jason
