Subject: [PATCH v7 01/14] mm: rename is_pinnable_pages to is_pinnable_longterm_pages

is_pinnable_page() and folio_is_pinnable() were renamed to
is_longterm_pinnable_page() and folio_is_longterm_pinnable()
respectively. These functions are used in the FOLL_LONGTERM flag
context.

Signed-off-by: Alex Sierra <[email protected]>
---
include/linux/memremap.h | 24 ++++++++++++++++++++++++
include/linux/mm.h | 24 ------------------------
mm/gup.c | 4 ++--
mm/gup_test.c | 4 ++--
mm/hugetlb.c | 2 +-
5 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/include/linux/memremap.h b/include/linux/memremap.h
index 8af304f6b504..c272bd0af3c1 100644
--- a/include/linux/memremap.h
+++ b/include/linux/memremap.h
@@ -150,6 +150,30 @@ static inline bool is_pci_p2pdma_page(const struct page *page)
page->pgmap->type == MEMORY_DEVICE_PCI_P2PDMA;
}

+/* MIGRATE_CMA and ZONE_MOVABLE do not allow pin pages */
+#ifdef CONFIG_MIGRATION
+static inline bool is_longterm_pinnable_page(struct page *page)
+{
+#ifdef CONFIG_CMA
+ int mt = get_pageblock_migratetype(page);
+
+ if (mt == MIGRATE_CMA || mt == MIGRATE_ISOLATE)
+ return false;
+#endif
+ return !(is_zone_movable_page(page) ||
+ is_zero_pfn(page_to_pfn(page)));
+}
+#else
+static inline bool is_longterm_pinnable_page(struct page *page)
+{
+ return true;
+}
+#endif
+static inline bool folio_is_longterm_pinnable(struct folio *folio)
+{
+ return is_longterm_pinnable_page(&folio->page);
+}
+
#ifdef CONFIG_ZONE_DEVICE
void *memremap_pages(struct dev_pagemap *pgmap, int nid);
void memunmap_pages(struct dev_pagemap *pgmap);
diff --git a/include/linux/mm.h b/include/linux/mm.h
index cf3d0d673f6b..bc0f201a4cff 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1590,30 +1590,6 @@ static inline bool page_needs_cow_for_dma(struct vm_area_struct *vma,
return page_maybe_dma_pinned(page);
}

-/* MIGRATE_CMA and ZONE_MOVABLE do not allow pin pages */
-#ifdef CONFIG_MIGRATION
-static inline bool is_pinnable_page(struct page *page)
-{
-#ifdef CONFIG_CMA
- int mt = get_pageblock_migratetype(page);
-
- if (mt == MIGRATE_CMA || mt == MIGRATE_ISOLATE)
- return false;
-#endif
- return !is_zone_movable_page(page) || is_zero_pfn(page_to_pfn(page));
-}
-#else
-static inline bool is_pinnable_page(struct page *page)
-{
- return true;
-}
-#endif
-
-static inline bool folio_is_pinnable(struct folio *folio)
-{
- return is_pinnable_page(&folio->page);
-}
-
static inline void set_page_zone(struct page *page, enum zone_type zone)
{
page->flags &= ~(ZONES_MASK << ZONES_PGSHIFT);
diff --git a/mm/gup.c b/mm/gup.c
index 551264407624..b65fe8bf5af4 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -133,7 +133,7 @@ struct folio *try_grab_folio(struct page *page, int refs, unsigned int flags)
* path.
*/
if (unlikely((flags & FOLL_LONGTERM) &&
- !is_pinnable_page(page)))
+ !is_longterm_pinnable_page(page)))
return NULL;

