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

[David Hildenbrand <david@redhat.com>]

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.

-- 
Cheers,

David / dhildenb