2020-04-29 03:29:59

by Joonsoo Kim

[permalink] [raw]
Subject: [PATCH v2 07/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.

Acked-by: Roman Gushchin <[email protected]>
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 555137b..891c214 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 fc5919e..7fe5115 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7444,7 +7444,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-05-01 12:33:14

by Christoph Hellwig

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

On Wed, Apr 29, 2020 at 12:26:40PM +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 #3 for this patch.
>
> Acked-by: Roman Gushchin <[email protected]>
> 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 555137b..891c214 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 fc5919e..7fe5115 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -7444,7 +7444,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

This function already uses the page_zone structure above, I think
life would be easier of you compare against that, as that makes
the code more obvious.

2020-05-04 03:32:05

by Joonsoo Kim

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

2020년 5월 1일 (금) 오후 9:30, Christoph Hellwig <[email protected]>님이 작성:
>
> On Wed, Apr 29, 2020 at 12:26:40PM +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 #3 for this patch.
> >
> > Acked-by: Roman Gushchin <[email protected]>
> > 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 555137b..891c214 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 fc5919e..7fe5115 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -7444,7 +7444,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
>
> This function already uses the page_zone structure above, I think
> life would be easier of you compare against that, as that makes
> the code more obvious.

If I can kill all the PageHighMemZone() macro, I will use page_zone() above.
However, if it's not possible, I will leave it as it is. It would be
simpler than
your suggestion.

Thanks.