[PATCH] mm/khugepaged: fix the address passed to notifier on testing young

Commit 8ee53820edfd ("thp: mmu_notifier_test_young") introduced
mmu_notifier_test_young(), but we should pass the address need to test.
In xxx_scan_pmd(), the actual iteration address is "_address" not
"address". We seem to misuse the variable on the very beginning.

Change it to the right one.

Fixes: 8ee53820edfd ("thp: mmu_notifier_test_young")
Signed-off-by: Wei Yang <redacted>
Cc: David Hildenbrand <redacted>
Cc: Lorenzo Stoakes <redacted>
Cc: Zi Yan <ziy@nvidia.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Liam R. Howlett <redacted>
Cc: Nico Pache <redacted>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Dev Jain <dev.jain@arm.com>
Cc: Barry Song <baohua@kernel.org>
CC: <redacted>

---
The original commit 8ee53820edfd is at 2011.
Then the code is moved to khugepaged.c in commit b46e756f5e470 ("thp:
extract khugepaged from mm/huge_memory.c") in 2022.
---
 mm/khugepaged.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 24e18a7f8a93..b000942250d1 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c

@@ -1418,7 +1418,7 @@ static int hpage_collapse_scan_pmd(struct mm_struct *mm,
 		if (cc->is_khugepaged &&
 		    (pte_young(pteval) || folio_test_young(folio) ||
 		     folio_test_referenced(folio) || mmu_notifier_test_young(vma->vm_mm,
-								     address)))
+								     _address)))
 			referenced++;
 	}
 	if (!writable) {

-- 
2.34.1

On 22/08/25 12:03 pm, Wei Yang wrote:

Wow, I have gone through this code so many times and never noticed this.

Reviewed-by: Dev Jain <dev.jain@arm.com>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

On 22/08/25 12:03 pm, Wei Yang wrote:

Commit 8ee53820edfd ("thp: mmu_notifier_test_young") introduced
mmu_notifier_test_young(), but we should pass the address need to test.
In xxx_scan_pmd(), the actual iteration address is "_address" not
"address". We seem to misuse the variable on the very beginning.

Change it to the right one.

Fixes: 8ee53820edfd ("thp: mmu_notifier_test_young")
Signed-off-by: Wei Yang <redacted>
Cc: David Hildenbrand <redacted>
Cc: Lorenzo Stoakes <redacted>
Cc: Zi Yan <ziy@nvidia.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Liam R. Howlett <redacted>
Cc: Nico Pache <redacted>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Dev Jain <dev.jain@arm.com>
Cc: Barry Song <baohua@kernel.org>
CC: <redacted>

---

I hope you must have rebased since your previous patch got pulled, and
the difference of time between these two events is less than 1.5 hours :)

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

On Fri, Aug 22, 2025 at 01:08:57PM +0530, Dev Jain wrote:

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Now deleted.

On 22 Aug 2025, at 2:33, Wei Yang wrote:

LGTM. Reviewed-by: Zi Yan [off-list ref]

--
Best Regards,
Yan, Zi

On Fri, Aug 22, 2025 at 01:08:57PM +0530, Dev Jain wrote:

On 22/08/25 12:03 pm, Wei Yang wrote:

quoted

Commit 8ee53820edfd ("thp: mmu_notifier_test_young") introduced
mmu_notifier_test_young(), but we should pass the address need to test.
In xxx_scan_pmd(), the actual iteration address is "_address" not
"address". We seem to misuse the variable on the very beginning.

Change it to the right one.

Fixes: 8ee53820edfd ("thp: mmu_notifier_test_young")
Signed-off-by: Wei Yang <redacted>
Cc: David Hildenbrand <redacted>
Cc: Lorenzo Stoakes <redacted>
Cc: Zi Yan <ziy@nvidia.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Liam R. Howlett <redacted>
Cc: Nico Pache <redacted>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Dev Jain <dev.jain@arm.com>
Cc: Barry Song <baohua@kernel.org>
CC: <redacted>

---

I hope you must have rebased since your previous patch got pulled, and
the difference of time between these two events is less than 1.5 hours :)

Thanks

I took a look at the latest mm-new, this one applies.


-- 
Wei Yang
Help you, Help me

On Fri, Aug 22, 2025 at 01:04:51PM +0530, Dev Jain wrote:

On 22/08/25 12:03 pm, Wei Yang wrote:

