This patchset introduces some more helper functions for the folio
conversions, and converts all callers of is_longterm_pinnable_page() to
use folios.
--
v2:
Fix a syntax issue
Move flags check to top of try_grab_folio
Vishal Moola (Oracle) (5):
mmzone: Introduce folio_is_zone_movable()
mmzone: Introduce folio_migratetype()
mm/gup_test.c: Convert verify_dma_pinned() to us folios
mm/gup.c: Reorganize try_get_folio()
mm: Remove is_longterm_pinnable_page() and Reimplement
folio_is_longterm_pinnable()
include/linux/mm.h | 22 +++++------
include/linux/mmzone.h | 8 ++++
mm/gup.c | 86 +++++++++++++++++++++---------------------
mm/gup_test.c | 13 ++++---
4 files changed, 67 insertions(+), 62 deletions(-)
--
2.40.1
folio_is_longterm_pinnable() already exists as a wrapper function. Now
that the whole implementation of is_longterm_pinnable_page() can be
implemented using folios, folio_is_longterm_pinnable() can be made its
own standalone function - and we can remove is_longterm_pinnable_page().
Signed-off-by: Vishal Moola (Oracle) <[email protected]>
Reviewed-by: Matthew Wilcox (Oracle) <[email protected]>
---
include/linux/mm.h | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 27ce77080c79..e2d35e272e07 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1910,39 +1910,35 @@ 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 */
+/* MIGRATE_CMA and ZONE_MOVABLE do not allow pin folios */
#ifdef CONFIG_MIGRATION
-static inline bool is_longterm_pinnable_page(struct page *page)
+static inline bool folio_is_longterm_pinnable(struct folio *folio)
{
#ifdef CONFIG_CMA
- int mt = get_pageblock_migratetype(page);
+ int mt = folio_migratetype(folio);
if (mt == MIGRATE_CMA || mt == MIGRATE_ISOLATE)
return false;
#endif
/* The zero page may always be pinned */
- if (is_zero_pfn(page_to_pfn(page)))
+ if (is_zero_pfn(folio_pfn(folio)))
return true;
/* Coherent device memory must always allow eviction. */
- if (is_device_coherent_page(page))
+ if (folio_is_device_coherent(folio))
return false;
- /* Otherwise, non-movable zone pages can be pinned. */
- return !is_zone_movable_page(page);
+ /* Otherwise, non-movable zone folios can be pinned. */
+ return !folio_is_zone_movable(folio);
+
}
#else
-static inline bool is_longterm_pinnable_page(struct page *page)
+static inline bool folio_is_longterm_pinnable(struct folio *folio)
{
return true;
}
#endif
-static inline bool folio_is_longterm_pinnable(struct folio *folio)
-{
- return is_longterm_pinnable_page(&folio->page);
-}
-
static inline void set_page_zone(struct page *page, enum zone_type zone)
{
page->flags &= ~(ZONES_MASK << ZONES_PGSHIFT);
--
2.40.1
Introduce folio_migratetype() as a folio equivalent for
get_pageblock_migratetype(). This function intends to return the
migratetype the folio is located in, hence the name choice.
Signed-off-by: Vishal Moola (Oracle) <[email protected]>
Reviewed-by: Matthew Wilcox (Oracle) <[email protected]>
---
include/linux/mmzone.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 744bf32e48a8..b58c76e68ac7 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -105,6 +105,9 @@ extern int page_group_by_mobility_disabled;
#define get_pageblock_migratetype(page) \
get_pfnblock_flags_mask(page, page_to_pfn(page), MIGRATETYPE_MASK)
+#define folio_migratetype(folio) \
+ get_pfnblock_flags_mask(&folio->page, folio_pfn(folio), \
+ MIGRATETYPE_MASK)
struct free_area {
struct list_head free_list[MIGRATE_TYPES];
unsigned long nr_free;
--
2.40.1
verify_dma_pinned() checks that pages are dma-pinned. We can convert
this to use folios.
Signed-off-by: Vishal Moola (Oracle) <[email protected]>
Reviewed-by: Matthew Wilcox (Oracle) <[email protected]>
---
mm/gup_test.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/mm/gup_test.c b/mm/gup_test.c
index 8ae7307a1bb6..860b093b4b3e 100644
--- a/mm/gup_test.c
+++ b/mm/gup_test.c
@@ -40,24 +40,25 @@ static void verify_dma_pinned(unsigned int cmd, struct page **pages,
unsigned long nr_pages)
{
unsigned long i;
- struct page *page;
+ struct folio *folio;
switch (cmd) {
case PIN_FAST_BENCHMARK:
case PIN_BASIC_TEST:
case PIN_LONGTERM_BENCHMARK:
for (i = 0; i < nr_pages; i++) {
- page = pages[i];
- if (WARN(!page_maybe_dma_pinned(page),
+ folio = page_folio(pages[i]);
+
+ if (WARN(!folio_maybe_dma_pinned(folio),
"pages[%lu] is NOT dma-pinned\n", i)) {
- dump_page(page, "gup_test failure");
+ dump_page(&folio->page, "gup_test failure");
break;
} else if (cmd == PIN_LONGTERM_BENCHMARK &&
- WARN(!is_longterm_pinnable_page(page),
+ WARN(!folio_is_longterm_pinnable(folio),
"pages[%lu] is NOT pinnable but pinned\n",
i)) {
- dump_page(page, "gup_test failure");
+ dump_page(&folio->page, "gup_test failure");
break;
}
}
--
2.40.1
On Tue, 13 Jun 2023 19:13:09 -0700 "Vishal Moola (Oracle)" <[email protected]> wrote:
> Introduce folio_migratetype() as a folio equivalent for
> get_pageblock_migratetype(). This function intends to return the
> migratetype the folio is located in, hence the name choice.
>
> ...
>
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -105,6 +105,9 @@ extern int page_group_by_mobility_disabled;
> #define get_pageblock_migratetype(page) \
> get_pfnblock_flags_mask(page, page_to_pfn(page), MIGRATETYPE_MASK)
>
> +#define folio_migratetype(folio) \
> + get_pfnblock_flags_mask(&folio->page, folio_pfn(folio), \
> + MIGRATETYPE_MASK)
Theoretically this is risky because it evaluates its argument more than
once. Although folio_migratetype(folio++) seems an unlikely thing to do.
An inlined C function is always preferable. My quick attempt at that
reveals that the header files are All Messed Up As Usual.
On Wed, Jun 14, 2023 at 01:13:05PM -0700, Andrew Morton wrote:
> On Tue, 13 Jun 2023 19:13:09 -0700 "Vishal Moola (Oracle)" <[email protected]> wrote:
>
> > Introduce folio_migratetype() as a folio equivalent for
> > get_pageblock_migratetype(). This function intends to return the
> > migratetype the folio is located in, hence the name choice.
> >
> > ...
> >
> > --- a/include/linux/mmzone.h
> > +++ b/include/linux/mmzone.h
> > @@ -105,6 +105,9 @@ extern int page_group_by_mobility_disabled;
> > #define get_pageblock_migratetype(page) \
> > get_pfnblock_flags_mask(page, page_to_pfn(page), MIGRATETYPE_MASK)
> >
> > +#define folio_migratetype(folio) \
> > + get_pfnblock_flags_mask(&folio->page, folio_pfn(folio), \
> > + MIGRATETYPE_MASK)
>
> Theoretically this is risky because it evaluates its argument more than
> once. Although folio_migratetype(folio++) seems an unlikely thing to do.
folio++ is always an unsafe thing to do. folios are not consecutive
in memory (unless we know they're order-0).
> An inlined C function is always preferable. My quick attempt at that
> reveals that the header files are All Messed Up As Usual.
The page-equivalent of this also evaluates its arguments more than once,
so it doesn't see too risky for now?
On Wed, 14 Jun 2023 21:17:34 +0100 Matthew Wilcox <[email protected]> wrote:
> On Wed, Jun 14, 2023 at 01:13:05PM -0700, Andrew Morton wrote:
> > On Tue, 13 Jun 2023 19:13:09 -0700 "Vishal Moola (Oracle)" <[email protected]> wrote:
> >
> > > Introduce folio_migratetype() as a folio equivalent for
> > > get_pageblock_migratetype(). This function intends to return the
> > > migratetype the folio is located in, hence the name choice.
> > >
> > > ...
> > >
> > > --- a/include/linux/mmzone.h
> > > +++ b/include/linux/mmzone.h
> > > @@ -105,6 +105,9 @@ extern int page_group_by_mobility_disabled;
> > > #define get_pageblock_migratetype(page) \
> > > get_pfnblock_flags_mask(page, page_to_pfn(page), MIGRATETYPE_MASK)
> > >
> > > +#define folio_migratetype(folio) \
> > > + get_pfnblock_flags_mask(&folio->page, folio_pfn(folio), \
> > > + MIGRATETYPE_MASK)
> >
> > Theoretically this is risky because it evaluates its argument more than
> > once. Although folio_migratetype(folio++) seems an unlikely thing to do.
>
> folio++ is always an unsafe thing to do. folios are not consecutive
> in memory (unless we know they're order-0).
OK, folio_migratetype(expensive_function_which_returns_a_folio()) or
folio_migratetype(function_with_side_effects_which_returns_a_folio()).
There are many failure modes.
> > An inlined C function is always preferable. My quick attempt at that
> > reveals that the header files are All Messed Up As Usual.
>
> The page-equivalent of this also evaluates its arguments more than once,
> so it doesn't see too risky for now?
We got lucky. It's just bad practice.
On Wed, 14 Jun 2023 at 03:14, Vishal Moola (Oracle)
<[email protected]> wrote:
>
> folio_is_longterm_pinnable() already exists as a wrapper function. Now
> that the whole implementation of is_longterm_pinnable_page() can be
> implemented using folios, folio_is_longterm_pinnable() can be made its
> own standalone function - and we can remove is_longterm_pinnable_page().
>
> Signed-off-by: Vishal Moola (Oracle) <[email protected]>
> Reviewed-by: Matthew Wilcox (Oracle) <[email protected]>
> ---
> include/linux/mm.h | 22 +++++++++-------------
> 1 file changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 27ce77080c79..e2d35e272e07 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1910,39 +1910,35 @@ 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 */
> +/* MIGRATE_CMA and ZONE_MOVABLE do not allow pin folios */
> #ifdef CONFIG_MIGRATION
> -static inline bool is_longterm_pinnable_page(struct page *page)
> +static inline bool folio_is_longterm_pinnable(struct folio *folio)
> {
> #ifdef CONFIG_CMA
> - int mt = get_pageblock_migratetype(page);
> + int mt = folio_migratetype(folio);
>
> if (mt == MIGRATE_CMA || mt == MIGRATE_ISOLATE)
> return false;
> #endif
> /* The zero page may always be pinned */
> - if (is_zero_pfn(page_to_pfn(page)))
> + if (is_zero_pfn(folio_pfn(folio)))
> return true;
>
> /* Coherent device memory must always allow eviction. */
> - if (is_device_coherent_page(page))
> + if (folio_is_device_coherent(folio))
> return false;
>
> - /* Otherwise, non-movable zone pages can be pinned. */
> - return !is_zone_movable_page(page);
> + /* Otherwise, non-movable zone folios can be pinned. */
> + return !folio_is_zone_movable(folio);
> +
Nit, but you're adding a newline here.
> }
> #else
> -static inline bool is_longterm_pinnable_page(struct page *page)
> +static inline bool folio_is_longterm_pinnable(struct folio *folio)
> {
> return true;
> }
> #endif
>
> -static inline bool folio_is_longterm_pinnable(struct folio *folio)
> -{
> - return is_longterm_pinnable_page(&folio->page);
> -}
> -
> static inline void set_page_zone(struct page *page, enum zone_type zone)
> {
> page->flags &= ~(ZONES_MASK << ZONES_PGSHIFT);
> --
> 2.40.1
>
>
Reviewed-by: Lorenzo Stoakes <[email protected]>