Re: [PATCH v5 21/27] iommu/arm-smmu-v3: Put the SVA mmu notifier in the smmu_domain

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, Mar 20, 2024 at 12:23:02AM +0800, Michael Shavit wrote:
> >  static void arm_smmu_sva_domain_free(struct iommu_domain *domain)
> >  {
> > -       kfree(to_smmu_domain(domain));
> > +       struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> > +
> > +       /*
> > +        * Ensure the ASID is empty in the iommu cache before allowing reuse.
> > +        */
> > +       arm_smmu_tlb_inv_asid(smmu_domain->smmu, smmu_domain->cd.asid);
> > +
> > +       /*
> > +        * Notice that the arm_smmu_mm_arch_invalidate_secondary_tlbs op can
> > +        * still be called/running at this point. We allow the ASID to be
> > +        * reused, and if there is a race then it just suffers harmless
> > +        * unnecessary invalidation.
> > +        */
> > +       xa_erase(&arm_smmu_asid_xa, smmu_domain->cd.asid);
> > +
> > +       /*
> > +        * Actual free is defered to the SRCU callback
> > +        * arm_smmu_mmu_notifier_free()
> > +        */
> > +       mmu_notifier_put(&smmu_domain->mmu_notifier);
> >  }
> 
> I'm far from familiar with mmu_notifier and how the sva layers manage
> its reference count....but is calling mmu_notifier_unregister instead
> of mmu_notifier_put possible here? 

Possible yes, but it is not good for performance..

The reason I created the mmnut_notifier_put() scheme is to avoid the
synchronize_srcu() penalty in places like this. We don't want to wait
around for a grace period just to destroy an iommu_domain.

So the put scheme piggybacks on the call_srcu that the does the rest
of the cleanup and we get the last bits of freeing done asynchronously.

All of this is because the notifier ops callers are protected by srcu
locks for performance and we can't guarentee no ops are running
without a grace period.

Jason