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

[Vlastimil Babka (SUSE)]

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