mm: skip if required_kernelcore is larger than totalpages

PATCH In review Xishi Qiu 4 participants 8 messages Oct 8, 2015 – Oct 16, 2015

Subsystems (2)

Memory management Page allocator

lore

Share & tools

lore

Xishi Qiu

author

2015-10-08 02:21 UTC #0 lore raw

PATCH 1/1 mm: skip if required_kernelcore is larger than totalpages

If kernelcore was not specified, or the kernelcore size is zero (required_movablecore >= totalpages), or the kernelcore size is larger than totalpages, there is no ZONE_MOVABLE. We should fill the zone with both kernel memory and movable memory.

Signed-off-by Xishi Qiu <[email protected]>

---
 mm/page_alloc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

mm/page_alloc.c +5 -2

index af3c9bd..6a6da0d 100644
@@ -5674,8 +5674,11 @@ static void __init find_zone_movable_pfns_for_nodes(void)
 		required_kernelcore = max(required_kernelcore, corepages);
 	}
 
-	/* If kernelcore was not specified, there is no ZONE_MOVABLE */
-	if (!required_kernelcore)
+	/*
+	 * If kernelcore was not specified or kernelcore size is larger
+	 * than totalpages, there is no ZONE_MOVABLE.
+	 */
+	if (!required_kernelcore || required_kernelcore >= totalpages)
 		goto out;
 
 	/* usable_startpfn is the lowest possible pfn ZONE_MOVABLE can be at */

Signature · 1 line

2.0.0

Yasuaki Ishimatsu

2015-10-09 15:41 UTC #1 lore raw

Quoted 2 lines · On Thu, 8 Oct 2015 10:21:05 +0800 Xishi Qiu <[email protected]> wrote:

If kernelcore was not specified, or the kernelcore size is zero (required_movablecore >= totalpages), or the kernelcore size is larger

Why does required_movablecore become larger than totalpages, when the kernelcore size is zero? I read the code but I could not find that you mention.

Thanks,
Yasuaki Ishimatsu

Quoted 35 lines

than totalpages, there is no ZONE_MOVABLE. We should fill the zone with both kernel memory and movable memory.

Signed-off-by Xishi Qiu <[email protected]>

---
 mm/page_alloc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

mm/page_alloc.c +5 -2

index af3c9bd..6a6da0d 100644
@@ -5674,8 +5674,11 @@ static void __init find_zone_movable_pfns_for_nodes(void)
 		required_kernelcore = max(required_kernelcore, corepages);
 	}
 
-	/* If kernelcore was not specified, there is no ZONE_MOVABLE */
-	if (!required_kernelcore)
+	/*
+	 * If kernelcore was not specified or kernelcore size is larger
+	 * than totalpages, there is no ZONE_MOVABLE.
+	 */
+	if (!required_kernelcore || required_kernelcore >= totalpages)
 		goto out;
 
 	/* usable_startpfn is the lowest possible pfn ZONE_MOVABLE can be at */

Signature · 8 lines

2.0.0


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Xishi Qiu

author

2015-10-12 01:40 UTC #2 lore raw

Quoted 11 lines · Yasuaki Ishimatsu

Quoted 2 lines · On Thu, 8 Oct 2015 10:21:05 +0800 Xishi Qiu <[email protected]> wrote:
If kernelcore was not specified, or the kernelcore size is zero (required_movablecore >= totalpages), or the kernelcore size is larger
Why does required_movablecore become larger than totalpages, when the kernelcore size is zero? I read the code but I could not find that you mention.

If user only set boot option movablecore, and the value is larger than totalpages, the calculation of kernelcore is zero, but we can't fill the zone only with kernelcore, so skip it.

I have send a patch before this patch.
"fix overflow in find_zone_movable_pfns_for_nodes()"

		...
 		required_movablecore =
 			roundup(required_movablecore, MAX_ORDER_NR_PAGES);
+		required_movablecore = min(totalpages, required_movablecore);
 		corepages = totalpages - required_movablecore;
		...

Thanks,
Xishi Qiu

Quoted 41 lines

Thanks,
Yasuaki Ishimatsu

Quoted 35 lines

than totalpages, there is no ZONE_MOVABLE. We should fill the zone with both kernel memory and movable memory.

Signed-off-by Xishi Qiu <[email protected]>

---
 mm/page_alloc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

mm/page_alloc.c +5 -2

index af3c9bd..6a6da0d 100644
@@ -5674,8 +5674,11 @@ static void __init find_zone_movable_pfns_for_nodes(void)
 		required_kernelcore = max(required_kernelcore, corepages);
 	}
 
