Re: [PATCH] mm/khugepaged: use [pmd|pte]_addr for better reading

[Lance Yang <lance.yang@linux.dev>]

On 2025/9/20 17:00, Wei Yang wrote:
> On Sat, Sep 20, 2025 at 12:30:52PM +0800, Lance Yang wrote:
>>
>> On 2025/9/20 08:54, Wei Yang wrote:
>>> When collapse a pmd, there are two address in use:
>>>
>>>     * address points to the start of pmd
>>>     * address points to each individual page
>>>
>>> Current naming is not easy to distinguish these two and error prone.
>>>
>>> Name the first one to pmd_addr and second one to pte_addr.
>>>
>>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>>> Suggested-by: David Hildenbrand <david@redhat.com>
>>
>> This renaming makes the code much easier to follow, but just
>> some minor style nits below :)
>>
>> Acked-by: Lance Yang <lance.yang@linux.dev>
>>
>>> ---
>>>    mm/khugepaged.c | 43 ++++++++++++++++++++++---------------------
>>>    1 file changed, 22 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>> index 4c957ce788d1..6d03072c1a92 100644
>>> --- a/mm/khugepaged.c
>>> +++ b/mm/khugepaged.c
>>> @@ -537,18 +537,19 @@ static void release_pte_pages(pte_t *pte, pte_t *_pte,
>>>    }
>>>    static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
>>> -					unsigned long address,
>>> +					unsigned long pmd_addr,
>>>    					pte_t *pte,
>>>    					struct collapse_control *cc,
>>>    					struct list_head *compound_pagelist)
>>>    {
>>>    	struct page *page = NULL;
>>>    	struct folio *folio = NULL;
>>> +	unsigned long pte_addr = pmd_addr;
>>>    	pte_t *_pte;
>>>    	int none_or_zero = 0, shared = 0, result = SCAN_FAIL, referenced = 0;
>>
>> Nit: could we refactor this block into the "reverse christmas tree" style?
>>
> 
> You mean sth like this?
> 
>     	int none_or_zero = 0, shared = 0, result = SCAN_FAIL, referenced = 0;
>     	struct page *page = NULL;
>     	struct folio *folio = NULL;
>    	unsigned long pte_addr = pmd_addr;
>     	pte_t *_pte;

That's getting closer. The formatting would be something like this:

	int none_or_zero = 0, shared = 0, result = SCAN_FAIL, referenced = 0;
	unsigned long pte_addr = pmd_addr;
	struct folio *folio = NULL;
	struct page *page = NULL;
	pte_t *_pte;

Feel free to change it :)