>> Just because you found a pte that maps a page from a large folio, that doesn't
>> mean that all pages from the folio are mapped, and it doesn't mean they are
>> mapped contiguously. We have to deal with partial munmap(), partial mremap()
>> etc. We could split in these cases (and in future it might be sensible to try),
>> but that can fail (due to GUP). So we still have to handle the corner case.
>>
>> But I can imagine doing a batched version of ptep_get_and_clear(), like I did
>> for ptep_set_wrprotects(). And I think this would be an improvement.
>>
>> The reason I haven't done that so far, is because ptep_get_and_clear() returns
>> the pte value when it was cleared and that's hard to do if batching due to the
>> storage requirement. But perhaps you could just return the logical OR of the
>> dirty and young bits across all ptes in the batch. The caller should be able to
>> reconstitute the rest if it needs it?
>>
>> What do you think?
>
> I really don't know why we care about the return value of ptep_get_and_clear()
> as zap_pte_range() doesn't ask for any ret value at all. so why not totally give
> up this kind of complex logical OR of dirty and young as they are useless in
> this case?
That's not the case in v6.7-rc1:
static unsigned long zap_pte_range(struct mmu_gather *tlb,
struct vm_area_struct *vma, pmd_t *pmd,
unsigned long addr, unsigned long end,
struct zap_details *details)
{
...
do {
pte_t ptent = ptep_get(pte);
...
if (pte_present(ptent)) {
...
ptent = ptep_get_and_clear_full(mm, addr, pte,
tlb->fullmm);
arch_check_zapped_pte(vma, ptent);
tlb_remove_tlb_entry(tlb, pte, addr);
zap_install_uffd_wp_if_needed(vma, addr, pte, details,
ptent);
if (unlikely(!page)) {
ksm_might_unmap_zero_page(mm, ptent);
continue;
}
delay_rmap = 0;
if (!PageAnon(page)) {
if (pte_dirty(ptent)) {
set_page_dirty(page);
if (tlb_delay_rmap(tlb)) {
delay_rmap = 1;
force_flush = 1;
}
}
if (pte_young(ptent) && likely(vma_has_recency(vma)))
mark_page_accessed(page);
}
...
}
...
} while (pte++, addr += PAGE_SIZE, addr != end);
...
}
Most importantly, file-backed mappings need the access/dirty bits to propagate that information back to the folio, so it will be written back to disk. x86 is also looking at the dirty bit in arch_check_zapped_pte(), and ksm is using it in ksm_might_unmap_zero_page().
Probably for your use case of anon memory on arm64 on a phone, you don't need the return value. But my solution is also setting cotnpte for file-backed memory, and there are performance wins to be had there, especially for executable mappings where contpte reduces iTLB pressure. (I have other work which ensures these file-backed mappings are in correctly-sized large folios).
So I don't think we can just do a clear without the get part. But I think I have a solution in the shape of clear_ptes(), as described in the other thread, which gives the characteristics you suggest.
>
> Is it possible for us to introduce a new api like?
>
> bool clear_folio_ptes(folio, ptep)
> {
> if(ptes are contiguous mapped) {
> clear all ptes all together // this also clears all CONTPTE
> return true;
> }
> return false;
> }
>
> in zap_pte_range():
>
> if (large_folio(folio) && clear_folio_ptes(folio, ptep)) {
> addr += nr - 1
> pte += nr -1
> } else
> old path.
>
>
>>
>>>
>>> zap_pte_range is the most frequent behaviour from userspace libc heap
>>> as i explained
>>> before. libc can call madvise(DONTNEED) the most often. It is crucial
>>> to performance.
>>>
>>> and this way can also help drop your full version by moving to full
>>> flushing the whole
>>> large folios? and we don't need to depend on fullmm any more?
>>>
>>>>
>>>> I don't think there is any correctness issue here. But there is a problem with
>>>> fragility, as raised by Alistair. I have some ideas on potentially how to solve
>>>> that. I'm going to try to work on it this afternoon and will post if I get some
>>>> confidence that it is a real solution.
>>>>
>>>> Thanks,
>>>> Ryan
>>>>
>>>>>
>>>>> static inline pte_t __cont_pte_huge_ptep_get_and_clear_flush(struct mm_struct *mm,
>>>>> unsigned long addr,
>>>>> pte_t *ptep,
>>>>> bool flush)
>>>>> {
>>>>> pte_t orig_pte = ptep_get(ptep);
>>>>>
>>>>> CHP_BUG_ON(!pte_cont(orig_pte));
>>>>> CHP_BUG_ON(!IS_ALIGNED(addr, HPAGE_CONT_PTE_SIZE));
>>>>> CHP_BUG_ON(!IS_ALIGNED(pte_pfn(orig_pte), HPAGE_CONT_PTE_NR));
>>>>>
>>>>> return get_clear_flush(mm, addr, ptep, PAGE_SIZE, CONT_PTES, flush);
>>>>> }
>>>>>
>>>>> [1] https://github.com/OnePlusOSS/android_kernel_oneplus_sm8550/blob/oneplus/sm8550_u_14.0.0_oneplus11/mm/memory.c#L1539
>>>>>
>>>>>> + */
>>>>>> +
>>>>>> + return __ptep_get_and_clear(mm, addr, ptep);
>>>>>> +}
>>>>>> +EXPORT_SYMBOL(contpte_ptep_get_and_clear_full);
>>>>>> +
>>>>>
>>>
> Thanks
> Barry