-	/* If kernelcore was not specified, there is no ZONE_MOVABLE */
-	if (!required_kernelcore)
+	/*
+	 * If kernelcore was not specified or kernelcore size is larger
+	 * than totalpages, there is no ZONE_MOVABLE.
+	 */
+	if (!required_kernelcore || required_kernelcore >= totalpages)
 		goto out;
 
 	/* usable_startpfn is the lowest possible pfn ZONE_MOVABLE can be at */

Signature · 8 lines

2.0.0


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Tang Chen

2015-10-14 02:52 UTC #3 lore raw

Hi, Qiu

The patch seems OK to me. Only one little concern below.

Quoted 20 lines · Xishi Qiu

Quoted 9 lines · Yasuaki Ishimatsu
Quoted 2 lines · On Thu, 8 Oct 2015 10:21:05 +0800 Xishi Qiu <[email protected]> wrote:
If kernelcore was not specified, or the kernelcore size is zero (required_movablecore >= totalpages), or the kernelcore size is larger
Why does required_movablecore become larger than totalpages, when the kernelcore size is zero? I read the code but I could not find that you mention.
If user only set boot option movablecore, and the value is larger than totalpages, the calculation of kernelcore is zero, but we can't fill the zone only with kernelcore, so skip it.
I have send a patch before this patch.
"fix overflow in find_zone_movable_pfns_for_nodes()"
		...
required_movablecore =
roundup(required_movablecore, MAX_ORDER_NR_PAGES);

Review

Quoted 3 lines

(quoted diff) +1 -0

+		required_movablecore = min(totalpages, required_movablecore);
  		corepages = totalpages - required_movablecore;
...

So if required_movablecore >= totalpages, there won't be any ZONE_MOVABLE. How about add a warning or debug info to tell the user he has specified a too large movablecore, and it is ignored ?

Thanks.

Quoted 46 lines

Thanks,
Xishi Qiu
Quoted 33 lines
Thanks,
Yasuaki Ishimatsu
Quoted 30 lines
than totalpages, there is no ZONE_MOVABLE. We should fill the zone with both kernel memory and movable memory.
Signed-off-by Xishi Qiu <[email protected]>
---
  mm/page_alloc.c | 7 +++++--
  1 file changed, 5 insertions(+), 2 deletions(-)
mm/page_alloc.c +5 -2
index af3c9bd..6a6da0d 100644
@@ -5674,8 +5674,11 @@ static void __init find_zone_movable_pfns_for_nodes(void)
  		required_kernelcore = max(required_kernelcore, corepages);
  	}
 
-	/* If kernelcore was not specified, there is no ZONE_MOVABLE */
-	if (!required_kernelcore)
+	/*
+	 * If kernelcore was not specified or kernelcore size is larger
+	 * than totalpages, there is no ZONE_MOVABLE.
+	 */
+	if (!required_kernelcore || required_kernelcore >= totalpages)
  		goto out;
 
  	/* usable_startpfn is the lowest possible pfn ZONE_MOVABLE can be at */
Signature · 1 line
2.0.0
Quoted 7 lines
Quoted 5 lines
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
.

Quoted 2 lines

.

Xishi Qiu

author

2015-10-14 03:37 UTC #4 lore raw

Quoted 30 lines · Tang Chen

Hi, Qiu
The patch seems OK to me. Only one little concern below.
Quoted 15 lines · Xishi Qiu
Quoted 9 lines · Yasuaki Ishimatsu
Quoted 2 lines · On Thu, 8 Oct 2015 10:21:05 +0800 Xishi Qiu <[email protected]> wrote:
If kernelcore was not specified, or the kernelcore size is zero (required_movablecore >= totalpages), or the kernelcore size is larger
Why does required_movablecore become larger than totalpages, when the kernelcore size is zero? I read the code but I could not find that you mention.
If user only set boot option movablecore, and the value is larger than totalpages, the calculation of kernelcore is zero, but we can't fill the zone only with kernelcore, so skip it.
Quoted 8 lines
I have send a patch before this patch.
"fix overflow in find_zone_movable_pfns_for_nodes()"
        ...
          required_movablecore =
              roundup(required_movablecore, MAX_ORDER_NR_PAGES);
+        required_movablecore = min(totalpages, required_movablecore);
          corepages = totalpages - required_movablecore;
        ...

Quoted 5 lines

So if required_movablecore >= totalpages, there won't be any ZONE_MOVABLE. How about add a warning or debug info to tell the user he has specified a too large movablecore, and it is ignored ?
Thanks.

Yes, but I don't think is is necessary, user should know the total memory before he set the boot option.

Thanks,
Xishi Qiu

Tang Chen

2015-10-14 04:05 UTC #5 lore raw

Quoted 31 lines · Xishi Qiu

