Re: [PATCH v2] powerpc/pseries: Fix maximum memory value
On Thursday 04 July 2019 04:43 PM, Michael Ellerman wrote: > "Naveen N. Rao" writes: >> Nathan Lynch wrote: >>> Aravinda Prasad writes: Calculating the maximum memory based on the number of lmbs and lmb size does not account for the RMA region. Hence use memory_hotplug_max(), which already accounts for the RMA region, to fetch the maximum memory value. Thanks to Nathan Lynch for suggesting the memory_hotplug_max() function. >>> >>> Well, I hope I haven't led you astray... will it give you the desired >>> result on a kernel configured without memory hotplug support, booted in >>> an LPAR with some huge pages configured? >>> >>> If so, then >>> Acked-by: Nathan Lynch >>> >>> It would likely help with review and future maintenance if the semantics >>> and intended use of the MaxMem field are made a little more >>> explicit. For example, is it supposed to include persistent memory? >>> Perhaps a follow-up patch could address this. Or maybe I'm overthinking >>> it. >> >> This was primarily aimed to replicate what AIX lparstat does and >> documentation (*) just says: >> >> Maximum Memory >> Maximum possible amount of Memory. >> >> I think this mirrors the maximum memory value set in the LPAR profile, >> and this provides a way to obtain that value from within the LPAR. > > But the doc string for memory_hotplug_max() says: > > * memory_hotplug_max - return max address of memory that may be added > > > ie. maximum *address* not maximum *amount*. > > Possibly it turns out to be the same value, but that is just because you > have no holes in your layout. > > So I don't think this patch is correct. memory_hotplug_max (in one of the cases) is taking the value from "ibm,lrdr-capacity" and according to PAPR: PAPR section C.6.3.1 ibm,lrdr-capacity: "The phys (of size #address-cells) communicates the maximum address in bytes and therefore, the most memory that can be allocated to this partition." On other cases memory_hotplug_max() is calculating based on the number of lmbs assigned to the partition, so should still give max mem value > > cheers > -- Regards, Aravinda
Re: [PATCH v2] powerpc/pseries: Fix maximum memory value
"Naveen N. Rao" writes: > Nathan Lynch wrote: >> Aravinda Prasad writes: >>> Calculating the maximum memory based on the number of lmbs >>> and lmb size does not account for the RMA region. Hence >>> use memory_hotplug_max(), which already accounts for the >>> RMA region, to fetch the maximum memory value. Thanks to >>> Nathan Lynch for suggesting the memory_hotplug_max() >>> function. >> >> Well, I hope I haven't led you astray... will it give you the desired >> result on a kernel configured without memory hotplug support, booted in >> an LPAR with some huge pages configured? >> >> If so, then >> Acked-by: Nathan Lynch >> >> It would likely help with review and future maintenance if the semantics >> and intended use of the MaxMem field are made a little more >> explicit. For example, is it supposed to include persistent memory? >> Perhaps a follow-up patch could address this. Or maybe I'm overthinking >> it. > > This was primarily aimed to replicate what AIX lparstat does and > documentation (*) just says: > > Maximum Memory > Maximum possible amount of Memory. > > I think this mirrors the maximum memory value set in the LPAR profile, > and this provides a way to obtain that value from within the LPAR. But the doc string for memory_hotplug_max() says: * memory_hotplug_max - return max address of memory that may be added ie. maximum *address* not maximum *amount*. Possibly it turns out to be the same value, but that is just because you have no holes in your layout. So I don't think this patch is correct. cheers
Re: [PATCH v2] powerpc/pseries: Fix maximum memory value
On Friday 28 June 2019 10:57 PM, Nathan Lynch wrote: > Aravinda Prasad writes: >> Calculating the maximum memory based on the number of lmbs >> and lmb size does not account for the RMA region. Hence >> use memory_hotplug_max(), which already accounts for the >> RMA region, to fetch the maximum memory value. Thanks to >> Nathan Lynch for suggesting the memory_hotplug_max() >> function. > > Well, I hope I haven't led you astray... will it give you the desired > result on a kernel configured without memory hotplug support, booted in > an LPAR with some huge pages configured? Yes. I have tested the patch both with CONFIG_MEMORY_HOTPLUG set and not set. It is working as expected. Regards, Aravinda > > If so, then > Acked-by: Nathan Lynch > > It would likely help with review and future maintenance if the semantics > and intended use of the MaxMem field are made a little more > explicit. For example, is it supposed to include persistent memory? > Perhaps a follow-up patch could address this. Or maybe I'm overthinking > it. > -- Regards, Aravinda
Re: [PATCH v2] powerpc/pseries: Fix maximum memory value
"Naveen N. Rao" writes: > Nathan Lynch wrote: >> It would likely help with review and future maintenance if the semantics >> and intended use of the MaxMem field are made a little more >> explicit. For example, is it supposed to include persistent memory? >> Perhaps a follow-up patch could address this. Or maybe I'm overthinking >> it. > > This was primarily aimed to replicate what AIX lparstat does and > documentation (*) just says: > > Maximum Memory > Maximum possible amount of Memory. > > I think this mirrors the maximum memory value set in the LPAR profile, > and this provides a way to obtain that value from within the LPAR. > > This doesn't necessarily answer your question, but that's at least the > reference. > > (*) > https://www.ibm.com/support/knowledgecenter/en/ssw_aix_72/com.ibm.aix.cmds3/lparstat.htm Thanks for the reference. Consider my concern withdrawn.
Re: [PATCH v2] powerpc/pseries: Fix maximum memory value
Nathan Lynch wrote: Aravinda Prasad writes: Calculating the maximum memory based on the number of lmbs and lmb size does not account for the RMA region. Hence use memory_hotplug_max(), which already accounts for the RMA region, to fetch the maximum memory value. Thanks to Nathan Lynch for suggesting the memory_hotplug_max() function. Well, I hope I haven't led you astray... will it give you the desired result on a kernel configured without memory hotplug support, booted in an LPAR with some huge pages configured? If so, then Acked-by: Nathan Lynch It would likely help with review and future maintenance if the semantics and intended use of the MaxMem field are made a little more explicit. For example, is it supposed to include persistent memory? Perhaps a follow-up patch could address this. Or maybe I'm overthinking it. This was primarily aimed to replicate what AIX lparstat does and documentation (*) just says: Maximum Memory Maximum possible amount of Memory. I think this mirrors the maximum memory value set in the LPAR profile, and this provides a way to obtain that value from within the LPAR. This doesn't necessarily answer your question, but that's at least the reference. (*) https://www.ibm.com/support/knowledgecenter/en/ssw_aix_72/com.ibm.aix.cmds3/lparstat.htm - Naveen
Re: [PATCH v2] powerpc/pseries: Fix maximum memory value
Aravinda Prasad writes: > Calculating the maximum memory based on the number of lmbs > and lmb size does not account for the RMA region. Hence > use memory_hotplug_max(), which already accounts for the > RMA region, to fetch the maximum memory value. Thanks to > Nathan Lynch for suggesting the memory_hotplug_max() > function. Well, I hope I haven't led you astray... will it give you the desired result on a kernel configured without memory hotplug support, booted in an LPAR with some huge pages configured? If so, then Acked-by: Nathan Lynch It would likely help with review and future maintenance if the semantics and intended use of the MaxMem field are made a little more explicit. For example, is it supposed to include persistent memory? Perhaps a follow-up patch could address this. Or maybe I'm overthinking it.
[PATCH v2] powerpc/pseries: Fix maximum memory value
Calculating the maximum memory based on the number of lmbs
and lmb size does not account for the RMA region. Hence
use memory_hotplug_max(), which already accounts for the
RMA region, to fetch the maximum memory value. Thanks to
Nathan Lynch for suggesting the memory_hotplug_max()
function.
Fixes: 772b039fd9a7: ("powerpc/pseries: Export maximum memory value")
Signed-off-by: Aravinda Prasad
---
arch/powerpc/platforms/pseries/lparcfg.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/lparcfg.c
b/arch/powerpc/platforms/pseries/lparcfg.c
index e33e8bc..010a41f 100644
--- a/arch/powerpc/platforms/pseries/lparcfg.c
+++ b/arch/powerpc/platforms/pseries/lparcfg.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -33,7 +34,6 @@
#include
#include
#include
-#include
#include "pseries.h"
@@ -435,7 +435,7 @@ static void maxmem_data(struct seq_file *m)
{
unsigned long maxmem = 0;
- maxmem += drmem_info->n_lmbs * drmem_info->lmb_size;
+ maxmem += memory_hotplug_max();
maxmem += hugetlb_total_pages() * PAGE_SIZE;
seq_printf(m, "MaxMem=%ld\n", maxmem);
