Re: [PATCH 1/2] mm/mprotect: use mmu_gather

[Nadav Amit <nadav.amit@gmail.com>]

> On Oct 3, 2021, at 5:10 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> 
> 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);
> 
> That seems unbalanced...

Bad rebase. Thanks for catching it!

> 
>> +	tlb_gather_mmu(&tlb, mm);
>> +	tlb_start_vma(&tlb, vma);
>> 	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);
> 
> ... seeing you do remove the extra decrement.

Is it really needed? We do not put this comment elsewhere for
tlb_finish_mmu(). But no problem, I’ll keep it.