diff mbox series

[03/13] mm: Provide generic pmd_thp_or_huge()

Message ID 20231219075538.414708-4-peterx@redhat.com
State New
Headers
Series mm/gup: Unify hugetlb, part 2 |

Commit Message

Peter Xu Dec. 19, 2023, 7:55 a.m. UTC 
  From: Peter Xu <peterx@redhat.com>

ARM defines pmd_thp_or_huge(), detecting either a THP or a huge PMD.  It
can be a helpful helper if we want to merge more THP and hugetlb code
paths.  Make it a generic default implementation, only exist when
CONFIG_MMU.  Arch can overwrite it by defining its own version.

For example, ARM's pgtable-2level.h defines it to always return false.

Keep the macro declared with all config, it should be optimized to a false
anyway if !THP && !HUGETLB.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 include/linux/pgtable.h | 4 ++++
 mm/gup.c                | 3 +--
 2 files changed, 5 insertions(+), 2 deletions(-)

Comments

Muchun Song Dec. 25, 2023, 6:29 a.m. UTC | #1 
On 2023/12/19 15:55, peterx@redhat.com wrote:
> From: Peter Xu <peterx@redhat.com>
>
> ARM defines pmd_thp_or_huge(), detecting either a THP or a huge PMD.  It
> can be a helpful helper if we want to merge more THP and hugetlb code
> paths.  Make it a generic default implementation, only exist when
> CONFIG_MMU.  Arch can overwrite it by defining its own version.
>
> For example, ARM's pgtable-2level.h defines it to always return false.
>
> Keep the macro declared with all config, it should be optimized to a false
> anyway if !THP && !HUGETLB.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   include/linux/pgtable.h | 4 ++++
>   mm/gup.c                | 3 +--
>   2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> index af7639c3b0a3..6f2fa1977b8a 100644
> --- a/include/linux/pgtable.h
> +++ b/include/linux/pgtable.h
> @@ -1355,6 +1355,10 @@ static inline int pmd_write(pmd_t pmd)
>   #endif /* pmd_write */
>   #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>   
> +#ifndef pmd_thp_or_huge

I think it may be the time to rename to pmd_thp_or_hugetlb,
the "huge" is really confusing. thp is not huge? Actually,
it is huge. It is better to make it more specific from now on, like
"hugetlb".

BTW, please cc me via the new email (muchun.song@linux.dev) next edition.

Thanks.

> +#define pmd_thp_or_huge(pmd)	(pmd_huge(pmd) || pmd_trans_huge(pmd))
> +#endif
> +
>   #ifndef pud_write
>   static inline int pud_write(pud_t pud)
>   {
> diff --git a/mm/gup.c b/mm/gup.c
> index 0a5f0e91bfec..efc9847e58fb 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -3004,8 +3004,7 @@ static int gup_pmd_range(pud_t *pudp, pud_t pud, unsigned long addr, unsigned lo
>   		if (!pmd_present(pmd))
>   			return 0;
>   
> -		if (unlikely(pmd_trans_huge(pmd) || pmd_huge(pmd) ||
> -			     pmd_devmap(pmd))) {
> +		if (unlikely(pmd_thp_or_huge(pmd) || pmd_devmap(pmd))) {
>   			/* See gup_pte_range() */
>   			if (pmd_protnone(pmd))
>   				return 0;
Peter Xu Jan. 2, 2024, 5:37 a.m. UTC | #2 
On Mon, Dec 25, 2023 at 02:29:53PM +0800, Muchun Song wrote:
> > @@ -1355,6 +1355,10 @@ static inline int pmd_write(pmd_t pmd)
> >   #endif /* pmd_write */
> >   #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
> > +#ifndef pmd_thp_or_huge
> 
> I think it may be the time to rename to pmd_thp_or_hugetlb,
> the "huge" is really confusing. thp is not huge? Actually,
> it is huge. It is better to make it more specific from now on, like
> "hugetlb".

The rename will need to touch ARM code, which I wanted to avoid, see:

arch/arm64/include/asm/pgtable.h:#define pmd_thp_or_huge(pmd)   (pmd_huge(pmd) || pmd_trans_huge(pmd))

So far this series only touches generic code.  Would you mind I keep this
patch as-is, and leave renaming to later?

> 
> BTW, please cc me via the new email (muchun.song@linux.dev) next edition.

Sure.  Thanks for taking a look.
Muchun Song Jan. 2, 2024, 6:30 a.m. UTC | #3 
> On Jan 2, 2024, at 13:37, Peter Xu <peterx@redhat.com> wrote:
> 
> On Mon, Dec 25, 2023 at 02:29:53PM +0800, Muchun Song wrote:
>>> @@ -1355,6 +1355,10 @@ static inline int pmd_write(pmd_t pmd)
>>>  #endif /* pmd_write */
>>>  #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
>>> +#ifndef pmd_thp_or_huge
>> 
>> I think it may be the time to rename to pmd_thp_or_hugetlb,
>> the "huge" is really confusing. thp is not huge? Actually,
>> it is huge. It is better to make it more specific from now on, like
>> "hugetlb".
> 
> The rename will need to touch ARM code, which I wanted to avoid, see:

I see.

> 
> arch/arm64/include/asm/pgtable.h:#define pmd_thp_or_huge(pmd)   (pmd_huge(pmd) || pmd_trans_huge(pmd))
> 
> So far this series only touches generic code.  Would you mind I keep this
> patch as-is, and leave renaming to later?

OK.

THanks.

> 
>> 
>> BTW, please cc me via the new email (muchun.song@linux.dev) next edition.
> 
> Sure.  Thanks for taking a look.
> 
> -- 
> Peter Xu
>
diff mbox series

Patch

diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
index af7639c3b0a3..6f2fa1977b8a 100644
--- a/include/linux/pgtable.h
+++ b/include/linux/pgtable.h
@@ -1355,6 +1355,10 @@  static inline int pmd_write(pmd_t pmd)
 #endif /* pmd_write */
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 
+#ifndef pmd_thp_or_huge
+#define pmd_thp_or_huge(pmd)	(pmd_huge(pmd) || pmd_trans_huge(pmd))
+#endif
+
 #ifndef pud_write
 static inline int pud_write(pud_t pud)
 {
diff --git a/mm/gup.c b/mm/gup.c
index 0a5f0e91bfec..efc9847e58fb 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -3004,8 +3004,7 @@  static int gup_pmd_range(pud_t *pudp, pud_t pud, unsigned long addr, unsigned lo
 		if (!pmd_present(pmd))
 			return 0;
 
-		if (unlikely(pmd_trans_huge(pmd) || pmd_huge(pmd) ||
-			     pmd_devmap(pmd))) {
+		if (unlikely(pmd_thp_or_huge(pmd) || pmd_devmap(pmd))) {
 			/* See gup_pte_range() */
 			if (pmd_protnone(pmd))
 				return 0;