Hi Peter, On 30/01/2019 05:57, Peter Xu wrote: > AMD IOMMU driver is using the clear_flush_young() to do cache flushing > but that's actually already covered by invalidate_range(). Remove the > extra notifier and the chunks.
Aren't uses of clear_flush_young() and invalidate_range() orthogonal? If I understood correctly, when doing reclaim the kernel clears the young bit from the PTE. This requires flushing secondary TLBs (ATCs), so that new accesses from devices will go through the IOMMU, set the young bit again and the kernel can accurately track page use. I didn't see invalidate_range() being called by rmap or vmscan in this case, but might just be missing it. Two MMU notifiers are used for the young bit, clear_flush_young() and clear_young(). I believe the former should invalidate ATCs so that DMA accesses participate in PTE aging. Otherwise the kernel can't know that the device is still using entries marked 'old'. The latter, clear_young() is exempted from invalidating the secondary TLBs by mmu_notifier.h and the IOMMU driver doesn't need to implement it. Thanks, Jean > > Signed-off-by: Peter Xu <pet...@redhat.com> > --- > drivers/iommu/amd_iommu_v2.c | 24 ------------------------ > 1 file changed, 24 deletions(-) > > diff --git a/drivers/iommu/amd_iommu_v2.c b/drivers/iommu/amd_iommu_v2.c > index 23dae9348ace..5d7ef750e4a0 100644 > --- a/drivers/iommu/amd_iommu_v2.c > +++ b/drivers/iommu/amd_iommu_v2.c > @@ -370,29 +370,6 @@ static struct pasid_state *mn_to_state(struct > mmu_notifier *mn) > return container_of(mn, struct pasid_state, mn); > } > > -static void __mn_flush_page(struct mmu_notifier *mn, > - unsigned long address) > -{ > - struct pasid_state *pasid_state; > - struct device_state *dev_state; > - > - pasid_state = mn_to_state(mn); > - dev_state = pasid_state->device_state; > - > - amd_iommu_flush_page(dev_state->domain, pasid_state->pasid, address); > -} > - > -static int mn_clear_flush_young(struct mmu_notifier *mn, > - struct mm_struct *mm, > - unsigned long start, > - unsigned long end) > -{ > - for (; start < end; start += PAGE_SIZE) > - __mn_flush_page(mn, start); > - > - return 0; > -} > - > static void mn_invalidate_range(struct mmu_notifier *mn, > struct mm_struct *mm, > unsigned long start, unsigned long end) > @@ -430,7 +407,6 @@ static void mn_release(struct mmu_notifier *mn, struct > mm_struct *mm) > > static const struct mmu_notifier_ops iommu_mn = { > .release = mn_release, > - .clear_flush_young = mn_clear_flush_young, > .invalidate_range = mn_invalidate_range, > }; > >