quoted

Commit 8ee53820edfd ("thp: mmu_notifier_test_young") introduced
mmu_notifier_test_young(), but we should pass the address need to test.
In xxx_scan_pmd(), the actual iteration address is "_address" not
"address". We seem to misuse the variable on the very beginning.

Change it to the right one.

Fixes: 8ee53820edfd ("thp: mmu_notifier_test_young")
Signed-off-by: Wei Yang <redacted>
Cc: David Hildenbrand <redacted>
Cc: Lorenzo Stoakes <redacted>
Cc: Zi Yan <ziy@nvidia.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Liam R. Howlett <redacted>
Cc: Nico Pache <redacted>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Dev Jain <dev.jain@arm.com>
Cc: Barry Song <baohua@kernel.org>
CC: <redacted>

---
The original commit 8ee53820edfd is at 2011.
Then the code is moved to khugepaged.c in commit b46e756f5e470 ("thp:
extract khugepaged from mm/huge_memory.c") in 2022.
---
  mm/khugepaged.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 24e18a7f8a93..b000942250d1 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c

@@ -1418,7 +1418,7 @@ static int hpage_collapse_scan_pmd(struct mm_struct *mm,
              if (cc->is_khugepaged &&
                  (pte_young(pteval) || folio_test_young(folio) ||
                   folio_test_referenced(folio) || mmu_notifier_test_young(vma->vm_mm,
-                                                                  address)))
+                                                                  _address)))
                      referenced++;
      }
      if (!writable) {

Wow, I have gone through this code so many times and never noticed this.

Yeah, also I am surprised when noticing it.


-- 
Wei Yang
Help you, Help me

On 22.08.25 08:33, Wei Yang wrote:

Commit 8ee53820edfd ("thp: mmu_notifier_test_young") introduced
mmu_notifier_test_young(), but we should pass the address need to test.

... "but we are passing the wrong address".

Please just put that into a single line, that's a perfectly reasonable case to exceed 80 chars.


Acked-by: David Hildenbrand <redacted>

  			referenced++;
  	}
  	if (!writable) {

Maybe, just maybe, it's because of *horrible* variable naming.

Can someone please send a cleanup to rename address -> pmd_addr and
_address -> pte_addr or sth like that?

pretty much any naming is better than this.

-- 
Cheers

David / dhildenb

On Tue, Aug 26, 2025 at 10:53:45AM +0200, David Hildenbrand wrote:

On 22.08.25 08:33, Wei Yang wrote:

quoted

Commit 8ee53820edfd ("thp: mmu_notifier_test_young") introduced
mmu_notifier_test_young(), but we should pass the address need to test.

... "but we are passing the wrong address".

quoted

In xxx_scan_pmd(), the actual iteration address is "_address" not
"address". We seem to misuse the variable on the very beginning.

Change it to the right one.

Fixes: 8ee53820edfd ("thp: mmu_notifier_test_young")
Signed-off-by: Wei Yang <redacted>
Cc: David Hildenbrand <redacted>
Cc: Lorenzo Stoakes <redacted>
Cc: Zi Yan <ziy@nvidia.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Liam R. Howlett <redacted>
Cc: Nico Pache <redacted>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Dev Jain <dev.jain@arm.com>
Cc: Barry Song <baohua@kernel.org>
CC: <redacted>

---
The original commit 8ee53820edfd is at 2011.
Then the code is moved to khugepaged.c in commit b46e756f5e470 ("thp:
extract khugepaged from mm/huge_memory.c") in 2022.
---
  mm/khugepaged.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 24e18a7f8a93..b000942250d1 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c

@@ -1418,7 +1418,7 @@ static int hpage_collapse_scan_pmd(struct mm_struct *mm,
  		if (cc->is_khugepaged &&
  		    (pte_young(pteval) || folio_test_young(folio) ||
  		     folio_test_referenced(folio) || mmu_notifier_test_young(vma->vm_mm,
-								     address)))
+								     _address)))

Please just put that into a single line, that's a perfectly reasonable case
to exceed 80 chars.


Acked-by: David Hildenbrand <redacted>

