Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1654928 > unrolled thread

[PATCH 1/2] mm, memory_hotplug: fix MMOP_ONLINE_KEEP behavior

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/2] mm, memory_hotplug: fix MMOP_ONLINE_KEEP behavior Michal Hocko <mhocko@kernel.org> - 2017-06-01 10:40 +0200
    Re: [PATCH 1/2] mm, memory_hotplug: fix MMOP_ONLINE_KEEP behavior Vlastimil Babka <vbabka@suse.cz> - 2017-06-01 14:40 +0200
      Re: [PATCH 1/2] mm, memory_hotplug: fix MMOP_ONLINE_KEEP behavior Michal Hocko <mhocko@kernel.org> - 2017-06-01 14:50 +0200

#1654928 — [PATCH 1/2] mm, memory_hotplug: fix MMOP_ONLINE_KEEP behavior

From: Michal Hocko <mhocko@suse.com>

Heiko Carstens has noticed that the MMOP_ONLINE_KEEP is broken currently
$ grep . memory3?/valid_zones
memory34/valid_zones:Normal Movable
memory35/valid_zones:Normal Movable
memory36/valid_zones:Normal Movable
memory37/valid_zones:Normal Movable

$ echo online_movable > memory34/state
$ grep . memory3?/valid_zones
memory34/valid_zones:Movable
memory35/valid_zones:Movable
memory36/valid_zones:Movable
memory37/valid_zones:Movable

$ echo online > memory36/state
$ grep . memory3?/valid_zones
memory34/valid_zones:Movable
memory36/valid_zones:Normal
memory37/valid_zones:Movable

so we have effectivelly punched a hole into the movable zone. The
problem is that move_pfn_range() check for MMOP_ONLINE_KEEP is wrong.
It only checks whether the given range is already part of the movable
zone which is not the case here as only memory34 is in the zone. Fix
this by using allow_online_pfn_range(..., MMOP_ONLINE_KERNEL) if that
is false then we can be sure that movable onlining is the right thing to
do.

