On 03.12.2024 12:12, Bernhard Kaindl wrote: > This the 2nd part of my submission to fix the NUMA node memsize > retured in xen_sysctl_meminfo[].memsize by the XEN_SYSCTL_numainfo > hypercall to not count MMIO memory holes etc but only memory pages. > > For this, we introduced NODE_DATA->node_present_pages as a prereq. > With the prereq merged in master, I send this 2nd part for review: > > This RFC is for changing the value of xen_sysctl_meminfo[]->memsize > from NODE_DATA->node_spanned_pages << PAGE_SHIFT > to NODE_DATA->node_present_pages << PAGE_SHIFT > for returing total present NUMA node memory instead of spanned range. > > Sample of struct xen_sysctl_meminfo[].* as presented by in xl info -n: > > xl info -n: > [...] > node: memsize memfree distances > 0: -> 67584 <- 60672 10,21 > 1: 65536 60958 21,10 > > The -> memsize <- marked here is the value that we'd like to fix: > The current value based node_spanned_pages is often 2TB too large. > > We're currently not using these often false memsize values in XenServer > according to my code review and and Andrew seemed to confirm this as well. > > I think that the same is likely true for other Xen toolstacks, but of course > to review this change or propose an alternaive is the purpose of this RFC. > > Thanks, > Bernhard
All of the above reads like a cover letter. What's missing is a patch description, part of which would be to clarify whether the field is indeed unused except for display purposes, or why respective users would at least not regress from this change. What's also unclear is what comments you're actually after (i.e. what question(s) you want to have answered), seeing this is tagged RFC. The code change is in principle fine, just that ... > Signed-off-by: Bernhard Kaindl <[email protected]> > --- > xen/common/sysctl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c > index d02f44fe3a..cba6d3cfea 100644 > --- a/xen/common/sysctl.c > +++ b/xen/common/sysctl.c > @@ -316,7 +316,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) > u_sysctl) > { > if ( node_online(i) ) > { > - meminfo.memsize = node_spanned_pages(i) << > PAGE_SHIFT; > + meminfo.memsize = node_present_pages(i) << > PAGE_SHIFT; > meminfo.memfree = avail_node_heap_pages(i) << > PAGE_SHIFT; > } > else > -- > 2.43.0 > > Signed-off-by: Bernhard Kaindl <[email protected]> > --- > xen/common/sysctl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c > index d02f44fe3a..cba6d3cfea 100644 > --- a/xen/common/sysctl.c > +++ b/xen/common/sysctl.c > @@ -316,7 +316,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) > u_sysctl) > { > if ( node_online(i) ) > { > - meminfo.memsize = node_spanned_pages(i) << > PAGE_SHIFT; > + meminfo.memsize = node_present_pages(i) << > PAGE_SHIFT; > meminfo.memfree = avail_node_heap_pages(i) << > PAGE_SHIFT; > } > else ... everything appears twice in your email. Jan
