2020-04-20 08:01:55

by Joonsoo Kim

[permalink] [raw]
Subject: [PATCH 00/10] change the implemenation of the PageHighMem()

From: Joonsoo Kim <[email protected]>

Hello,

This patchset separates two use cases of PageHighMem() by introducing
PageHighMemZone() macro. And, it changes the implementation of
PageHighMem() to reflect the actual meaning of this macro. This patchset
is a preparation step for the patchset,
"mm/cma: manage the memory of the CMA area by using the ZONE_MOVABLE" [1].

PageHighMem() is used for two different cases. One is to check if there
is a direct mapping for this page or not. The other is to check the
zone of this page, that is, weather it is the highmem type zone or not.

Until now, both the cases are the perfectly same thing. So, implementation
of the PageHighMem() uses the one case that checks if the zone of the page
is the highmem type zone or not.

"#define PageHighMem(__p) is_highmem_idx(page_zonenum(__p))"

ZONE_MOVABLE is special. It is considered as normal type zone on
!CONFIG_HIGHMEM, but, it is considered as highmem type zone
on CONFIG_HIGHMEM. Let's focus on later case. In later case, all pages
on the ZONE_MOVABLE has no direct mapping until now.

However, following patchset
"mm/cma: manage the memory of the CMA area by using the ZONE_MOVABLE"
, which is once merged and reverted, will be tried again and will break
this assumption that all pages on the ZONE_MOVABLE has no direct mapping.
Hence, the ZONE_MOVABLE which is considered as highmem type zone could
have the both types of pages, direct mapped and not. Since
the ZONE_MOVABLE could have both type of pages, __GFP_HIGHMEM is still
required to allocate the memory from it. And, we conservatively need to
consider the ZONE_MOVABLE as highmem type zone.

Even in this situation, PageHighMem() for the pages on the ZONE_MOVABLE
when it is called for checking the direct mapping should return correct
result. Current implementation of PageHighMem() just returns TRUE
if the zone of the page is on a highmem type zone. So, it could be wrong
if the page on the MOVABLE_ZONE is actually direct mapped.

To solve this potential problem, this patch introduces a new
PageHighMemZone() macro. In following patches, two use cases of
PageHighMem() are separated by calling proper macro, PageHighMem() and
PageHighMemZone(). Then, implementation of PageHighMem() will be changed
as just checking if the direct mapping exists or not, regardless of
the zone of the page.

Note that there are some rules to determine the proper macro.

1. If PageHighMem() is called for checking if the direct mapping exists
or not, use PageHighMem().
2. If PageHighMem() is used to predict the previous gfp_flags for
this page, use PageHighMemZone(). The zone of the page is related to
the gfp_flags.
3. If purpose of calling PageHighMem() is to count highmem page and
to interact with the system by using this count, use PageHighMemZone().
This counter is usually used to calculate the available memory for an
kernel allocation and pages on the highmem zone cannot be available
for an kernel allocation.
4. Otherwise, use PageHighMemZone(). It's safe since it's implementation
is just copy of the previous PageHighMem() implementation and won't
be changed.

My final plan is to change the name, PageHighMem() to PageNoDirectMapped()
or something else in order to represent proper meaning.

This patchset is based on next-20200420 and you can find the full patchset on the
following link.

https://github.com/JoonsooKim/linux/tree/page_highmem-cleanup-v1.00-next-20200420

Thanks.

[1]: https://lore.kernel.org/linux-mm/[email protected]
Joonsoo Kim (10):
mm/page-flags: introduce PageHighMemZone()
drm/ttm: separate PageHighMem() and PageHighMemZone() use case
mm/migrate: separate PageHighMem() and PageHighMemZone() use case
kexec: separate PageHighMem() and PageHighMemZone() use case
power: separate PageHighMem() and PageHighMemZone() use case
mm/gup: separate PageHighMem() and PageHighMemZone() use case
mm/hugetlb: separate PageHighMem() and PageHighMemZone() use case
mm: separate PageHighMem() and PageHighMemZone() use case
mm/page_alloc: correct the use of is_highmem_idx()
mm/page-flags: change the implementation of the PageHighMem()

drivers/gpu/drm/ttm/ttm_memory.c | 4 ++--
drivers/gpu/drm/ttm/ttm_page_alloc.c | 2 +-
drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 2 +-
drivers/gpu/drm/ttm/ttm_tt.c | 2 +-
include/linux/migrate.h | 2 +-
include/linux/page-flags.h | 10 +++++++++-
kernel/kexec_core.c | 2 +-
kernel/power/snapshot.c | 12 ++++++------
mm/gup.c | 2 +-
mm/hugetlb.c | 2 +-
mm/memory_hotplug.c | 2 +-
mm/page_alloc.c | 4 ++--
12 files changed, 27 insertions(+), 19 deletions(-)

--
2.7.4


2020-04-20 08:02:03

by Joonsoo Kim

[permalink] [raw]
Subject: [PATCH 01/10] mm/page-flags: introduce PageHighMemZone()

From: Joonsoo Kim <[email protected]>

PageHighMem() is used for two different cases. One is to check if there
is a direct mapping for this page or not. The other is to check the
zone of this page, that is, weather it is the highmem type zone or not.

Until now, both the cases are the perfectly same thing. So, implementation
of the PageHighMem() uses the one case that checks if the zone of the page
is the highmem type zone or not.

"#define PageHighMem(__p) is_highmem_idx(page_zonenum(__p))"

ZONE_MOVABLE is special. It is considered as normal type zone on
!CONFIG_HIGHMEM, but, it is considered as highmem type zone
on CONFIG_HIGHMEM. Let's focus on later case. In later case, all pages
on the ZONE_MOVABLE has no direct mapping until now.

However, following patchset
"mm/cma: manage the memory of the CMA area by using the ZONE_MOVABLE"
, which is once merged and reverted, will be tried again and will break
this assumption that all pages on the ZONE_MOVABLE has no direct mapping.
Hence, the ZONE_MOVABLE which is considered as highmem type zone could
have the both types of pages, direct mapped and not. Since
the ZONE_MOVABLE could have both type of pages, __GFP_HIGHMEM is still
required to allocate the memory from it. And, we conservatively need to
consider the ZONE_MOVABLE as highmem type zone.

Even in this situation, PageHighMem() for the pages on the ZONE_MOVABLE
when it is called for checking the direct mapping should return correct
result. Current implementation of PageHighMem() just returns TRUE
if the zone of the page is on a highmem type zone. So, it could be wrong
if the page on the MOVABLE_ZONE is actually direct mapped.

