Re: [PATCH 1/5] mm: use inline helper functions instead of ugly macros

["David Hildenbrand (Arm)" <david@kernel.org>]

On 2/24/26 02:56, Baolin Wang wrote:
> People have already complained that these *_clear_young_notify() related
> macros are very ugly, so let's use inline helpers to make them more readable.

Could it have been me :)

> 
> In addition, I cannot implement these inline helper functions in the

"We cannot implement ..."

> mmu_notifier.h file, because some arch-specific files will include the
> mmu_notifier.h, which introduces header compilation dependencies and causes
> build errors (e.g., arch/arm64/include/asm/tlbflush.h). Moreover, since
> these functions are only used in the mm, implementing these inline helpers
> in the mm/internal.h header seems reasonable.

Agreed. if it compiles, all good.

> 
> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> ---
>  include/linux/mmu_notifier.h | 54 ------------------------------------
>  mm/internal.h                | 53 +++++++++++++++++++++++++++++++++++
>  2 files changed, 53 insertions(+), 54 deletions(-)
> 
> diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
> index 07a2bbaf86e9..93894b90c8c1 100644
> --- a/include/linux/mmu_notifier.h
> +++ b/include/linux/mmu_notifier.h
> @@ -515,55 +515,6 @@ static inline void mmu_notifier_range_init_owner(
>  	range->owner = owner;
>  }
>  
> -#define clear_flush_young_ptes_notify(__vma, __address, __ptep, __nr)	\
> -({									\
> -	int __young;							\
> -	struct vm_area_struct *___vma = __vma;				\
> -	unsigned long ___address = __address;				\
> -	unsigned int ___nr = __nr;					\
> -	__young = clear_flush_young_ptes(___vma, ___address, __ptep, ___nr);	\
> -	__young |= mmu_notifier_clear_flush_young(___vma->vm_mm,	\
> -						  ___address,		\
> -						  ___address +		\
> -						  ___nr * PAGE_SIZE);	\
> -	__young;							\
> -})
> -
> -#define pmdp_clear_flush_young_notify(__vma, __address, __pmdp)		\
> -({									\
> -	int __young;							\
> -	struct vm_area_struct *___vma = __vma;				\
> -	unsigned long ___address = __address;				\
> -	__young = pmdp_clear_flush_young(___vma, ___address, __pmdp);	\
> -	__young |= mmu_notifier_clear_flush_young(___vma->vm_mm,	\
> -						  ___address,		\
> -						  ___address +		\
> -							PMD_SIZE);	\
> -	__young;							\
> -})
> -
> -#define ptep_clear_young_notify(__vma, __address, __ptep)		\
> -({									\
> -	int __young;							\
> -	struct vm_area_struct *___vma = __vma;				\
> -	unsigned long ___address = __address;				\
> -	__young = ptep_test_and_clear_young(___vma, ___address, __ptep);\
> -	__young |= mmu_notifier_clear_young(___vma->vm_mm, ___address,	\
> -					    ___address + PAGE_SIZE);	\
> -	__young;							\
> -})
> -
> -#define pmdp_clear_young_notify(__vma, __address, __pmdp)		\
> -({									\
> -	int __young;							\
> -	struct vm_area_struct *___vma = __vma;				\
> -	unsigned long ___address = __address;				\
> -	__young = pmdp_test_and_clear_young(___vma, ___address, __pmdp);\
> -	__young |= mmu_notifier_clear_young(___vma->vm_mm, ___address,	\
> -					    ___address + PMD_SIZE);	\
> -	__young;							\
> -})
> -
>  #else /* CONFIG_MMU_NOTIFIER */
>  
>  struct mmu_notifier_range {
> @@ -651,11 +602,6 @@ static inline void mmu_notifier_subscriptions_destroy(struct mm_struct *mm)
>  
>  #define mmu_notifier_range_update_to_read_only(r) false
>  
> -#define clear_flush_young_ptes_notify clear_flush_young_ptes
> -#define pmdp_clear_flush_young_notify pmdp_clear_flush_young
> -#define ptep_clear_young_notify ptep_test_and_clear_young
> -#define pmdp_clear_young_notify pmdp_test_and_clear_young
> -
>  static inline void mmu_notifier_synchronize(void)
>  {
>  }
> diff --git a/mm/internal.h b/mm/internal.h
> index e0ef192b0be3..1ba175b8d4f1 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -11,6 +11,7 @@
>  #include <linux/khugepaged.h>
>  #include <linux/mm.h>
>  #include <linux/mm_inline.h>
> +#include <linux/mmu_notifier.h>
>  #include <linux/pagemap.h>
>  #include <linux/pagewalk.h>
>  #include <linux/rmap.h>
> @@ -1789,4 +1790,56 @@ static inline int io_remap_pfn_range_complete(struct vm_area_struct *vma,
>  	return remap_pfn_range_complete(vma, addr, pfn, size, prot);
>  }
>  
> +#ifdef CONFIG_MMU_NOTIFIER
> +static inline int clear_flush_young_ptes_notify(struct vm_area_struct *vma,
> +						unsigned long addr, pte_t *ptep,
> +						unsigned int nr)

Two tabs indent on the second line makes this fit into two lines.

Same for the others.

With that

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

> +{
> +	int young;
> +
> +	young = clear_flush_young_ptes(vma, addr, ptep, nr);
> +	young |= mmu_notifier_clear_flush_young(vma->vm_mm, addr,
> +						addr + nr * PAGE_SIZE);
> +	return young;
> +}
> +
> +static inline int pmdp_clear_flush_young_notify(struct vm_area_struct *vma,
> +						unsigned long addr, pmd_t *pmdp)
> +{
> +	int young;
> +
> +	young = pmdp_clear_flush_young(vma, addr, pmdp);
> +	young |= mmu_notifier_clear_flush_young(vma->vm_mm, addr, addr + PMD_SIZE);
> +	return young;
> +}
> +
> +static inline int ptep_clear_young_notify(struct vm_area_struct *vma,
> +					  unsigned long addr, pte_t *ptep)
> +{
> +	int young;
> +
> +	young = ptep_test_and_clear_young(vma, addr, ptep);
> +	young |= mmu_notifier_clear_young(vma->vm_mm, addr, addr + PAGE_SIZE);
> +	return young;
> +}
> +
> +static inline int pmdp_clear_young_notify(struct vm_area_struct *vma,
> +					  unsigned long addr, pmd_t *pmdp)
> +{
> +	int young;
> +
> +	young = pmdp_test_and_clear_young(vma, addr, pmdp);
> +	young |= mmu_notifier_clear_young(vma->vm_mm, addr, addr + PMD_SIZE);
> +	return young;
> +}

Wonder why someone thought it would be a good idea to not include the
"test_and_", to make the function names inconsistent.

-- 
Cheers,

David