Re: [PATCH v3 4/9] mm: introduce skip_none_ptes()

[Qi Zheng <zhengqi.arch@bytedance.com>]

On 2024/11/15 22:59, David Hildenbrand wrote:
> On 15.11.24 15:41, Qi Zheng wrote:
>>
>>
>> On 2024/11/15 18:22, David Hildenbrand wrote:
>>>>>> *nr_skip = nr;
>>>>>>
>>>>>> and then:
>>>>>>
>>>>>> zap_pte_range
>>>>>> --> nr = do_zap_pte_range(tlb, vma, pte, addr, end, details, 
>>>>>> &skip_nr,
>>>>>>                            rss, &force_flush, &force_break);
>>>>>>         if (can_reclaim_pt) {
>>>>>>             none_nr += count_pte_none(pte, nr);
>>>>>>             none_nr += nr_skip;
>>>>>>         }
>>>>>>
>>>>>> Right?
>>>>>
>>>>> Yes. I did not look closely at the patch that adds the counting of
>>>>
>>>> Got it.
>>>>
>>>>> pte_none though (to digest why it is required :) ).
>>>>
>>>> Because 'none_nr == PTRS_PER_PTE' is used in patch #7 to detect
>>>> empty PTE page.
>>>
>>> Okay, so the problem is that "nr" would be "all processed entries" but
>>> there are cases where we "process an entry but not zap it".
>>>
>>> What you really only want to know is "was any entry not zapped", which
>>> could be a simple input boolean variable passed into do_zap_pte_range?
>>>
>>> Because as soon as any entry was processed but  no zapped, you can
>>> immediately give up on reclaiming that table.
>>
>> Yes, we can set can_reclaim_pt to false when a !pte_none() entry is
>> found in count_pte_none().
> 
> I'm not sure if well need cont_pte_none(), but I'll have to take a look 
> at your new patch to see how this fits together with doing the pte_none 
> detection+skipping in do_zap_pte_range().
> 
> I was wondering if you cannot simply avoid the additional scanning and 
> simply set "can_reclaim_pt" if you skip a zap.

Maybe we can return the information whether the zap was skipped from
zap_present_ptes() and zap_nonpresent_ptes() through parameters like I
did in [PATCH v1 3/7] and [PATCH v1 4/7].

In theory, we can detect empty PTE pages in the following two ways:

1) If no zap is skipped, it means that all pte entries have been
    zap, and the PTE page must be empty.
2) If all pte entries are detected to be none, then the PTE page is
    empty.

In the error case, 1) may cause non-empty PTE pages to be reclaimed
(which is unacceptable), while the 2) will at most cause empty PTE pages
to not be reclaimed.

So the most reliable and efficient method may be:

a. If there is a zap that is skipped, stop scanning and do not reclaim
    the PTE page;
b. Otherwise, as now, detect the empty PTE page through count_pte_none()


>