To solve this potential problem, this patch introduces a new
PageHighMemZone() macro. In following patches, two use cases of
PageHighMem() are separated by calling proper macro, PageHighMem() and
PageHighMemZone(). Then, implementation of PageHighMem() will be changed
as just checking if the direct mapping exists or not, regardless of
the zone of the page.

Note that there are some rules to determine the proper macro.

1. If PageHighMem() is called for checking if the direct mapping exists
or not, use PageHighMem().
2. If PageHighMem() is used to predict the previous gfp_flags for
this page, use PageHighMemZone(). The zone of the page is related to
the gfp_flags.
3. If purpose of calling PageHighMem() is to count highmem page and
to interact with the system by using this count, use PageHighMemZone().
This counter is usually used to calculate the available memory for an
kernel allocation and pages on the highmem zone cannot be available
for an kernel allocation.
4. Otherwise, use PageHighMemZone(). It's safe since it's implementation
is just copy of the previous PageHighMem() implementation and won't
be changed.

My final plan is to change the name, PageHighMem() to PageNoDirectMapped()
or something else in order to represent proper meaning.

Signed-off-by: Joonsoo Kim <[email protected]>
---
include/linux/page-flags.h | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 222f6f7..fca0cce 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -378,10 +378,16 @@ PAGEFLAG(Readahead, reclaim, PF_NO_COMPOUND)
/*
* Must use a macro here due to header dependency issues. page_zone() is not
* available at this point.
+ * PageHighMem() is for checking if the direct mapping exists or not.
+ * PageHighMemZone() is for checking the zone, where the page is belong to,
+ * in order to predict previous gfp_flags or to count something for system
+ * memory management.
*/
#define PageHighMem(__p) is_highmem_idx(page_zonenum(__p))
+#define PageHighMemZone(__p) is_highmem_idx(page_zonenum(__p))
#else
PAGEFLAG_FALSE(HighMem)
+PAGEFLAG_FALSE(HighMemZone)
#endif

#ifdef CONFIG_SWAP
--
2.7.4

2020-04-20 08:02:10

by Joonsoo Kim

[permalink] [raw]
Subject: [PATCH 02/10] drm/ttm: separate PageHighMem() and PageHighMemZone() use case

From: Joonsoo Kim <[email protected]>

Until now, PageHighMem() is used for two different cases. One is to check
if there is a direct mapping for this page or not. The other is to check
the zone of this page, that is, weather it is the highmem type zone or not.

Now, we have separate functions, PageHighMem() and PageHighMemZone() for
each cases. Use appropriate one.

Note that there are some rules to determine the proper macro.

1. If PageHighMem() is called for checking if the direct mapping exists
or not, use PageHighMem().
2. If PageHighMem() is used to predict the previous gfp_flags for
this page, use PageHighMemZone(). The zone of the page is related to
the gfp_flags.
3. If purpose of calling PageHighMem() is to count highmem page and
to interact with the system by using this count, use PageHighMemZone().
This counter is usually used to calculate the available memory for an
kernel allocation and pages on the highmem zone cannot be available
for an kernel allocation.
4. Otherwise, use PageHighMemZone(). It's safe since it's implementation
is just copy of the previous PageHighMem() implementation and won't
be changed.

I apply the rule #4 for this patch.