Reported-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Tested-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Fixes: "mm, memory_hotplug: do not associate hotadded memory to zones until online"
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 mm/memory_hotplug.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 0a895df2397e..b3895fd609f4 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -950,11 +950,12 @@ static struct zone * __meminit move_pfn_range(int online_type, int nid,
 	if (online_type == MMOP_ONLINE_KEEP) {
 		struct zone *movable_zone = &pgdat->node_zones[ZONE_MOVABLE];
 		/*
-		 * MMOP_ONLINE_KEEP inherits the current zone which is
-		 * ZONE_NORMAL by default but we might be within ZONE_MOVABLE
-		 * already.
+		 * MMOP_ONLINE_KEEP defaults to MMOP_ONLINE_KERNEL but use
+		 * movable zone if that is not possible (e.g. we are within
+		 * or past the existing movable zone)
 		 */
-		if (zone_intersects(movable_zone, start_pfn, nr_pages))
+		if (!allow_online_pfn_range(nid, start_pfn, nr_pages,
+					MMOP_ONLINE_KERNEL))
 			zone = movable_zone;
 	} else if (online_type == MMOP_ONLINE_MOVABLE) {
 		zone = &pgdat->node_zones[ZONE_MOVABLE];
-- 
2.11.0

[toc] | [next] | [standalone]

#1655098

On 06/01/2017 10:37 AM, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
> 
> Heiko Carstens has noticed that the MMOP_ONLINE_KEEP is broken currently
> $ grep . memory3?/valid_zones
> memory34/valid_zones:Normal Movable
> memory35/valid_zones:Normal Movable
> memory36/valid_zones:Normal Movable
> memory37/valid_zones:Normal Movable
> 
> $ echo online_movable > memory34/state
> $ grep . memory3?/valid_zones
> memory34/valid_zones:Movable
> memory35/valid_zones:Movable
> memory36/valid_zones:Movable
> memory37/valid_zones:Movable
> 
> $ echo online > memory36/state
> $ grep . memory3?/valid_zones
> memory34/valid_zones:Movable
> memory36/valid_zones:Normal
> memory37/valid_zones:Movable
> 
> so we have effectivelly punched a hole into the movable zone. The
> problem is that move_pfn_range() check for MMOP_ONLINE_KEEP is wrong.
> It only checks whether the given range is already part of the movable
> zone which is not the case here as only memory34 is in the zone. Fix
> this by using allow_online_pfn_range(..., MMOP_ONLINE_KERNEL) if that
> is false then we can be sure that movable onlining is the right thing to
> do.
> 
> Reported-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> Tested-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> Fixes: "mm, memory_hotplug: do not associate hotadded memory to zones until online"

Just fold it there before sending to Linus, right?

> Signed-off-by: Michal Hocko <mhocko@suse.com>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  mm/memory_hotplug.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 0a895df2397e..b3895fd609f4 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -950,11 +950,12 @@ static struct zone * __meminit move_pfn_range(int online_type, int nid,
>  	if (online_type == MMOP_ONLINE_KEEP) {
>  		struct zone *movable_zone = &pgdat->node_zones[ZONE_MOVABLE];
>  		/*
> -		 * MMOP_ONLINE_KEEP inherits the current zone which is
> -		 * ZONE_NORMAL by default but we might be within ZONE_MOVABLE
> -		 * already.
> +		 * MMOP_ONLINE_KEEP defaults to MMOP_ONLINE_KERNEL but use
> +		 * movable zone if that is not possible (e.g. we are within
> +		 * or past the existing movable zone)
>  		 */
> -		if (zone_intersects(movable_zone, start_pfn, nr_pages))
> +		if (!allow_online_pfn_range(nid, start_pfn, nr_pages,
> +					MMOP_ONLINE_KERNEL))
>  			zone = movable_zone;
>  	} else if (online_type == MMOP_ONLINE_MOVABLE) {
>  		zone = &pgdat->node_zones[ZONE_MOVABLE];
> 

[toc] | [prev] | [next] | [standalone]

#1655106

On Thu 01-06-17 14:32:42, Vlastimil Babka wrote:
> On 06/01/2017 10:37 AM, Michal Hocko wrote:
> > From: Michal Hocko <mhocko@suse.com>
> > 
> > Heiko Carstens has noticed that the MMOP_ONLINE_KEEP is broken currently
> > $ grep . memory3?/valid_zones
> > memory34/valid_zones:Normal Movable
> > memory35/valid_zones:Normal Movable
> > memory36/valid_zones:Normal Movable
> > memory37/valid_zones:Normal Movable
> > 
> > $ echo online_movable > memory34/state
> > $ grep . memory3?/valid_zones
> > memory34/valid_zones:Movable
> > memory35/valid_zones:Movable
> > memory36/valid_zones:Movable
> > memory37/valid_zones:Movable
> > 
> > $ echo online > memory36/state
> > $ grep . memory3?/valid_zones
> > memory34/valid_zones:Movable
> > memory36/valid_zones:Normal
> > memory37/valid_zones:Movable
> > 
> > so we have effectivelly punched a hole into the movable zone. The
> > problem is that move_pfn_range() check for MMOP_ONLINE_KEEP is wrong.
> > It only checks whether the given range is already part of the movable
> > zone which is not the case here as only memory34 is in the zone. Fix
> > this by using allow_online_pfn_range(..., MMOP_ONLINE_KERNEL) if that
> > is false then we can be sure that movable onlining is the right thing to
> > do.
> > 
> > Reported-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> > Tested-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> > Fixes: "mm, memory_hotplug: do not associate hotadded memory to zones until online"
> 
> Just fold it there before sending to Linus, right?

I do not have a strong preference. The changelog could still be helpful
for reference. The original patch is quite large and details like this
are likely to get lost there.

> 
> > Signed-off-by: Michal Hocko <mhocko@suse.com>
> 
> Acked-by: Vlastimil Babka <vbabka@suse.cz>

Thanks!
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web