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

["Yin, Fengwei" <fengwei.yin@intel.com>]

On 1/30/2023 10:14 PM, Matthew Wilcox wrote:
> On Mon, Jan 30, 2023 at 08:55:04PM +0800, Yin Fengwei wrote:
>> +++ b/mm/filemap.c
>> @@ -3360,28 +3360,52 @@ static vm_fault_t filemap_map_folio_range(struct vm_fault *vmf,
>>  	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;
>> +	int ref_count = 0, count = 0, maplen = 0;
>> +	struct page *pg = page;
> 
> OK, having read it through, I think you're making your life a lot harder
> by keeping page pointers at all.  I'd pass xas.xa_index - folio->index
> from filemap_map_pages() as a parameter called something like 'first'.
I use page pointer here because I saw other changes you made kept the
page pointer as parameter. And tried to align with that.

> 
>>  	do {
>> -		if (PageHWPoison(page))
>> +		if (PageHWPoison(page)) {
>> +			if (maplen) {
>> +				page_add_file_rmap_range(folio, pg, maplen,
>> +					vma, false);
>> +				add_mm_counter(vma->vm_mm,
>> +					mm_counter_file(pg), maplen);
> 
> Again you've made your life harder ;-)  Try something like this:
Yes. This is much better.

> 
> void do_set_pte(struct vm_fault *vmf, struct page *page, unsigned long addr)
> {
> 	do_set_pte_range(vmf, page, addr, 1);
> }
> 
> ... and then here, you can do:
> 
> 			if (maplen)
> 				do_set_pte_range(vmf, page - maplen - 1,
> 						base_addr, maplen);
> 			base_addr += (maplen + 1) * PAGE_SIZE
> 			maplen = 0;
> 			continue;
Thanks a lot for all your comments. I will address all your comments in
next version.

Regards
Yin, Fengwei

>