Signed-off-by: Joonsoo Kim <[email protected]>
---
drivers/gpu/drm/ttm/ttm_memory.c | 4 ++--
drivers/gpu/drm/ttm/ttm_page_alloc.c | 2 +-
drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 2 +-
drivers/gpu/drm/ttm/ttm_tt.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
index acd63b7..d071b71 100644
--- a/drivers/gpu/drm/ttm/ttm_memory.c
+++ b/drivers/gpu/drm/ttm/ttm_memory.c
@@ -641,7 +641,7 @@ int ttm_mem_global_alloc_page(struct ttm_mem_global *glob,
*/

#ifdef CONFIG_HIGHMEM
- if (PageHighMem(page) && glob->zone_highmem != NULL)
+ if (PageHighMemZone(page) && glob->zone_highmem != NULL)
zone = glob->zone_highmem;
#else
if (glob->zone_dma32 && page_to_pfn(page) > 0x00100000UL)
@@ -656,7 +656,7 @@ void ttm_mem_global_free_page(struct ttm_mem_global *glob, struct page *page,
struct ttm_mem_zone *zone = NULL;

#ifdef CONFIG_HIGHMEM
- if (PageHighMem(page) && glob->zone_highmem != NULL)
+ if (PageHighMemZone(page) && glob->zone_highmem != NULL)
zone = glob->zone_highmem;
#else
if (glob->zone_dma32 && page_to_pfn(page) > 0x00100000UL)
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index b40a467..847fabe 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -530,7 +530,7 @@ static int ttm_alloc_new_pages(struct list_head *pages, gfp_t gfp_flags,
/* gfp flags of highmem page should never be dma32 so we
* we should be fine in such case
*/
- if (PageHighMem(p))
+ if (PageHighMemZone(p))
continue;

#endif
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index faefaae..338b2a2 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -747,7 +747,7 @@ static int ttm_dma_pool_alloc_new_pages(struct dma_pool *pool,
/* gfp flags of highmem page should never be dma32 so we
* we should be fine in such case
*/
- if (PageHighMem(p))
+ if (PageHighMemZone(p))
continue;
#endif

diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 2ec448e..6e094dd 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -119,7 +119,7 @@ static int ttm_tt_set_page_caching(struct page *p,
{
int ret = 0;

- if (PageHighMem(p))
+ if (PageHighMemZone(p))
return 0;

if (c_old != tt_cached) {
--
2.7.4

2020-04-20 08:02:21

by Joonsoo Kim

[permalink] [raw]
Subject: [PATCH 03/10] mm/migrate: separate PageHighMem() and PageHighMemZone() use case

From: Joonsoo Kim <[email protected]>

Until now, PageHighMem() is used for two different cases. One is to check
if there is a direct mapping for this page or not. The other is to check
the zone of this page, that is, weather it is the highmem type zone or not.

Now, we have separate functions, PageHighMem() and PageHighMemZone() for
each cases. Use appropriate one.

Note that there are some rules to determine the proper macro.

1. If PageHighMem() is called for checking if the direct mapping exists
or not, use PageHighMem().
2. If PageHighMem() is used to predict the previous gfp_flags for
this page, use PageHighMemZone(). The zone of the page is related to
the gfp_flags.
3. If purpose of calling PageHighMem() is to count highmem page and
to interact with the system by using this count, use PageHighMemZone().
This counter is usually used to calculate the available memory for an
kernel allocation and pages on the highmem zone cannot be available
for an kernel allocation.
4. Otherwise, use PageHighMemZone(). It's safe since it's implementation
is just copy of the previous PageHighMem() implementation and won't
be changed.

I apply the rule #2 for this patch.

Signed-off-by: Joonsoo Kim <[email protected]>
---
include/linux/migrate.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index 3e546cb..9a57766 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -47,7 +47,7 @@ static inline struct page *new_page_nodemask(struct page *page,
order = HPAGE_PMD_ORDER;
}

- if (PageHighMem(page) || (zone_idx(page_zone(page)) == ZONE_MOVABLE))
+ if (PageHighMemZone(page) || zone_idx(page_zone(page)) == ZONE_MOVABLE)
gfp_mask |= __GFP_HIGHMEM;

new_page = __alloc_pages_nodemask(gfp_mask, order,
--
2.7.4

2020-04-20 08:02:25

by Joonsoo Kim

[permalink] [raw]
Subject: [PATCH 04/10] kexec: separate PageHighMem() and PageHighMemZone() use case

From: Joonsoo Kim <[email protected]>

Until now, PageHighMem() is used for two different cases. One is to check
if there is a direct mapping for this page or not. The other is to check
the zone of this page, that is, weather it is the highmem type zone or not.

Now, we have separate functions, PageHighMem() and PageHighMemZone() for
each cases. Use appropriate one.

Note that there are some rules to determine the proper macro.

1. If PageHighMem() is called for checking if the direct mapping exists
or not, use PageHighMem().
2. If PageHighMem() is used to predict the previous gfp_flags for
this page, use PageHighMemZone(). The zone of the page is related to
the gfp_flags.
3. If purpose of calling PageHighMem() is to count highmem page and
to interact with the system by using this count, use PageHighMemZone().
This counter is usually used to calculate the available memory for an
kernel allocation and pages on the highmem zone cannot be available
for an kernel allocation.
4. Otherwise, use PageHighMemZone(). It's safe since it's implementation
is just copy of the previous PageHighMem() implementation and won't
be changed.

I apply the rule #2 for this patch.

Signed-off-by: Joonsoo Kim <[email protected]>
---
kernel/kexec_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index ba1d91e..33097b7 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -766,7 +766,7 @@ static struct page *kimage_alloc_page(struct kimage *image,
* gfp_flags honor the ones passed in.
*/
if (!(gfp_mask & __GFP_HIGHMEM) &&
- PageHighMem(old_page)) {
+ PageHighMemZone(old_page)) {
kimage_free_pages(old_page);
continue;
}
--
2.7.4

2020-04-20 08:02:58

by Joonsoo Kim

[permalink] [raw]
Subject: [PATCH 07/10] mm/hugetlb: separate PageHighMem() and PageHighMemZone() use case

From: Joonsoo Kim <[email protected]>

Until now, PageHighMem() is used for two different cases. One is to check
if there is a direct mapping for this page or not. The other is to check
the zone of this page, that is, weather it is the highmem type zone or not.

Now, we have separate functions, PageHighMem() and PageHighMemZone() for
each cases. Use appropriate one.

Note that there are some rules to determine the proper macro.

1. If PageHighMem() is called for checking if the direct mapping exists
or not, use PageHighMem().
2. If PageHighMem() is used to predict the previous gfp_flags for
this page, use PageHighMemZone(). The zone of the page is related to
the gfp_flags.
3. If purpose of calling PageHighMem() is to count highmem page and
to interact with the system by using this count, use PageHighMemZone().
This counter is usually used to calculate the available memory for an
kernel allocation and pages on the highmem zone cannot be available
for an kernel allocation.
4. Otherwise, use PageHighMemZone(). It's safe since it's implementation
is just copy of the previous PageHighMem() implementation and won't
be changed.

I apply the rule #3 for this patch.

Signed-off-by: Joonsoo Kim <[email protected]>
---
mm/hugetlb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 1c50d23..fc3a701 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2639,7 +2639,7 @@ static void try_to_free_low(struct hstate *h, unsigned long count,
list_for_each_entry_safe(page, next, freel, lru) {
if (count >= h->nr_huge_pages)
return;
- if (PageHighMem(page))
+ if (PageHighMemZone(page))
continue;
list_del(&page->lru);
update_and_free_page(h, page);
--
2.7.4

2020-04-20 08:03:08

by Joonsoo Kim

[permalink] [raw]
Subject: [PATCH 08/10] mm: separate PageHighMem() and PageHighMemZone() use case

From: Joonsoo Kim <[email protected]>

Until now, PageHighMem() is used for two different cases. One is to check
if there is a direct mapping for this page or not. The other is to check
the zone of this page, that is, weather it is the highmem type zone or not.

Now, we have separate functions, PageHighMem() and PageHighMemZone() for
each cases. Use appropriate one.

Note that there are some rules to determine the proper macro.

1. If PageHighMem() is called for checking if the direct mapping exists
or not, use PageHighMem().
2. If PageHighMem() is used to predict the previous gfp_flags for
this page, use PageHighMemZone(). The zone of the page is related to
the gfp_flags.
3. If purpose of calling PageHighMem() is to count highmem page and
to interact with the system by using this count, use PageHighMemZone().
This counter is usually used to calculate the available memory for an
kernel allocation and pages on the highmem zone cannot be available
for an kernel allocation.
4. Otherwise, use PageHighMemZone(). It's safe since it's implementation
is just copy of the previous PageHighMem() implementation and won't
be changed.

I apply the rule #3 for this patch.

Signed-off-by: Joonsoo Kim <[email protected]>
---
mm/memory_hotplug.c | 2 +-
mm/page_alloc.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 8573d2fc..85361e2 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -593,7 +593,7 @@ void generic_online_page(struct page *page, unsigned int order)
__free_pages_core(page, order);
totalram_pages_add(1UL << order);
#ifdef CONFIG_HIGHMEM
- if (PageHighMem(page))
+ if (PageHighMemZone(page))
totalhigh_pages_add(1UL << order);
#endif
}
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index d469384..9997f87 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7470,7 +7470,7 @@ void adjust_managed_page_count(struct page *page, long count)
atomic_long_add(count, &page_zone(page)->managed_pages);
totalram_pages_add(count);
#ifdef CONFIG_HIGHMEM
- if (PageHighMem(page))
+ if (PageHighMemZone(page))
totalhigh_pages_add(count);
#endif
}
--
2.7.4

2020-04-20 08:03:24

by Joonsoo Kim

[permalink] [raw]
Subject: [PATCH 09/10] mm/page_alloc: correct the use of is_highmem_idx()

From: Joonsoo Kim <[email protected]>

What we'd like to check here is whether page has direct mapping or not.
Use PageHighMem() since it is perfectly matched for this purpose.

Signed-off-by: Joonsoo Kim <[email protected]>
---
mm/page_alloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 9997f87..e07148d 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1399,7 +1399,7 @@ static void __meminit __init_single_page(struct page *page, unsigned long pfn,
INIT_LIST_HEAD(&page->lru);
#ifdef WANT_PAGE_VIRTUAL
/* The shift won't overflow because ZONE_NORMAL is below 4G. */
- if (!is_highmem_idx(zone))
+ if (!PageHighMem(page))
set_page_address(page, __va(pfn << PAGE_SHIFT));
#endif
}
--
2.7.4

2020-04-20 08:04:24

by Joonsoo Kim

[permalink] [raw]
Subject: [PATCH 10/10] mm/page-flags: change the implementation of the PageHighMem()

From: Joonsoo Kim <[email protected]>

Until now, PageHighMem() is used for two different cases. One is to check
if there is a direct mapping for this page or not. The other is to check
the zone of this page, that is, weather it is the highmem type zone or not.

Previous patches introduce PageHighMemZone() macro and separates both
cases strictly. So, now, PageHighMem() is used just for checking if
there is a direct mapping for this page or not.

In the following patchset, ZONE_MOVABLE which could be considered as
the highmem type zone in some configuration could have both types of
pages, direct mapped pages and unmapped pages. So, current implementation
of PageHighMem() that checks the zone rather than checks the page in order
to check if a direct mapping exists will be invalid. This patch prepares
that case by implementing PageHighMem() with the max_low_pfn.

Signed-off-by: Joonsoo Kim <[email protected]>
---
include/linux/page-flags.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index fca0cce..7ac5fc8 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -375,6 +375,8 @@ PAGEFLAG(Readahead, reclaim, PF_NO_COMPOUND)
TESTCLEARFLAG(Readahead, reclaim, PF_NO_COMPOUND)

#ifdef CONFIG_HIGHMEM
+extern unsigned long max_low_pfn;
+
/*
* Must use a macro here due to header dependency issues. page_zone() is not
* available at this point.
@@ -383,7 +385,7 @@ PAGEFLAG(Readahead, reclaim, PF_NO_COMPOUND)
* in order to predict previous gfp_flags or to count something for system
* memory management.
*/
-#define PageHighMem(__p) is_highmem_idx(page_zonenum(__p))
+#define PageHighMem(__p) (page_to_pfn(__p) >= max_low_pfn)
#define PageHighMemZone(__p) is_highmem_idx(page_zonenum(__p))
#else
PAGEFLAG_FALSE(HighMem)
--
2.7.4

2020-04-20 08:04:45

by Joonsoo Kim

[permalink] [raw]
Subject: [PATCH 05/10] power: separate PageHighMem() and PageHighMemZone() use case

From: Joonsoo Kim <[email protected]>

Until now, PageHighMem() is used for two different cases. One is to check
if there is a direct mapping for this page or not. The other is to check
the zone of this page, that is, weather it is the highmem type zone or not.

Now, we have separate functions, PageHighMem() and PageHighMemZone() for
each cases. Use appropriate one.

Note that there are some rules to determine the proper macro.

1. If PageHighMem() is called for checking if the direct mapping exists
or not, use PageHighMem().
2. If PageHighMem() is used to predict the previous gfp_flags for
this page, use PageHighMemZone(). The zone of the page is related to
the gfp_flags.
3. If purpose of calling PageHighMem() is to count highmem page and
to interact with the system by using this count, use PageHighMemZone().
This counter is usually used to calculate the available memory for an
kernel allocation and pages on the highmem zone cannot be available
for an kernel allocation.
4. Otherwise, use PageHighMemZone(). It's safe since it's implementation
is just copy of the previous PageHighMem() implementation and won't
be changed.

I apply the rule #3 for this patch.

Signed-off-by: Joonsoo Kim <[email protected]>
---
kernel/power/snapshot.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index 6598001..be759a6 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -1227,7 +1227,7 @@ static struct page *saveable_highmem_page(struct zone *zone, unsigned long pfn)
if (!page || page_zone(page) != zone)
return NULL;

- BUG_ON(!PageHighMem(page));
+ BUG_ON(!PageHighMemZone(page));

if (swsusp_page_is_forbidden(page) || swsusp_page_is_free(page))
return NULL;
@@ -1291,7 +1291,7 @@ static struct page *saveable_page(struct zone *zone, unsigned long pfn)
if (!page || page_zone(page) != zone)
return NULL;

- BUG_ON(PageHighMem(page));
+ BUG_ON(PageHighMemZone(page));

if (swsusp_page_is_forbidden(page) || swsusp_page_is_free(page))
return NULL;
@@ -1529,7 +1529,7 @@ static unsigned long preallocate_image_pages(unsigned long nr_pages, gfp_t mask)
if (!page)
break;
memory_bm_set_bit(&copy_bm, page_to_pfn(page));
- if (PageHighMem(page))
+ if (PageHighMemZone(page))
alloc_highmem++;
else
alloc_normal++;
@@ -1625,7 +1625,7 @@ static unsigned long free_unnecessary_pages(void)
unsigned long pfn = memory_bm_next_pfn(&copy_bm);
struct page *page = pfn_to_page(pfn);

- if (PageHighMem(page)) {
+ if (PageHighMemZone(page)) {
if (!to_free_highmem)
continue;
to_free_highmem--;
@@ -2264,7 +2264,7 @@ static unsigned int count_highmem_image_pages(struct memory_bitmap *bm)
memory_bm_position_reset(bm);
pfn = memory_bm_next_pfn(bm);
while (pfn != BM_END_OF_MAP) {
- if (PageHighMem(pfn_to_page(pfn)))
+ if (PageHighMemZone(pfn_to_page(pfn)))
cnt++;

pfn = memory_bm_next_pfn(bm);
@@ -2541,7 +2541,7 @@ static void *get_buffer(struct memory_bitmap *bm, struct chain_allocator *ca)
return ERR_PTR(-EFAULT);

page = pfn_to_page(pfn);
- if (PageHighMem(page))
+ if (PageHighMemZone(page))
return get_highmem_page_buffer(page, ca);

if (swsusp_page_is_forbidden(page) && swsusp_page_is_free(page))
--
2.7.4

2020-04-20 08:04:45

by Joonsoo Kim

[permalink] [raw]
Subject: [PATCH 06/10] mm/gup: separate PageHighMem() and PageHighMemZone() use case

From: Joonsoo Kim <[email protected]>

Until now, PageHighMem() is used for two different cases. One is to check
if there is a direct mapping for this page or not. The other is to check
the zone of this page, that is, weather it is the highmem type zone or not.

Now, we have separate functions, PageHighMem() and PageHighMemZone() for
each cases. Use appropriate one.

Note that there are some rules to determine the proper macro.

1. If PageHighMem() is called for checking if the direct mapping exists
or not, use PageHighMem().
2. If PageHighMem() is used to predict the previous gfp_flags for
this page, use PageHighMemZone(). The zone of the page is related to
the gfp_flags.
3. If purpose of calling PageHighMem() is to count highmem page and
to interact with the system by using this count, use PageHighMemZone().
This counter is usually used to calculate the available memory for an
kernel allocation and pages on the highmem zone cannot be available
for an kernel allocation.
4. Otherwise, use PageHighMemZone(). It's safe since it's implementation
is just copy of the previous PageHighMem() implementation and won't
be changed.

I apply the rule #2 for this patch.

Signed-off-by: Joonsoo Kim <[email protected]>
---
mm/gup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/gup.c b/mm/gup.c
index 43b0d83..5199de0 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1603,7 +1603,7 @@ static struct page *new_non_cma_page(struct page *page, unsigned long private)
*/
gfp_t gfp_mask = GFP_USER | __GFP_NOWARN;

- if (PageHighMem(page))
+ if (PageHighMemZone(page))
gfp_mask |= __GFP_HIGHMEM;

#ifdef CONFIG_HUGETLB_PAGE
--
2.7.4

2020-04-20 08:43:50

by Christian König

[permalink] [raw]
Subject: Re: [PATCH 02/10] drm/ttm: separate PageHighMem() and PageHighMemZone() use case

Am 20.04.20 um 09:59 schrieb [email protected]:
> From: Joonsoo Kim <[email protected]>
>
> Until now, PageHighMem() is used for two different cases. One is to check
> if there is a direct mapping for this page or not. The other is to check
> the zone of this page, that is, weather it is the highmem type zone or not.
>
> Now, we have separate functions, PageHighMem() and PageHighMemZone() for
> each cases. Use appropriate one.
>
> Note that there are some rules to determine the proper macro.
>
> 1. If PageHighMem() is called for checking if the direct mapping exists
> or not, use PageHighMem().
> 2. If PageHighMem() is used to predict the previous gfp_flags for
> this page, use PageHighMemZone(). The zone of the page is related to
> the gfp_flags.
> 3. If purpose of calling PageHighMem() is to count highmem page and
> to interact with the system by using this count, use PageHighMemZone().
> This counter is usually used to calculate the available memory for an
> kernel allocation and pages on the highmem zone cannot be available
> for an kernel allocation.
> 4. Otherwise, use PageHighMemZone(). It's safe since it's implementation
> is just copy of the previous PageHighMem() implementation and won't
> be changed.
>
> I apply the rule #4 for this patch.
>
> Signed-off-by: Joonsoo Kim <[email protected]>

Reviewed-by: Christian König <[email protected]> for the TTM
changes, but I can't judge if the general approach makes sense or not.

Regards,
Christian.

> ---
> drivers/gpu/drm/ttm/ttm_memory.c | 4 ++--
> drivers/gpu/drm/ttm/ttm_page_alloc.c | 2 +-
> drivers/gpu/drm/ttm/ttm_page_alloc_dma.c | 2 +-
> drivers/gpu/drm/ttm/ttm_tt.c | 2 +-
> 4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/ttm/ttm_memory.c b/drivers/gpu/drm/ttm/ttm_memory.c
> index acd63b7..d071b71 100644
> --- a/drivers/gpu/drm/ttm/ttm_memory.c
> +++ b/drivers/gpu/drm/ttm/ttm_memory.c
> @@ -641,7 +641,7 @@ int ttm_mem_global_alloc_page(struct ttm_mem_global *glob,
> */
>
> #ifdef CONFIG_HIGHMEM
> - if (PageHighMem(page) && glob->zone_highmem != NULL)
> + if (PageHighMemZone(page) && glob->zone_highmem != NULL)
> zone = glob->zone_highmem;
> #else
> if (glob->zone_dma32 && page_to_pfn(page) > 0x00100000UL)
> @@ -656,7 +656,7 @@ void ttm_mem_global_free_page(struct ttm_mem_global *glob, struct page *page,
> struct ttm_mem_zone *zone = NULL;
>
> #ifdef CONFIG_HIGHMEM
> - if (PageHighMem(page) && glob->zone_highmem != NULL)
> + if (PageHighMemZone(page) && glob->zone_highmem != NULL)
> zone = glob->zone_highmem;
> #else
> if (glob->zone_dma32 && page_to_pfn(page) > 0x00100000UL)
> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c b/drivers/gpu/drm/ttm/ttm_page_alloc.c
> index b40a467..847fabe 100644
> --- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
> @@ -530,7 +530,7 @@ static int ttm_alloc_new_pages(struct list_head *pages, gfp_t gfp_flags,
> /* gfp flags of highmem page should never be dma32 so we
> * we should be fine in such case
> */
> - if (PageHighMem(p))
> + if (PageHighMemZone(p))
> continue;
>
> #endif
> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> index faefaae..338b2a2 100644
> --- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
> @@ -747,7 +747,7 @@ static int ttm_dma_pool_alloc_new_pages(struct dma_pool *pool,
> /* gfp flags of highmem page should never be dma32 so we
> * we should be fine in such case
> */
> - if (PageHighMem(p))
> + if (PageHighMemZone(p))
> continue;
> #endif
>
> diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
> index 2ec448e..6e094dd 100644
> --- a/drivers/gpu/drm/ttm/ttm_tt.c
> +++ b/drivers/gpu/drm/ttm/ttm_tt.c
> @@ -119,7 +119,7 @@ static int ttm_tt_set_page_caching(struct page *p,
> {
> int ret = 0;
>
> - if (PageHighMem(p))
> + if (PageHighMemZone(p))
> return 0;
>
> if (c_old != tt_cached) {

2020-04-20 11:23:12

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH 01/10] mm/page-flags: introduce PageHighMemZone()

On Mon, Apr 20, 2020 at 04:59:33PM +0900, [email protected] wrote:
> ZONE_MOVABLE is special. It is considered as normal type zone on
> !CONFIG_HIGHMEM, but, it is considered as highmem type zone
> on CONFIG_HIGHMEM. Let's focus on later case. In later case, all pages
> on the ZONE_MOVABLE has no direct mapping until now.
>
> However, following patchset
> "mm/cma: manage the memory of the CMA area by using the ZONE_MOVABLE"
> , which is once merged and reverted, will be tried again and will break
> this assumption that all pages on the ZONE_MOVABLE has no direct mapping.
> Hence, the ZONE_MOVABLE which is considered as highmem type zone could
> have the both types of pages, direct mapped and not. Since
> the ZONE_MOVABLE could have both type of pages, __GFP_HIGHMEM is still
> required to allocate the memory from it. And, we conservatively need to
> consider the ZONE_MOVABLE as highmem type zone.

I don't understand why CMA allocating pages from ZONE_MOVABLE somehow
gives these pages a direct mapping. Maybe you have a freaky layout in
the architecture that makes no sense and that's what needs to be fixed?

My understanding of the zones is based on x86, and it looks like this
on a 32-bit system with 8GB of memory:

ZONE_DMA 0-16MB
ZONE_NORMAL 16-896MB
ZONE_HIGHMEM 896-xMB
ZONE_MOVABLE x-8192MB

where x is a boot option used to partition the highmem between movable
and unmovable.

Now, why would allocating the CMA from ZONE_NORMAL suddenly make these
pages part of the direct mapping?

2020-04-20 13:23:17

by Vlastimil Babka

[permalink] [raw]
Subject: Re: [PATCH 01/10] mm/page-flags: introduce PageHighMemZone()

On 4/20/20 1:20 PM, Matthew Wilcox wrote:
> On Mon, Apr 20, 2020 at 04:59:33PM +0900, [email protected] wrote:
>> ZONE_MOVABLE is special. It is considered as normal type zone on
>> !CONFIG_HIGHMEM, but, it is considered as highmem type zone
>> on CONFIG_HIGHMEM. Let's focus on later case. In later case, all pages
>> on the ZONE_MOVABLE has no direct mapping until now.
>>
>> However, following patchset
>> "mm/cma: manage the memory of the CMA area by using the ZONE_MOVABLE"
>> , which is once merged and reverted, will be tried again and will break
>> this assumption that all pages on the ZONE_MOVABLE has no direct mapping.
>> Hence, the ZONE_MOVABLE which is considered as highmem type zone could
>> have the both types of pages, direct mapped and not. Since
>> the ZONE_MOVABLE could have both type of pages, __GFP_HIGHMEM is still
>> required to allocate the memory from it. And, we conservatively need to
>> consider the ZONE_MOVABLE as highmem type zone.
>
> I don't understand why CMA allocating pages from ZONE_MOVABLE somehow
> gives these pages a direct mapping. Maybe you have a freaky layout in
> the architecture that makes no sense and that's what needs to be fixed?
>
> My understanding of the zones is based on x86, and it looks like this
> on a 32-bit system with 8GB of memory:
>
> ZONE_DMA 0-16MB
> ZONE_NORMAL 16-896MB
> ZONE_HIGHMEM 896-xMB
> ZONE_MOVABLE x-8192MB
>
> where x is a boot option used to partition the highmem between movable
> and unmovable.
>
> Now, why would allocating the CMA from ZONE_NORMAL suddenly make these
> pages part of the direct mapping?

I assume the scenario is that ZONE_MOVABLE could extend into today's ZONE_NORMAL
range, which is the range covered by direct mapping.
At that point, testing page's zone stops being a reliable test of "does this
page have direct mapping"?

I don't know the exact motivation why that will happen but I can imagine two.
1) some CMA user needs the CMA allocations to be in direct mapping range
2) the amount of CMA memory reservation required is so high it won't fit in
highmem range only.

2020-04-21 06:45:19

by Joonsoo Kim

[permalink] [raw]
Subject: Re: [PATCH 01/10] mm/page-flags: introduce PageHighMemZone()

Hello, Matthew and Vlastimil.

2020년 4월 20일 (월) 오후 8:37, Vlastimil Babka <[email protected]>님이 작성:
>
> On 4/20/20 1:20 PM, Matthew Wilcox wrote:
> > On Mon, Apr 20, 2020 at 04:59:33PM +0900, [email protected] wrote:
> >> ZONE_MOVABLE is special. It is considered as normal type zone on
> >> !CONFIG_HIGHMEM, but, it is considered as highmem type zone
> >> on CONFIG_HIGHMEM. Let's focus on later case. In later case, all pages
> >> on the ZONE_MOVABLE has no direct mapping until now.
> >>
> >> However, following patchset
> >> "mm/cma: manage the memory of the CMA area by using the ZONE_MOVABLE"
> >> , which is once merged and reverted, will be tried again and will break
> >> this assumption that all pages on the ZONE_MOVABLE has no direct mapping.
> >> Hence, the ZONE_MOVABLE which is considered as highmem type zone could
> >> have the both types of pages, direct mapped and not. Since
> >> the ZONE_MOVABLE could have both type of pages, __GFP_HIGHMEM is still
> >> required to allocate the memory from it. And, we conservatively need to
> >> consider the ZONE_MOVABLE as highmem type zone.
> >
> > I don't understand why CMA allocating pages from ZONE_MOVABLE somehow
> > gives these pages a direct mapping. Maybe you have a freaky layout in
> > the architecture that makes no sense and that's what needs to be fixed?
> >
> > My understanding of the zones is based on x86, and it looks like this
> > on a 32-bit system with 8GB of memory:
> >
> > ZONE_DMA 0-16MB
> > ZONE_NORMAL 16-896MB
> > ZONE_HIGHMEM 896-xMB
> > ZONE_MOVABLE x-8192MB
> >
> > where x is a boot option used to partition the highmem between movable
> > and unmovable.
> >
> > Now, why would allocating the CMA from ZONE_NORMAL suddenly make these
> > pages part of the direct mapping?
>
> I assume the scenario is that ZONE_MOVABLE could extend into today's ZONE_NORMAL
> range, which is the range covered by direct mapping.
> At that point, testing page's zone stops being a reliable test of "does this
> page have direct mapping"?

Correct explanation. Thanks, Vlastimil.

This patchset is a preparation for my future patchset "mm/cma: manage the memory
of the CMA area by using the ZONE_MOVABLE" [1] to solve the many CMA problems.

CMA areas can be on the all the memory range, from ZONE_DMA to ZONE_HIGHMEM.
And, in my future patchset [1], all the CMA areas are managed through
the ZONE_MOVABLE
and the range of the ZONE_MOVABLE is extended to cover all the CMA
areas. In this
case, following scenario would be possible.

CMA area 1: 32MB size on the memory range 16MB~48MB (originally on the
ZONE_NORMAL)
CMA area 2: 32MB size on the memory range 896MB~928MB (originally on
the ZONE_HIGHMEM)

With my future patchset [1], ZONE_MOVABLE manages all the pages from
CMA area 1 and 2.
So, ZONE_MOVABLE has both direct mapped page and un-mapped page. Since one zone
has two types of pages, current PageHighMem() implemented by using
zone index could not
work correctly. So, I make this patchset to change the PagHighMem()
implementation.

> I don't know the exact motivation why that will happen but I can imagine two.
> 1) some CMA user needs the CMA allocations to be in direct mapping range
> 2) the amount of CMA memory reservation required is so high it won't fit in
> highmem range only.

The range of CMA area is highly depends on system architecture and
device. Each device
using CMA area would have different limitation for address range and
someone's limitation
could be low memory range.

Thanks.

2020-04-21 06:53:54

by Joonsoo Kim

[permalink] [raw]
Subject: Re: [PATCH 02/10] drm/ttm: separate PageHighMem() and PageHighMemZone() use case

2020년 4월 20일 (월) 오후 5:42, Christian König <[email protected]>님이 작성:
>
> Am 20.04.20 um 09:59 schrieb [email protected]:
> > From: Joonsoo Kim <[email protected]>
> >
> > Until now, PageHighMem() is used for two different cases. One is to check
> > if there is a direct mapping for this page or not. The other is to check
> > the zone of this page, that is, weather it is the highmem type zone or not.
> >
> > Now, we have separate functions, PageHighMem() and PageHighMemZone() for
> > each cases. Use appropriate one.
> >
> > Note that there are some rules to determine the proper macro.
> >
> > 1. If PageHighMem() is called for checking if the direct mapping exists
> > or not, use PageHighMem().
> > 2. If PageHighMem() is used to predict the previous gfp_flags for
> > this page, use PageHighMemZone(). The zone of the page is related to
> > the gfp_flags.
> > 3. If purpose of calling PageHighMem() is to count highmem page and
> > to interact with the system by using this count, use PageHighMemZone().
> > This counter is usually used to calculate the available memory for an
> > kernel allocation and pages on the highmem zone cannot be available
> > for an kernel allocation.
> > 4. Otherwise, use PageHighMemZone(). It's safe since it's implementation
> > is just copy of the previous PageHighMem() implementation and won't
> > be changed.
> >
> > I apply the rule #4 for this patch.
> >
> > Signed-off-by: Joonsoo Kim <[email protected]>
>
> Reviewed-by: Christian König <[email protected]> for the TTM
> changes, but I can't judge if the general approach makes sense or not.

Thanks for review!
General approach needs to get an agreement from more developers.

Thanks.

2020-04-21 09:01:48

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 01/10] mm/page-flags: introduce PageHighMemZone()

I don't think the names are very good. I'd keep PageHighMem for the
existing highmem zone, and add something like PageDirectMapped or
similar for ay page that has a valid direct mapping address. This will
also come in handy if we plan to go ahead with the xpfo work.

2020-04-22 01:06:58

by Roman Gushchin

[permalink] [raw]
Subject: Re: [PATCH 01/10] mm/page-flags: introduce PageHighMemZone()

On Tue, Apr 21, 2020 at 02:00:05AM -0700, Christoph Hellwig wrote:
> I don't think the names are very good. I'd keep PageHighMem for the
> existing highmem zone, and add something like PageDirectMapped or
> similar for ay page that has a valid direct mapping address. This will
> also come in handy if we plan to go ahead with the xpfo work.
>

I agree. It also looks like the only remaining place with PageHighMem()
is using "if (!PageHighMem(page))", so "if (PageDirectlyMapped(page))" would be
even better.

Joonsoo, the series in general looks very good to me. Please, feel free
to add "Acked-by: Roman Gushchin <[email protected]>".

Thanks!

2020-04-22 07:44:32

by Joonsoo Kim

[permalink] [raw]
Subject: Re: [PATCH 01/10] mm/page-flags: introduce PageHighMemZone()

2020년 4월 21일 (화) 오후 6:00, Christoph Hellwig <[email protected]>님이 작성:
>
> I don't think the names are very good. I'd keep PageHighMem for the
> existing highmem zone, and add something like PageDirectMapped or
> similar for ay page that has a valid direct mapping address. This will
> also come in handy if we plan to go ahead with the xpfo work.

For PageHighMem(), as mentioned in patch description, my next plan is
to rename PageHighMem() that checks valid direct mapping to
PageNoDirectMapped() or something else. PageDirectMapped() looks better.
Reason that rename isn't implemented in this patchset is that I'd like to rename
after everything is settle down.

For PageHighMemZone(), I think that it serves correct meaning.

Thanks.

2020-04-22 07:45:55

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 03/10] mm/migrate: separate PageHighMem() and PageHighMemZone() use case

On Mon, Apr 20, 2020 at 04:59:35PM +0900, [email protected] wrote:
> From: Joonsoo Kim <[email protected]>
>
> Until now, PageHighMem() is used for two different cases. One is to check
> if there is a direct mapping for this page or not. The other is to check
> the zone of this page, that is, weather it is the highmem type zone or not.
>
> Now, we have separate functions, PageHighMem() and PageHighMemZone() for
> each cases. Use appropriate one.
>
> Note that there are some rules to determine the proper macro.
>
> 1. If PageHighMem() is called for checking if the direct mapping exists
> or not, use PageHighMem().
> 2. If PageHighMem() is used to predict the previous gfp_flags for
> this page, use PageHighMemZone(). The zone of the page is related to
> the gfp_flags.
> 3. If purpose of calling PageHighMem() is to count highmem page and
> to interact with the system by using this count, use PageHighMemZone().
> This counter is usually used to calculate the available memory for an
> kernel allocation and pages on the highmem zone cannot be available
> for an kernel allocation.
> 4. Otherwise, use PageHighMemZone(). It's safe since it's implementation
> is just copy of the previous PageHighMem() implementation and won't
> be changed.
>
> I apply the rule #2 for this patch.
>
> Signed-off-by: Joonsoo Kim <[email protected]>
> ---
> include/linux/migrate.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/migrate.h b/include/linux/migrate.h
> index 3e546cb..9a57766 100644
> --- a/include/linux/migrate.h
> +++ b/include/linux/migrate.h
> @@ -47,7 +47,7 @@ static inline struct page *new_page_nodemask(struct page *page,
> order = HPAGE_PMD_ORDER;
> }
>
> - if (PageHighMem(page) || (zone_idx(page_zone(page)) == ZONE_MOVABLE))
> + if (PageHighMemZone(page) || zone_idx(page_zone(page)) == ZONE_MOVABLE)
> gfp_mask |= __GFP_HIGHMEM;

I think this would be much cleaner if you open coded PageHighMemZone
here.

E.g.

int zone = page_zone(page);

if (is_highmem_idx(zone) || zone_idx(zone) == ZONE_MOVABLE)

2020-04-22 07:46:12

by Joonsoo Kim

[permalink] [raw]
Subject: Re: [PATCH 01/10] mm/page-flags: introduce PageHighMemZone()

2020년 4월 22일 (수) 오전 10:02, Roman Gushchin <[email protected]>님이 작성:
>
> On Tue, Apr 21, 2020 at 02:00:05AM -0700, Christoph Hellwig wrote:
> > I don't think the names are very good. I'd keep PageHighMem for the
> > existing highmem zone, and add something like PageDirectMapped or
> > similar for ay page that has a valid direct mapping address. This will
> > also come in handy if we plan to go ahead with the xpfo work.
> >
>
> I agree. It also looks like the only remaining place with PageHighMem()
> is using "if (!PageHighMem(page))", so "if (PageDirectlyMapped(page))" would be
> even better.

As mentioned in previous reply to Christoph, I will change the name after
everything is settle down.

> Joonsoo, the series in general looks very good to me. Please, feel free
> to add "Acked-by: Roman Gushchin <[email protected]>".

Thanks for reviewing this!

Thanks.

2020-04-22 07:59:11

by Joonsoo Kim

[permalink] [raw]
Subject: Re: [PATCH 03/10] mm/migrate: separate PageHighMem() and PageHighMemZone() use case

2020년 4월 22일 (수) 오후 4:44, Christoph Hellwig <[email protected]>님이 작성:
>
> On Mon, Apr 20, 2020 at 04:59:35PM +0900, [email protected] wrote:
> > From: Joonsoo Kim <[email protected]>
> >
> > Until now, PageHighMem() is used for two different cases. One is to check
> > if there is a direct mapping for this page or not. The other is to check
> > the zone of this page, that is, weather it is the highmem type zone or not.
> >
> > Now, we have separate functions, PageHighMem() and PageHighMemZone() for
> > each cases. Use appropriate one.
> >
> > Note that there are some rules to determine the proper macro.
> >
> > 1. If PageHighMem() is called for checking if the direct mapping exists
> > or not, use PageHighMem().
> > 2. If PageHighMem() is used to predict the previous gfp_flags for
> > this page, use PageHighMemZone(). The zone of the page is related to
> > the gfp_flags.
> > 3. If purpose of calling PageHighMem() is to count highmem page and
> > to interact with the system by using this count, use PageHighMemZone().
> > This counter is usually used to calculate the available memory for an
> > kernel allocation and pages on the highmem zone cannot be available
> > for an kernel allocation.
> > 4. Otherwise, use PageHighMemZone(). It's safe since it's implementation
> > is just copy of the previous PageHighMem() implementation and won't
> > be changed.
> >
> > I apply the rule #2 for this patch.
> >
> > Signed-off-by: Joonsoo Kim <[email protected]>
> > ---
> > include/linux/migrate.h | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/migrate.h b/include/linux/migrate.h
> > index 3e546cb..9a57766 100644
> > --- a/include/linux/migrate.h
> > +++ b/include/linux/migrate.h
> > @@ -47,7 +47,7 @@ static inline struct page *new_page_nodemask(struct page *page,
> > order = HPAGE_PMD_ORDER;
> > }
> >
> > - if (PageHighMem(page) || (zone_idx(page_zone(page)) == ZONE_MOVABLE))
> > + if (PageHighMemZone(page) || zone_idx(page_zone(page)) == ZONE_MOVABLE)
> > gfp_mask |= __GFP_HIGHMEM;
>
> I think this would be much cleaner if you open coded PageHighMemZone
> here.
>
> E.g.
>
> int zone = page_zone(page);
>
> if (is_highmem_idx(zone) || zone_idx(zone) == ZONE_MOVABLE)

Okay. I will use open code here to make two condition checks similar form.

Thanks.

2021-02-10 13:06:15

by Prakash Gupta

[permalink] [raw]
Subject: Re: [PATCH 01/10] mm/page-flags: introduce PageHighMemZone()


On 4/21/2020 12:13 PM, Joonsoo Kim wrote:
> This patchset is a preparation for my future patchset "mm/cma: manage the memory
> of the CMA area by using the ZONE_MOVABLE" [1] to solve the many CMA problems.
>
> CMA areas can be on the all the memory range, from ZONE_DMA to ZONE_HIGHMEM.
> And, in my future patchset [1], all the CMA areas are managed through
> the ZONE_MOVABLE
> and the range of the ZONE_MOVABLE is extended to cover all the CMA
> areas. In this
> case, following scenario would be possible.
>
> CMA area 1: 32MB size on the memory range 16MB~48MB (originally on the
> ZONE_NORMAL)
> CMA area 2: 32MB size on the memory range 896MB~928MB (originally on
> the ZONE_HIGHMEM)
>
> With my future patchset [1], ZONE_MOVABLE manages all the pages from
> CMA area 1 and 2.
> So, ZONE_MOVABLE has both direct mapped page and un-mapped page. Since one zone
> has two types of pages, current PageHighMem() implemented by using
> zone index could not
> work correctly. So, I make this patchset to change the PagHighMem()
> implementation.

Hello Joonsoo,

Resurrecting this thread. I wanted to check if you are still working on [1].

If yes, would you be posting it soon.

I think these would be useful for high CMA system.


Thanks,

Prakash