quoted

  			referenced++;
  	}
  	if (!writable) {

Maybe, just maybe, it's because of *horrible* variable naming.

Can someone please send a cleanup to rename address -> pmd_addr and
_address -> pte_addr or sth like that?

YES THIS.

pretty much any naming is better than this.

I despise it, and I realyl underlined this on review in Nico's series because
it's just beyond belief.

It's terrible. I mean maybe even I will do something about this, if my review
load eases up at some point...

--
Cheers

David / dhildenb

Cheers, Lorenzo

On Fri, Aug 22, 2025 at 06:33:18AM +0000, Wei Yang wrote:

Commit 8ee53820edfd ("thp: mmu_notifier_test_young") introduced
mmu_notifier_test_young(), but we should pass the address need to test.
In xxx_scan_pmd(), the actual iteration address is "_address" not
"address". We seem to misuse the variable on the very beginning.

Change it to the right one.

Fixes: 8ee53820edfd ("thp: mmu_notifier_test_young")
Signed-off-by: Wei Yang <redacted>
Cc: David Hildenbrand <redacted>
Cc: Lorenzo Stoakes <redacted>
Cc: Zi Yan <ziy@nvidia.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Liam R. Howlett <redacted>
Cc: Nico Pache <redacted>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Dev Jain <dev.jain@arm.com>
Cc: Barry Song <baohua@kernel.org>
CC: <redacted>

Good spot. This code is beyond belief.

Reviewed-by: Lorenzo Stoakes <redacted>

---
The original commit 8ee53820edfd is at 2011.

Oopsies!

On Tue, Aug 26, 2025 at 10:53:45AM +0200, David Hildenbrand wrote:

On 22.08.25 08:33, Wei Yang wrote:

quoted

Commit 8ee53820edfd ("thp: mmu_notifier_test_young") introduced
mmu_notifier_test_young(), but we should pass the address need to test.

... "but we are passing the wrong address".

quoted

In xxx_scan_pmd(), the actual iteration address is "_address" not
"address". We seem to misuse the variable on the very beginning.

Change it to the right one.

Fixes: 8ee53820edfd ("thp: mmu_notifier_test_young")
Signed-off-by: Wei Yang <redacted>
Cc: David Hildenbrand <redacted>
Cc: Lorenzo Stoakes <redacted>
Cc: Zi Yan <ziy@nvidia.com>
Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
Cc: Liam R. Howlett <redacted>
Cc: Nico Pache <redacted>
Cc: Ryan Roberts <ryan.roberts@arm.com>
Cc: Dev Jain <dev.jain@arm.com>
Cc: Barry Song <baohua@kernel.org>
CC: <redacted>

---
The original commit 8ee53820edfd is at 2011.
Then the code is moved to khugepaged.c in commit b46e756f5e470 ("thp:
extract khugepaged from mm/huge_memory.c") in 2022.
---
  mm/khugepaged.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 24e18a7f8a93..b000942250d1 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c

@@ -1418,7 +1418,7 @@ static int hpage_collapse_scan_pmd(struct mm_struct *mm,
  		if (cc->is_khugepaged &&
  		    (pte_young(pteval) || folio_test_young(folio) ||
  		     folio_test_referenced(folio) || mmu_notifier_test_young(vma->vm_mm,
-								     address)))
+								     _address)))

Please just put that into a single line, that's a perfectly reasonable case
to exceed 80 chars.


Acked-by: David Hildenbrand <redacted>

Thanks.

@Andrew

Would you mind adjust the changelog and put it into one line?

quoted

  			referenced++;
  	}
  	if (!writable) {

Maybe, just maybe, it's because of *horrible* variable naming.

Can someone please send a cleanup to rename address -> pmd_addr and
_address -> pte_addr or sth like that?

pretty much any naming is better than this.

-- 
Cheers

David / dhildenb

-- 
Wei Yang
Help you, Help me

On Tue, Aug 26, 2025 at 10:03:27AM +0100, Lorenzo Stoakes wrote:
[...]

quoted

quoted

  			referenced++;
  	}
  	if (!writable) {

Maybe, just maybe, it's because of *horrible* variable naming.

Can someone please send a cleanup to rename address -> pmd_addr and
_address -> pte_addr or sth like that?

YES THIS.

quoted

pretty much any naming is better than this.

I despise it, and I realyl underlined this on review in Nico's series because
it's just beyond belief.

I see your comment to Nico's series.

It's terrible. I mean maybe even I will do something about this, if my review
load eases up at some point...

If you are fine, I could help to do the renaming.

Cheers, Lorenzo

-- 
Wei Yang
Help you, Help me