diff mbox series

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

Message ID 1587369582-3882-4-git-send-email-iamjoonsoo.kim@lge.com (mailing list archive)
State New, archived
Headers show
Series change the implemenation of the PageHighMem() | expand

Commit Message

Joonsoo Kim April 20, 2020, 7:59 a.m. UTC
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>

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 <iamjoonsoo.kim@lge.com>
---
 include/linux/migrate.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Christoph Hellwig April 22, 2020, 7:44 a.m. UTC | #1
On Mon, Apr 20, 2020 at 04:59:35PM +0900, js1304@gmail.com wrote:
> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> 
> 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 <iamjoonsoo.kim@lge.com>
> ---
>  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)
Joonsoo Kim April 22, 2020, 7:55 a.m. UTC | #2
2020년 4월 22일 (수) 오후 4:44, Christoph Hellwig <hch@infradead.org>님이 작성:
>
> On Mon, Apr 20, 2020 at 04:59:35PM +0900, js1304@gmail.com wrote:
> > From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> >
> > 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 <iamjoonsoo.kim@lge.com>
> > ---
> >  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.
diff mbox series

Patch

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,