Re: [RFC PATCH v4 4/4] filemap: batched update mm counter,rmap when map file folio

[Matthew Wilcox <willy@infradead.org>]

On Mon, Feb 06, 2023 at 10:06:39PM +0800, Yin Fengwei wrote:
> @@ -3354,11 +3354,12 @@ static vm_fault_t filemap_map_folio_range(struct vm_fault *vmf,
>  	struct file *file = vma->vm_file;
>  	struct page *page = folio_page(folio, start);
>  	unsigned int mmap_miss = READ_ONCE(file->f_ra.mmap_miss);
> -	unsigned int ref_count = 0, count = 0;
> +	unsigned int mapped = 0;
> +	pte_t *pte = vmf->pte;
>  
>  	do {
>  		if (PageHWPoison(page))
> -			continue;
> +			goto map;
>  
>  		if (mmap_miss > 0)
>  			mmap_miss--;
> @@ -3368,20 +3369,34 @@ static vm_fault_t filemap_map_folio_range(struct vm_fault *vmf,
>  		 * handled in the specific fault path, and it'll prohibit the
>  		 * fault-around logic.
>  		 */
> -		if (!pte_none(*vmf->pte))
> -			continue;
> +		if (!pte_none(pte[mapped]))
> +			goto map;

I see what you're trying to do here, but do_set_pte_range() uses the
pte from vmf->pte.  Perhaps best to save it at the beginning of the
function and restore it at the end.  ie:

	pte_t *old_ptep = vmf->pte;

	} while (vmf->pte++);

	vmf->pte = old_ptep;

The only other thing that bugs me about this patch is the use of
'mapped' as the variable name.  It's in the past tense, not the future
tense, so it gives me the wrong impression about what it's counting.
While we could use 'tomap', that's kind of clunky.  'count' or even just
'i' would be better.