mm/pagewalk: honor action in pgd_entry and p4d_entry

[PATCH] mm/pagewalk: honor action in pgd_entry and p4d_entry

Posted by Cao Ruichuang 5 hours ago

walk_pud_range() and walk_pmd_range() reset walk->action to ACTION_SUBTREE and honor ACTION_CONTINUE/ACTION_AGAIN after invoking their callbacks, but walk_pgd_range() and walk_p4d_range() do not.

That leaves the top levels with inconsistent callback semantics. In particular, ptdump sets ACTION_CONTINUE from pgd_entry() and p4d_entry() for leaf entries, but the generic walker still descends into lower levels instead of skipping the subtree.

Initialize walk->action before calling pgd_entry() and p4d_entry(), and handle ACTION_CONTINUE and ACTION_AGAIN afterwards just like the lower page-table levels do. Also update the action comment to reflect that it applies to pgd_entry() and p4d_entry() as well.
---
 include/linux/pagewalk.h |  4 ++--
 mm/pagewalk.c            | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/include/linux/pagewalk.h b/include/linux/pagewalk.h
index 88e18615dd7..d3f84781792 100644
--- a/include/linux/pagewalk.h
+++ b/include/linux/pagewalk.h
@@ -94,8 +94,8 @@ struct mm_walk_ops {
 };
 
 /*
- * Action for pud_entry / pmd_entry callbacks.
- * ACTION_SUBTREE is the default
+ * Action for pgd_entry / p4d_entry / pud_entry / pmd_entry callbacks.
+ * ACTION_SUBTREE is the default.
  */
 enum page_walk_action {
 	/* Descend to next level, splitting huge pages if needed and possible */
diff --git a/mm/pagewalk.c b/mm/pagewalk.c
index 4e7bcd975c5..4268e08eabb 100644
--- a/mm/pagewalk.c
+++ b/mm/pagewalk.c
@@ -261,6 +261,7 @@ static int walk_p4d_range(pgd_t *pgd, unsigned long addr, unsigned long end,
 
 	p4d = p4d_offset(pgd, addr);
 	do {
+again:
 		next = p4d_addr_end(addr, end);
 		if (p4d_none_or_clear_bad(p4d)) {
 			if (has_install)
@@ -272,11 +273,20 @@ static int walk_p4d_range(pgd_t *pgd, unsigned long addr, unsigned long end,
 			if (!has_install)
 				continue;
 		}
+
+		walk->action = ACTION_SUBTREE;
+
 		if (ops->p4d_entry) {
 			err = ops->p4d_entry(p4d, addr, next, walk);
 			if (err)
 				break;
 		}
+
+		if (walk->action == ACTION_AGAIN)
+			goto again;
+		if (walk->action == ACTION_CONTINUE)
+			continue;
+
 		if (has_handler || has_install)
 			err = walk_pud_range(p4d, addr, next, walk);
 		if (err)
@@ -302,6 +312,7 @@ static int walk_pgd_range(unsigned long addr, unsigned long end,
 	else
 		pgd = pgd_offset(walk->mm, addr);
 	do {
+again:
 		next = pgd_addr_end(addr, end);
 		if (pgd_none_or_clear_bad(pgd)) {
 			if (has_install)
@@ -313,11 +324,20 @@ static int walk_pgd_range(unsigned long addr, unsigned long end,
 			if (!has_install)
 				continue;
 		}
+
+		walk->action = ACTION_SUBTREE;
+
 		if (ops->pgd_entry) {
 			err = ops->pgd_entry(pgd, addr, next, walk);
 			if (err)
 				break;
 		}
+
+		if (walk->action == ACTION_AGAIN)
+			goto again;
+		if (walk->action == ACTION_CONTINUE)
+			continue;
+
 		if (has_handler || has_install)
 			err = walk_p4d_range(pgd, addr, next, walk);
 		if (err)

base-commit: e39f5a33eec1a4ea03358d82e861d6bb0a426b17
-- 
2.39.5 (Apple Git-154)

Re: [PATCH] mm/pagewalk: honor action in pgd_entry and p4d_entry

Posted by Vlastimil Babka (SUSE) 2 hours ago

On 4/14/26 07:18, Cao Ruichuang wrote:
> walk_pud_range() and walk_pmd_range() reset walk->action to ACTION_SUBTREE and honor ACTION_CONTINUE/ACTION_AGAIN after invoking their callbacks, but walk_pgd_range() and walk_p4d_range() do not.
> 
> That leaves the top levels with inconsistent callback semantics. In particular, ptdump sets ACTION_CONTINUE from pgd_entry() and p4d_entry() for leaf entries, but the generic walker still descends into lower levels instead of skipping the subtree.
> 
> Initialize walk->action before calling pgd_entry() and p4d_entry(), and handle ACTION_CONTINUE and ACTION_AGAIN afterwards just like the lower page-table levels do. Also update the action comment to reflect that it applies to pgd_entry() and p4d_entry() as well.

Your LLM has broken line wrapping. But it's ok, we're not interested in
patches produced by LLM reviewing code, with no human oversight and
understanding. NAK.

> ---
>  include/linux/pagewalk.h |  4 ++--
>  mm/pagewalk.c            | 20 ++++++++++++++++++++
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/pagewalk.h b/include/linux/pagewalk.h
> index 88e18615dd7..d3f84781792 100644
> --- a/include/linux/pagewalk.h
> +++ b/include/linux/pagewalk.h
> @@ -94,8 +94,8 @@ struct mm_walk_ops {
>  };
>  
>  /*
> - * Action for pud_entry / pmd_entry callbacks.
> - * ACTION_SUBTREE is the default
> + * Action for pgd_entry / p4d_entry / pud_entry / pmd_entry callbacks.
> + * ACTION_SUBTREE is the default.
>   */
>  enum page_walk_action {
>  	/* Descend to next level, splitting huge pages if needed and possible */
> diff --git a/mm/pagewalk.c b/mm/pagewalk.c
> index 4e7bcd975c5..4268e08eabb 100644
> --- a/mm/pagewalk.c
> +++ b/mm/pagewalk.c
> @@ -261,6 +261,7 @@ static int walk_p4d_range(pgd_t *pgd, unsigned long addr, unsigned long end,
>  
>  	p4d = p4d_offset(pgd, addr);
>  	do {
> +again:
>  		next = p4d_addr_end(addr, end);
>  		if (p4d_none_or_clear_bad(p4d)) {
>  			if (has_install)
> @@ -272,11 +273,20 @@ static int walk_p4d_range(pgd_t *pgd, unsigned long addr, unsigned long end,
>  			if (!has_install)
>  				continue;
>  		}
> +
> +		walk->action = ACTION_SUBTREE;
> +
>  		if (ops->p4d_entry) {
>  			err = ops->p4d_entry(p4d, addr, next, walk);
>  			if (err)
>  				break;
>  		}
> +
> +		if (walk->action == ACTION_AGAIN)
> +			goto again;
> +		if (walk->action == ACTION_CONTINUE)
> +			continue;
> +
>  		if (has_handler || has_install)
>  			err = walk_pud_range(p4d, addr, next, walk);
>  		if (err)
> @@ -302,6 +312,7 @@ static int walk_pgd_range(unsigned long addr, unsigned long end,
>  	else
>  		pgd = pgd_offset(walk->mm, addr);
>  	do {
> +again:
>  		next = pgd_addr_end(addr, end);
>  		if (pgd_none_or_clear_bad(pgd)) {
>  			if (has_install)
> @@ -313,11 +324,20 @@ static int walk_pgd_range(unsigned long addr, unsigned long end,
>  			if (!has_install)
>  				continue;
>  		}
> +
> +		walk->action = ACTION_SUBTREE;
> +
>  		if (ops->pgd_entry) {
>  			err = ops->pgd_entry(pgd, addr, next, walk);
>  			if (err)
>  				break;
>  		}
> +
> +		if (walk->action == ACTION_AGAIN)
> +			goto again;
> +		if (walk->action == ACTION_CONTINUE)
> +			continue;
> +
>  		if (has_handler || has_install)
>  			err = walk_p4d_range(pgd, addr, next, walk);
>  		if (err)
> 
> base-commit: e39f5a33eec1a4ea03358d82e861d6bb0a426b17

Re: [PATCH] mm/pagewalk: honor action in pgd_entry and p4d_entry

Posted by David Hildenbrand (Arm) 2 hours ago

On 4/14/26 09:57, Vlastimil Babka (SUSE) wrote:
> On 4/14/26 07:18, Cao Ruichuang wrote:
>> walk_pud_range() and walk_pmd_range() reset walk->action to ACTION_SUBTREE and honor ACTION_CONTINUE/ACTION_AGAIN after invoking their callbacks, but walk_pgd_range() and walk_p4d_range() do not.
>>
>> That leaves the top levels with inconsistent callback semantics. In particular, ptdump sets ACTION_CONTINUE from pgd_entry() and p4d_entry() for leaf entries, but the generic walker still descends into lower levels instead of skipping the subtree.
>>
>> Initialize walk->action before calling pgd_entry() and p4d_entry(), and handle ACTION_CONTINUE and ACTION_AGAIN afterwards just like the lower page-table levels do. Also update the action comment to reflect that it applies to pgd_entry() and p4d_entry() as well.
> 
> Your LLM has broken line wrapping. But it's ok, we're not interested in
> patches produced by LLM reviewing code, with no human oversight and
> understanding. NAK.

Agreed. The educated reader would know that we never end up with p4d/pgd
leaves in non-hugetlb VMAs.

The ptdump code is overly cautious ;)

-- 
Cheers,

David