On 2/3/2023 10:30 PM, Chih-En Lin wrote:
> On Fri, Feb 03, 2023 at 09:38:15PM +0800, Yin, Fengwei wrote:
>>
>>
>> On 2/3/2023 9:32 PM, Chih-En Lin wrote:
>>> On Fri, Feb 03, 2023 at 09:16:35PM +0800, Yin Fengwei wrote:
>>>> do_set_pte_range() allows to setup page table entries for a
>>>> specific range. It calls folio_add_file_rmap_range() to take
>>>> advantage of batched rmap update for large folio.
>>>>
>>>> Signed-off-by: Yin Fengwei <fengwei.yin@xxxxxxxxx>
>>>> ---
>>>> include/linux/mm.h | 3 +++
>>>> mm/filemap.c | 1 -
>>>> mm/memory.c | 59 ++++++++++++++++++++++++++++++----------------
>>>> 3 files changed, 42 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>>>> index d6f8f41514cc..93192f04b276 100644
>>>> --- a/include/linux/mm.h
>>>> +++ b/include/linux/mm.h
>>>> @@ -1162,6 +1162,9 @@ static inline pte_t maybe_mkwrite(pte_t pte, struct vm_area_struct *vma)
>>>>
>>>> vm_fault_t do_set_pmd(struct vm_fault *vmf, struct page *page);
>>>> void do_set_pte(struct vm_fault *vmf, struct page *page, unsigned long addr);
>>>> +void do_set_pte_range(struct vm_fault *vmf, struct folio *folio,
>>>> + unsigned long addr, pte_t *pte,
>>>> + unsigned long start, unsigned int nr);
>>>>
>>>> vm_fault_t finish_fault(struct vm_fault *vmf);
>>>> vm_fault_t finish_mkwrite_fault(struct vm_fault *vmf);
>>>> diff --git a/mm/filemap.c b/mm/filemap.c
>>>> index f444684db9f2..74046a3a0ff5 100644
>>>> --- a/mm/filemap.c
>>>> +++ b/mm/filemap.c
>>>> @@ -3386,7 +3386,6 @@ static vm_fault_t filemap_map_folio_range(struct vm_fault *vmf,
>>>>
>>>> ref_count++;
>>>> do_set_pte(vmf, page, addr);
>>>> - update_mmu_cache(vma, addr, vmf->pte);
>>>> } while (vmf->pte++, page++, addr += PAGE_SIZE, ++count < nr_pages);
>>>>
>>>> /* Restore the vmf->pte */
>>>> diff --git a/mm/memory.c b/mm/memory.c
>>>> index 7a04a1130ec1..3754b2ef166a 100644
>>>> --- a/mm/memory.c
>>>> +++ b/mm/memory.c
>>>> @@ -4257,36 +4257,58 @@ vm_fault_t do_set_pmd(struct vm_fault *vmf, struct page *page)
>>>> }
>>>> #endif
>>>>
>>>> -void do_set_pte(struct vm_fault *vmf, struct page *page, unsigned long addr)
>>>> +void do_set_pte_range(struct vm_fault *vmf, struct folio *folio,
>>>> + unsigned long addr, pte_t *pte,
>>>> + unsigned long start, unsigned int nr)
>>>> {
>>>> struct vm_area_struct *vma = vmf->vma;
>>>> bool uffd_wp = pte_marker_uffd_wp(vmf->orig_pte);
>>>> bool write = vmf->flags & FAULT_FLAG_WRITE;
>>>> + bool cow = write && !(vma->vm_flags & VM_SHARED);
>>>
>>> Why don't use is_cow_mapping()?
>>> Is there anything messed up with VM_MAYWRITE?
>> My understanding is it's not related with the mapping. It's related with
>> what operation triggers fault here. Say, if it's a writable mapping, and
>> if the read operation triggers fault here, no cow or maybe_mkwrite() needed
>> here. Thanks.
>
> Sorry, I didn't describe the thing properly.
> It makes sense for the relationship with the write/read fault.
> I'm just wondering if "!(vma->vm_flags & VM_SHARED)" is enough to determine
> the COW page? And, I also found it in do_fault().
>
> Like, copy_present_pte() use is_cow_mapping() for COW page and
> "vm_flags & VM_SHARED" for shared mapping.
>
> So, I looked up the commit that introduced the is_cow_mapping(),
> 67121172f9753 ("Allow arbitrary read-only shared pfn-remapping too").
>
> Here is the commit message:
> "
> The VM layer (for historical reasons) turns a read-only shared mmap into
> a private-like mapping with the VM_MAYWRITE bit clear. Thus checking
> just VM_SHARED isn't actually sufficient.
>
> So use a trivial helper function for the cases where we wanted to inquire
> if a mapping was COW-like or not.
> "
>
> hmm, but it is v2.6.15.
> So is "vm_flags & VM_SHARED" enough to check the COW page now?
Thanks for the detail info here. Yes. VM_MAYWRITE bit of vma->vm_flags needs
be checked for COW also.
In the page fault path, the VM_MAYWRITE bit was checked in sanitize_fault_flags():
/* Write faults on read-only mappings are impossible ... */
if (WARN_ON_ONCE(!(vma->vm_flags & VM_MAYWRITE)))
return VM_FAULT_SIGSEGV;
and bail out early if it's write fault and no VM_MAYWRITE.
My understanding is that sanitize_fault_flags() is called first before hit
do_set_pte()/do_set_pte_range().
Regards
Yin, Fengwei
>
> Thanks,
> Chih-En Lin