/*
@@ -1891,7 +1891,7 @@ static long check_and_migrate_movable_pages(unsigned long nr_pages,
continue;
prev_folio = folio;

- if (folio_is_pinnable(folio))
+ if (folio_is_longterm_pinnable(folio))
continue;

/*
diff --git a/mm/gup_test.c b/mm/gup_test.c
index d974dec19e1c..9d705ba6737e 100644
--- a/mm/gup_test.c
+++ b/mm/gup_test.c
@@ -1,5 +1,5 @@
#include <linux/kernel.h>
-#include <linux/mm.h>
+#include <linux/memremap.h>
#include <linux/slab.h>
#include <linux/uaccess.h>
#include <linux/ktime.h>
@@ -53,7 +53,7 @@ static void verify_dma_pinned(unsigned int cmd, struct page **pages,
dump_page(page, "gup_test failure");
break;
} else if (cmd == PIN_LONGTERM_BENCHMARK &&
- WARN(!is_pinnable_page(page),
+ WARN(!is_longterm_pinnable_page(page),
"pages[%lu] is NOT pinnable but pinned\n",
i)) {
dump_page(page, "gup_test failure");
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index a57e1be41401..368fd33787b0 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1135,7 +1135,7 @@ static struct page *dequeue_huge_page_node_exact(struct hstate *h, int nid)

lockdep_assert_held(&hugetlb_lock);
list_for_each_entry(page, &h->hugepage_freelists[nid], lru) {
- if (pin && !is_pinnable_page(page))
+ if (pin && !is_longterm_pinnable_page(page))
continue;

if (PageHWPoison(page))
--
2.32.0


2022-06-29 07:38:00

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH v7 01/14] mm: rename is_pinnable_pages to is_pinnable_longterm_pages

On 29.06.22 05:54, Alex Sierra wrote:
> is_pinnable_page() and folio_is_pinnable() were renamed to
> is_longterm_pinnable_page() and folio_is_longterm_pinnable()
> respectively. These functions are used in the FOLL_LONGTERM flag
> context.

Subject talks about "*_pages"


Can you elaborate why the move from mm.h to memremap.h is justified?

I'd have called it "is_longterm_pinnable_page", but I am not a native
speaker, so no strong opinion :)


--
Thanks,

David / dhildenb

2022-06-29 22:14:21

by Felix Kuehling

[permalink] [raw]
Subject: Re: [PATCH v7 01/14] mm: rename is_pinnable_pages to is_pinnable_longterm_pages

On 2022-06-29 03:33, David Hildenbrand wrote:
> On 29.06.22 05:54, Alex Sierra wrote:
>> is_pinnable_page() and folio_is_pinnable() were renamed to
>> is_longterm_pinnable_page() and folio_is_longterm_pinnable()
>> respectively. These functions are used in the FOLL_LONGTERM flag
>> context.
> Subject talks about "*_pages"
>
>
> Can you elaborate why the move from mm.h to memremap.h is justified?

Patch 2 adds is_device_coherent_page in memremap.h and updates
is_longterm_pinnable_page to call is_device_coherent_page. memremap.h
cannot include mm.h because it is itself included by mm.h. So the choice
was to move is_longterm_pinnable_page to memremap.h, or move
is_device_coherent_page and all its dependencies to mm.h. The latter
would have been a bigger change.


>
> I'd have called it "is_longterm_pinnable_page", but I am not a native
> speaker, so no strong opinion :)

I think only the patch title has the name backwards. The code uses
is_longterm_pinnable_page.

Regards,
  Felix


>
>

2022-06-29 22:16:16

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH v7 01/14] mm: rename is_pinnable_pages to is_pinnable_longterm_pages

On 30.06.22 00:08, Felix Kuehling wrote:
> On 2022-06-29 03:33, David Hildenbrand wrote:
>> On 29.06.22 05:54, Alex Sierra wrote:
>>> is_pinnable_page() and folio_is_pinnable() were renamed to
>>> is_longterm_pinnable_page() and folio_is_longterm_pinnable()
>>> respectively. These functions are used in the FOLL_LONGTERM flag
>>> context.
>> Subject talks about "*_pages"
>>
>>
>> Can you elaborate why the move from mm.h to memremap.h is justified?
>
> Patch 2 adds is_device_coherent_page in memremap.h and updates
> is_longterm_pinnable_page to call is_device_coherent_page. memremap.h
> cannot include mm.h because it is itself included by mm.h. So the choice
> was to move is_longterm_pinnable_page to memremap.h, or move
> is_device_coherent_page and all its dependencies to mm.h. The latter
> would have been a bigger change.

I really don't think something mm generic that compiles without
ZONE_DEVICE belongs into memremap.h. Please find a cleaner way to get
this done.

--
Thanks,

David / dhildenb

2022-06-29 23:28:26

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v7 01/14] mm: rename is_pinnable_pages to is_pinnable_longterm_pages

On Wed, 29 Jun 2022 18:08:40 -0400 Felix Kuehling <[email protected]> wrote:

> >
> > I'd have called it "is_longterm_pinnable_page", but I am not a native
> > speaker, so no strong opinion :)
>
> I think only the patch title has the name backwards. The code uses
> is_longterm_pinnable_page.

Patch title was quite buggy ;) I made it "mm: rename is_pinnable_page()
to is_longterm_pinnable_page()" in my copy.


2022-07-02 04:27:31

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v7 01/14] mm: rename is_pinnable_pages to is_pinnable_longterm_pages

On Thu, 30 Jun 2022 00:15:06 +0200 David Hildenbrand <[email protected]> wrote:

> On 30.06.22 00:08, Felix Kuehling wrote:
> > On 2022-06-29 03:33, David Hildenbrand wrote:
> >> On 29.06.22 05:54, Alex Sierra wrote:
> >>> is_pinnable_page() and folio_is_pinnable() were renamed to
> >>> is_longterm_pinnable_page() and folio_is_longterm_pinnable()
> >>> respectively. These functions are used in the FOLL_LONGTERM flag
> >>> context.
> >> Subject talks about "*_pages"
> >>
> >>
> >> Can you elaborate why the move from mm.h to memremap.h is justified?
> >
> > Patch 2 adds is_device_coherent_page in memremap.h and updates
> > is_longterm_pinnable_page to call is_device_coherent_page. memremap.h
> > cannot include mm.h because it is itself included by mm.h. So the choice
> > was to move is_longterm_pinnable_page to memremap.h, or move
> > is_device_coherent_page and all its dependencies to mm.h. The latter
> > would have been a bigger change.
>
> I really don't think something mm generic that compiles without
> ZONE_DEVICE belongs into memremap.h. Please find a cleaner way to get
> this done.

(without having bothered to look at the code...)

The solution is always to create a new purpose-specific header file.