Re: [RFC PATCH 2/5] filemap: do file page mapping with folio granularity

["Huang, Ying" <ying.huang@intel.com>]

Yin Fengwei <fengwei.yin@intel.com> writes:

> Add function to do file page mapping based on folio and update
> filemap_map_pages() to use new function. So the filemap page
> mapping will deal with folio granularity instead of page
> granularity. This allow batched folio refcount update.
>
> Signed-off-by: Yin Fengwei <fengwei.yin@intel.com>
> ---
>  mm/filemap.c | 82 ++++++++++++++++++++++++++++++----------------------
>  1 file changed, 48 insertions(+), 34 deletions(-)
>
> diff --git a/mm/filemap.c b/mm/filemap.c
> index c915ded191f0..fe0c226c8b1e 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -3351,6 +3351,43 @@ static inline struct folio *next_map_page(struct address_space *mapping,
>  				  mapping, xas, end_pgoff);
>  }
>  
> +
> +static vm_fault_t filemap_map_folio_range(struct vm_fault *vmf,
> +	struct folio *folio, struct page *page, unsigned long addr,
> +	int len)

As Matthew pointed out, we should rename 'len'. And some comments about
the meaning of the parameters should be good.  For example,

/* Map sub-pages [start_page, start_page + nr_pages) of folio */
static vm_fault_t filemap_map_folio_range(struct vm_fault *vmf,
	struct folio *folio, struct page *start_page, unsigned int nr_pages,
        unsigned long start)

Best Regards,
Huang, Ying

> +{
> +	vm_fault_t ret = 0;
> +	struct vm_area_struct *vma = vmf->vma;
> +	struct file *file = vma->vm_file;
> +	unsigned int mmap_miss = READ_ONCE(file->f_ra.mmap_miss);
> +	int ref_count = 0, count = 0;
> +
> +	do {
> +		if (PageHWPoison(page))
> +			continue;
> +
> +		if (mmap_miss > 0)
> +			mmap_miss--;
> +
> +		if (!pte_none(*vmf->pte))
> +			continue;
> +
> +		if (vmf->address == addr)
> +			ret = VM_FAULT_NOPAGE;
> +
> +		ref_count++;
> +
> +		do_set_pte(vmf, page, addr);
> +		update_mmu_cache(vma, addr, vmf->pte);
> +
> +	} while (vmf->pte++, page++, addr += PAGE_SIZE, ++count < len);
> +
> +	folio_ref_add(folio, ref_count);
> +	WRITE_ONCE(file->f_ra.mmap_miss, mmap_miss);
> +
> +	return ret;
> +}
> +
>  vm_fault_t filemap_map_pages(struct vm_fault *vmf,
>  			     pgoff_t start_pgoff, pgoff_t end_pgoff)
>  {
> @@ -3361,9 +3398,9 @@ vm_fault_t filemap_map_pages(struct vm_fault *vmf,
>  	unsigned long addr;
>  	XA_STATE(xas, &mapping->i_pages, start_pgoff);
>  	struct folio *folio;
> -	struct page *page;
>  	unsigned int mmap_miss = READ_ONCE(file->f_ra.mmap_miss);
>  	vm_fault_t ret = 0;
> +	int len = 0;
>  
>  	rcu_read_lock();
>  	folio = first_map_page(mapping, &xas, end_pgoff);
> @@ -3378,45 +3415,22 @@ vm_fault_t filemap_map_pages(struct vm_fault *vmf,
>  	addr = vma->vm_start + ((start_pgoff - vma->vm_pgoff) << PAGE_SHIFT);
>  	vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, addr, &vmf->ptl);
>  	do {
> -again:
> -		page = folio_file_page(folio, xas.xa_index);
> -		if (PageHWPoison(page))
> -			goto unlock;
> -
> -		if (mmap_miss > 0)
> -			mmap_miss--;
> +		struct page *page;
> +		unsigned long end;
>  
> +		page = folio_file_page(folio, xas.xa_index);
>  		addr += (xas.xa_index - last_pgoff) << PAGE_SHIFT;
> -		vmf->pte += xas.xa_index - last_pgoff;
> +		vmf->pte += xas.xa_index - last_pgoff - len;
>  		last_pgoff = xas.xa_index;
> +		end = folio->index + folio_nr_pages(folio) - 1;
> +		len = min(end, end_pgoff) - xas.xa_index + 1;
>  
> -		/*
> -		 * NOTE: If there're PTE markers, we'll leave them to be
> -		 * handled in the specific fault path, and it'll prohibit the
> -		 * fault-around logic.
> -		 */
> -		if (!pte_none(*vmf->pte))
> -			goto unlock;
> -
> -		/* We're about to handle the fault */
> -		if (vmf->address == addr)
> +		if (VM_FAULT_NOPAGE ==
> +			filemap_map_folio_range(vmf, folio, page, addr, len))
>  			ret = VM_FAULT_NOPAGE;
>  
> -		do_set_pte(vmf, page, addr);
> -		/* no need to invalidate: a not-present page won't be cached */
> -		update_mmu_cache(vma, addr, vmf->pte);
> -		if (folio_more_pages(folio, xas.xa_index, end_pgoff)) {
> -			xas.xa_index++;
> -			folio_ref_inc(folio);
> -			goto again;
> -		}
> -		folio_unlock(folio);
> -		continue;
> -unlock:
> -		if (folio_more_pages(folio, xas.xa_index, end_pgoff)) {
> -			xas.xa_index++;
> -			goto again;
> -		}
> +		xas.xa_index = end;
> +
>  		folio_unlock(folio);
>  		folio_put(folio);
>  	} while ((folio = next_map_page(mapping, &xas, end_pgoff)) != NULL);