Re: [PATCH v3 10/13] mm/huge_memory: separate out the folio part of zap_huge_pmd()

[Baolin Wang <baolin.wang@linux.alibaba.com>]

On 3/23/26 6:42 PM, Lorenzo Stoakes (Oracle) wrote:
> On Sat, Mar 21, 2026 at 01:59:34PM +0800, Baolin Wang wrote:
>>
>>
>> On 3/21/26 2:07 AM, Lorenzo Stoakes (Oracle) wrote:
>>> Place the part of the logic that manipulates counters and possibly updates
>>> the accessed bit of the folio into its own function to make zap_huge_pmd()
>>> more readable.
>>>
>>> Also rename flush_needed to is_present as we only require a flush for
>>> present entries.
>>>
>>> Additionally add comments as to why we're doing what we're doing with
>>> respect to softleaf entries.
>>>
>>> This also lays the ground for further refactoring.
>>>
>>> Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>
>>> ---
>>>    mm/huge_memory.c | 61 +++++++++++++++++++++++++++---------------------
>>>    1 file changed, 35 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>>> index 673d0c4734ad..9ddf38d68406 100644
>>> --- a/mm/huge_memory.c
>>> +++ b/mm/huge_memory.c
>>> @@ -2325,6 +2325,37 @@ static inline void zap_deposited_table(struct mm_struct *mm, pmd_t *pmd)
>>>    	mm_dec_nr_ptes(mm);
>>>    }
>>> +static void zap_huge_pmd_folio(struct mm_struct *mm, struct vm_area_struct *vma,
>>> +		pmd_t pmdval, struct folio *folio, bool is_present,
>>> +		bool *has_deposit)
>>> +{
>>> +	const bool is_device_private = folio_is_device_private(folio);
>>> +
>>> +	/* Present and device private folios are rmappable. */
>>> +	if (is_present || is_device_private)
>>> +		folio_remove_rmap_pmd(folio, &folio->page, vma);
>>> +
>>> +	if (folio_test_anon(folio)) {
>>> +		*has_deposit = true;
>>> +		add_mm_counter(mm, MM_ANONPAGES, -HPAGE_PMD_NR);
>>> +	} else {
>>> +		add_mm_counter(mm, mm_counter_file(folio),
>>> +			       -HPAGE_PMD_NR);
>>> +
>>> +		/*
>>> +		 * Use flush_needed to indicate whether the PMD entry
>>> +		 * is present, instead of checking pmd_present() again.
>>> +		 */
>>> +		if (is_present && pmd_young(pmdval) &&
>>> +		    likely(vma_has_recency(vma)))
>>> +			folio_mark_accessed(folio);
>>
>> Nit: these comments were added by me to explain why 'flush_needed' was
>> used:). Since it has been renamed to the more readable 'is_present', these
>> comments are now redundant and can be removed.
> 
> Ack, I think it's _probably_ ok to leave that as a later commit removes it
> anyway, if that works for you?

Ah, I saw you removed them in the following patch. Looks fine to me. Thanks.