On 2/6/2023 10:34 PM, Matthew Wilcox wrote:
> 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;
Yes. This also works.
>
> 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.
OK. Will use count in next version.
Regards
Yin, Fengwei
