On Sat, Sep 25, 2021 at 01:54:22PM -0700, Nadav Amit wrote: > @@ -338,25 +344,25 @@ static unsigned long change_protection_range(struct vm_area_struct *vma, > struct mm_struct *mm = vma->vm_mm; > pgd_t *pgd; > unsigned long next; > - unsigned long start = addr; > unsigned long pages = 0; > + struct mmu_gather tlb; > > BUG_ON(addr >= end); > pgd = pgd_offset(mm, addr); > flush_cache_range(vma, addr, end); > inc_tlb_flush_pending(mm); > + tlb_gather_mmu(&tlb, mm); > + tlb_start_vma(&tlb, vma); Pure question: I actually have no idea why tlb_start_vma() is needed here, as protection range can be just a single page, but anyway.. I do see that tlb_start_vma() contains a whole-vma flush_cache_range() when the arch needs it, then does it mean that besides the inc_tlb_flush_pending() to be dropped, so as to the other call to flush_cache_range() above? > do { > next = pgd_addr_end(addr, end); > if (pgd_none_or_clear_bad(pgd)) > continue; > - pages += change_p4d_range(vma, pgd, addr, next, newprot, > + pages += change_p4d_range(&tlb, vma, pgd, addr, next, newprot, > cp_flags); > } while (pgd++, addr = next, addr != end); > > - /* Only flush the TLB if we actually modified any entries: */ > - if (pages) > - flush_tlb_range(vma, start, end); > - dec_tlb_flush_pending(mm); > + tlb_end_vma(&tlb, vma); > + tlb_finish_mmu(&tlb); > > return pages; > } > -- > 2.25.1 > -- Peter Xu