[PATCH v3 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests

[PATCH v3 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests

[Leonardo Bras]

While providing guests, it's desirable to resize it's memory on demand.

By now, it's possible to do so by creating a guest with a small base
memory, hot-plugging all the rest, and using 'movable_node' kernel
command-line parameter, which puts all hot-plugged memory in
ZONE_MOVABLE, allowing it to be removed whenever needed.

But there is an issue regarding guest reboot:
If memory is hot-plugged, and then the guest is rebooted, all hot-plugged
memory goes to ZONE_NORMAL, which offers no guaranteed hot-removal.
It usually prevents this memory to be hot-removed from the guest.

It's possible to use device-tree information to fix that behavior, as
it stores flags for LMB ranges on ibm,dynamic-memory-vN.
It involves marking each memblock with the correct flags as hotpluggable
memory, which mm/memblock.c puts in ZONE_MOVABLE during boot if
'movable_node' is passed.

For carrying such information, the new flag DRCONF_MEM_HOTREMOVABLE was
proposed and accepted into Power Architecture documentation.
This flag should be:
- true (b=1) if the hypervisor may want to hot-remove it later, and
- false (b=0) if it does not care.

During boot, guest kernel reads the device-tree, early_init_drmem_lmb()
is called for every added LMBs. Here, checking for this new flag and
marking memblocks as hotplugable memory is enough to get the desirable
behavior.

This should cause no change if 'movable_node' parameter is not passed
in kernel command-line.

Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
Reviewed-by: Bharata B Rao <bharata@linux.ibm.com>

---

Changes since v2:
- New flag name changed from DRCONF_MEM_HOTPLUGGED to
	DRCONF_MEM_HOTREMOVABLE

Changes since v1:
- Adds new flag, so PowerVM is compatible with the change.
- Fixes mistakes in code
---
 arch/powerpc/include/asm/drmem.h | 1 +
 arch/powerpc/kernel/prom.c       | 9 +++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
index 3d76e1c388c2..ad99e27e5b65 100644
--- a/arch/powerpc/include/asm/drmem.h
+++ b/arch/powerpc/include/asm/drmem.h
@@ -65,6 +65,7 @@ struct of_drconf_cell_v2 {
 #define DRCONF_MEM_ASSIGNED	0x00000008
 #define DRCONF_MEM_AI_INVALID	0x00000040
 #define DRCONF_MEM_RESERVED	0x00000080
+#define DRCONF_MEM_HOTREMOVABLE	0x00000100
 
 static inline u32 drmem_lmb_size(void)
 {
diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 6620f37abe73..abc9b04d03ce 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -515,9 +515,14 @@ static void __init early_init_drmem_lmb(struct drmem_lmb *lmb,
 				size = 0x80000000ul - base;
 		}
 
+		if (!validate_mem_limit(base, &size))
+			continue;
+
 		DBG("Adding: %llx -> %llx\n", base, size);
-		if (validate_mem_limit(base, &size))
-			memblock_add(base, size);
+		memblock_add(base, size);
+
+		if (lmb->flags & DRCONF_MEM_HOTREMOVABLE)
+			memblock_mark_hotplug(base, size);
 	} while (--rngs);
 }
 #endif /* CONFIG_PPC_PSERIES */
-- 
2.25.1

Re: [PATCH v3 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests

[Oliver O'Halloran]

On Fri, Apr 3, 2020 at 6:55 AM Leonardo Bras <leonardo@linux.ibm.com> wrote:
>
> While providing guests, it's desirable to resize it's memory on demand.
>
> By now, it's possible to do so by creating a guest with a small base
> memory, hot-plugging all the rest, and using 'movable_node' kernel
> command-line parameter, which puts all hot-plugged memory in
> ZONE_MOVABLE, allowing it to be removed whenever needed.
>
> But there is an issue regarding guest reboot:
> If memory is hot-plugged, and then the guest is rebooted, all hot-plugged
> memory goes to ZONE_NORMAL, which offers no guaranteed hot-removal.
> It usually prevents this memory to be hot-removed from the guest.
>
> It's possible to use device-tree information to fix that behavior, as
> it stores flags for LMB ranges on ibm,dynamic-memory-vN.
> It involves marking each memblock with the correct flags as hotpluggable
> memory, which mm/memblock.c puts in ZONE_MOVABLE during boot if
> 'movable_node' is passed.

I don't really understand why the flag is needed at all. According to
PAPR any memory provided by dynamic reconfiguration can be hot-removed
so why aren't we treating all DR memory as hot removable? The only
memory guaranteed to be there 100% of the time is what's in the
/memory@0 node since that's supposed to cover the real mode area.

Re: [PATCH v3 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests

[Leonardo Bras]

[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]

Hello Oliver, thank you for the feedback.
Comments inline:

On Fri, 2020-04-03 at 09:46 +1100, Oliver O'Halloran wrote:
> 
> I don't really understand why the flag is needed at all. According to
> PAPR any memory provided by dynamic reconfiguration can be hot-removed
> so why aren't we treating all DR memory as hot removable? The only
> memory guaranteed to be there 100% of the time is what's in the
> /memory@0 node since that's supposed to cover the real mode area.

All LMBs are listed in DR memory, even the base memory.

The v1 of the patch would work this way, as qemu would configure it's
DR memory with (DRC_INVALID | RESERVED) flags and the hot-added memory
with (ASSIGNED) flag. Looking for assigned flag would be enough.

But as of today, PowerVM doesn't seem to work that way. 
When you boot a PowerVM virtual machine with Linux, all memory is added
with the same flags (ASSIGNED).

To create a solution that doesn't break PowerVM, this new flag was made
necessary.

Best regards,
Leonardo Bras

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests

[Oliver O'Halloran]

On Fri, Apr 3, 2020 at 10:07 AM Leonardo Bras <leonardo@linux.ibm.com> wrote:
>
> Hello Oliver, thank you for the feedback.
> Comments inline:
>
> On Fri, 2020-04-03 at 09:46 +1100, Oliver O'Halloran wrote:
> >
> > I don't really understand why the flag is needed at all. According to
> > PAPR any memory provided by dynamic reconfiguration can be hot-removed
> > so why aren't we treating all DR memory as hot removable? The only
> > memory guaranteed to be there 100% of the time is what's in the
> > /memory@0 node since that's supposed to cover the real mode area.
>
> All LMBs are listed in DR memory, even the base memory.
>
> The v1 of the patch would work this way, as qemu would configure it's
> DR memory with (DRC_INVALID | RESERVED) flags and the hot-added memory
> with (ASSIGNED) flag. Looking for assigned flag would be enough.
>
> But as of today, PowerVM doesn't seem to work that way.
> When you boot a PowerVM virtual machine with Linux, all memory is added
> with the same flags (ASSIGNED).
>
> To create a solution that doesn't break PowerVM, this new flag was made
> necessary.

I'm still not convinced it's necessary. Why not check memory@0 and use
the size as a clip level? Any memory above that level gets marked as
hotpluggable and anything below doesn't. Seems to me that would work
on all current platforms, so what am I missing here?

>
> Best regards,
> Leonardo Bras

Re: [PATCH v3 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests

[Leonardo Bras]

[-- Attachment #1: Type: text/plain, Size: 1823 bytes --]

On Fri, 2020-04-03 at 10:31 +1100, Oliver O'Halloran wrote:
> On Fri, Apr 3, 2020 at 10:07 AM Leonardo Bras <leonardo@linux.ibm.com> wrote:
> > Hello Oliver, thank you for the feedback.
> > Comments inline:
> > 
> > On Fri, 2020-04-03 at 09:46 +1100, Oliver O'Halloran wrote:
> > > I don't really understand why the flag is needed at all. According to
> > > PAPR any memory provided by dynamic reconfiguration can be hot-removed
> > > so why aren't we treating all DR memory as hot removable? The only
> > > memory guaranteed to be there 100% of the time is what's in the
> > > /memory@0 node since that's supposed to cover the real mode area.
> > 
> > All LMBs are listed in DR memory, even the base memory.
> > 
> > The v1 of the patch would work this way, as qemu would configure it's
> > DR memory with (DRC_INVALID | RESERVED) flags and the hot-added memory
> > with (ASSIGNED) flag. Looking for assigned flag would be enough.
> > 
> > But as of today, PowerVM doesn't seem to work that way.
> > When you boot a PowerVM virtual machine with Linux, all memory is added
> > with the same flags (ASSIGNED).
> > 
> > To create a solution that doesn't break PowerVM, this new flag was made
> > necessary.
> 
> I'm still not convinced it's necessary. Why not check memory@0 and use
> the size as a clip level? Any memory above that level gets marked as
> hotpluggable and anything below doesn't. Seems to me that would work
> on all current platforms, so what am I missing here?
> 

Humm, wouldn't that assume that all unmovable memory should be at the
first LMBs? 

If we use the recently approved flag, there would be no such
limitation, and there would be possible to do some other things, like
hot-adding more unmovable memory to kernel usage.

What do you think?

Best regards,

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests

[Bharata B Rao]

On Thu, Apr 02, 2020 at 04:51:57PM -0300, Leonardo Bras wrote:
> While providing guests, it's desirable to resize it's memory on demand.
> 
> By now, it's possible to do so by creating a guest with a small base
> memory, hot-plugging all the rest, and using 'movable_node' kernel
> command-line parameter, which puts all hot-plugged memory in
> ZONE_MOVABLE, allowing it to be removed whenever needed.
> 
> But there is an issue regarding guest reboot:
> If memory is hot-plugged, and then the guest is rebooted, all hot-plugged
> memory goes to ZONE_NORMAL, which offers no guaranteed hot-removal.
> It usually prevents this memory to be hot-removed from the guest.
> 
> It's possible to use device-tree information to fix that behavior, as
> it stores flags for LMB ranges on ibm,dynamic-memory-vN.
> It involves marking each memblock with the correct flags as hotpluggable
> memory, which mm/memblock.c puts in ZONE_MOVABLE during boot if
> 'movable_node' is passed.
> 
> For carrying such information, the new flag DRCONF_MEM_HOTREMOVABLE was
> proposed and accepted into Power Architecture documentation.
> This flag should be:
> - true (b=1) if the hypervisor may want to hot-remove it later, and
> - false (b=0) if it does not care.
> 
> During boot, guest kernel reads the device-tree, early_init_drmem_lmb()
> is called for every added LMBs. Here, checking for this new flag and
> marking memblocks as hotplugable memory is enough to get the desirable
> behavior.
> 
> This should cause no change if 'movable_node' parameter is not passed
> in kernel command-line.
> 
> Signed-off-by: Leonardo Bras <leonardo@linux.ibm.com>
> Reviewed-by: Bharata B Rao <bharata@linux.ibm.com>
> 
> ---
> 
> Changes since v2:
> - New flag name changed from DRCONF_MEM_HOTPLUGGED to
> 	DRCONF_MEM_HOTREMOVABLE

The patch would be more complete with the following change that ensures
that DRCONF_MEM_HOTREMOVABLE flag is set for non-boot-time hotplugged
memory too. This will ensure that ibm,dynamic-memory-vN property
reflects the right flags value for memory that gets hotplugged
post boot.

diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index a4d40a3ceea3..6d75f6e182ae 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -395,7 +395,8 @@ static int dlpar_remove_lmb(struct drmem_lmb *lmb)
 
 	invalidate_lmb_associativity_index(lmb);
 	lmb_clear_nid(lmb);
-	lmb->flags &= ~DRCONF_MEM_ASSIGNED;
+	lmb->flags &= ~(DRCONF_MEM_ASSIGNED |
+			DRCONF_MEM_HOTREMOVABLE);
 
 	return 0;
 }
@@ -678,7 +679,8 @@ static int dlpar_add_lmb(struct drmem_lmb *lmb)
 		invalidate_lmb_associativity_index(lmb);
 		lmb_clear_nid(lmb);
 	} else {
-		lmb->flags |= DRCONF_MEM_ASSIGNED;
+		lmb->flags |= (DRCONF_MEM_ASSIGNED |
+			       DRCONF_MEM_HOTREMOVABLE);
 	}
 
 	return rc;

Regards,
Bharata.

Re: [PATCH v3 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests

[Leonardo Bras]

[-- Attachment #1: Type: text/plain, Size: 521 bytes --]

Hello Bharata,

On Fri, 2020-04-03 at 20:08 +0530, Bharata B Rao wrote:
> The patch would be more complete with the following change that ensures
> that DRCONF_MEM_HOTREMOVABLE flag is set for non-boot-time hotplugged
> memory too. This will ensure that ibm,dynamic-memory-vN property
> reflects the right flags value for memory that gets hotplugged
> post boot.
> 

You just sent that on a separated patchset, so I think it's dealt with.
Do you have any other comments on the present patch?

Best regards,

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests

[Bharata B Rao]

On Mon, Apr 06, 2020 at 12:41:01PM -0300, Leonardo Bras wrote:
> Hello Bharata,
> 
> On Fri, 2020-04-03 at 20:08 +0530, Bharata B Rao wrote:
> > The patch would be more complete with the following change that ensures
> > that DRCONF_MEM_HOTREMOVABLE flag is set for non-boot-time hotplugged
> > memory too. This will ensure that ibm,dynamic-memory-vN property
> > reflects the right flags value for memory that gets hotplugged
> > post boot.
> > 
> 
> You just sent that on a separated patchset, so I think it's dealt with.
> Do you have any other comments on the present patch?

None, thanks.

Regards,
Bharata.

Re: [PATCH v3 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests

[Leonardo Bras]

[-- Attachment #1: Type: text/plain, Size: 817 bytes --]

Hello Michael,

Would it be ok to add this patch for 5.7 ? Or too late?

Regards,

On Tue, 2020-04-07 at 09:30 +0530, Bharata B Rao wrote:
> On Mon, Apr 06, 2020 at 12:41:01PM -0300, Leonardo Bras wrote:
> > Hello Bharata,
> > 
> > On Fri, 2020-04-03 at 20:08 +0530, Bharata B Rao wrote:
> > > The patch would be more complete with the following change that ensures
> > > that DRCONF_MEM_HOTREMOVABLE flag is set for non-boot-time hotplugged
> > > memory too. This will ensure that ibm,dynamic-memory-vN property
> > > reflects the right flags value for memory that gets hotplugged
> > > post boot.
> > > 
> > 
> > You just sent that on a separated patchset, so I think it's dealt with.
> > Do you have any other comments on the present patch?
> 
> None, thanks.
> 
> Regards,
> Bharata.
> 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests

[Michael Ellerman]

On Thu, 2 Apr 2020 16:51:57 -0300, Leonardo Bras wrote:
> While providing guests, it's desirable to resize it's memory on demand.
> 
> By now, it's possible to do so by creating a guest with a small base
> memory, hot-plugging all the rest, and using 'movable_node' kernel
> command-line parameter, which puts all hot-plugged memory in
> ZONE_MOVABLE, allowing it to be removed whenever needed.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests
      https://git.kernel.org/powerpc/c/b6eca183e23e7a6625a0d2cdb806b7cd1abcd2d2

cheers