Quoted 29 lines · Tang Chen
Hi, Qiu
The patch seems OK to me. Only one little concern below.
Quoted 23 lines · Xishi Qiu
Quoted 9 lines · Yasuaki Ishimatsu
Quoted 2 lines · On Thu, 8 Oct 2015 10:21:05 +0800 Xishi Qiu <[email protected]> wrote:
If kernelcore was not specified, or the kernelcore size is zero (required_movablecore >= totalpages), or the kernelcore size is larger
Why does required_movablecore become larger than totalpages, when the kernelcore size is zero? I read the code but I could not find that you mention.
If user only set boot option movablecore, and the value is larger than totalpages, the calculation of kernelcore is zero, but we can't fill the zone only with kernelcore, so skip it.
I have send a patch before this patch.
"fix overflow in find_zone_movable_pfns_for_nodes()"
         ...
           required_movablecore =
               roundup(required_movablecore, MAX_ORDER_NR_PAGES);
+        required_movablecore = min(totalpages, required_movablecore);
           corepages = totalpages - required_movablecore;
         ...

Quoted 7 lines

Quoted 5 lines
So if required_movablecore >= totalpages, there won't be any ZONE_MOVABLE. How about add a warning or debug info to tell the user he has specified a too large movablecore, and it is ignored ?
Thanks.
Yes, but I don't think is is necessary, user should know the total memory before he set the boot option.

Well, I'm just thinking if I set a too large movablecore, but I didn't realize it. And when the system boots up, there is no ZONE_MOVABLE. It may confuse me.

Thanks.

Quoted 6 lines

Thanks,
Xishi Qiu
.

Yasuaki Ishimatsu

Reviewed-by

2015-10-16 16:58 UTC #6 lore raw

In reply to Xishi Qiu

Quoted 9 lines · On Mon, 12 Oct 2015 09:37:17 +0800 Xishi Qiu <[email protected]> wrote:

Quoted 7 lines · Yasuaki Ishimatsu
Quoted 2 lines · On Thu, 8 Oct 2015 10:21:05 +0800 Xishi Qiu <[email protected]> wrote:
If kernelcore was not specified, or the kernelcore size is zero (required_movablecore >= totalpages), or the kernelcore size is larger

Quoted 8 lines

Quoted 4 lines
Why does required_movablecore become larger than totalpages, when the kernelcore size is zero? I read the code but I could not find that you mention.
If user only set boot option movablecore, and the value is larger than totalpages, the calculation of kernelcore is zero, but we can't fill the zone only with kernelcore, so skip it.

Thank you for the explantion. Your patch looks good to me.

Reviewed-by Yasuaki Ishimatsu <[email protected]>

Thanks,
Yasuaki Ishimatsu

Quoted 10 lines

I have send a patch before this patch.
"fix overflow in find_zone_movable_pfns_for_nodes()"
		...
(quoted diff) +1 -0
 		required_movablecore =
 			roundup(required_movablecore, MAX_ORDER_NR_PAGES);
+		required_movablecore = min(totalpages, required_movablecore);
 		corepages = totalpages - required_movablecore;
...

Quoted 47 lines

Thanks,
Xishi Qiu

Quoted 41 lines

Thanks,
Yasuaki Ishimatsu

Quoted 35 lines

than totalpages, there is no ZONE_MOVABLE. We should fill the zone with both kernel memory and movable memory.

Signed-off-by Xishi Qiu <[email protected]>

---
 mm/page_alloc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

mm/page_alloc.c +5 -2

index af3c9bd..6a6da0d 100644
@@ -5674,8 +5674,11 @@ static void __init find_zone_movable_pfns_for_nodes(void)
 		required_kernelcore = max(required_kernelcore, corepages);
 	}
 
-	/* If kernelcore was not specified, there is no ZONE_MOVABLE */
-	if (!required_kernelcore)
+	/*
+	 * If kernelcore was not specified or kernelcore size is larger
+	 * than totalpages, there is no ZONE_MOVABLE.
+	 */
+	if (!required_kernelcore || required_kernelcore >= totalpages)
 		goto out;
 
 	/* usable_startpfn is the lowest possible pfn ZONE_MOVABLE can be at */

Signature · 8 lines

2.0.0


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

David Rientjes

Acked-by on v1 1/1

2015-10-14 02:31 UTC #7 lore raw

In reply to Xishi Qiu

Quoted 6 lines · Xishi Qiu

If kernelcore was not specified, or the kernelcore size is zero (required_movablecore >= totalpages), or the kernelcore size is larger than totalpages, there is no ZONE_MOVABLE. We should fill the zone with both kernel memory and movable memory.
Signed-off-by Xishi Qiu <[email protected]>

Acked-by David Rientjes <[email protected]>