| Message ID | b810cdce-00aa-6cf5-05a9-acc7f3dbb8b6@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [4.17] x86/NUMA: correct off-by-1 in node map population | expand |
On Tue, Oct 04, 2022 at 12:13:49PM +0200, Jan Beulich wrote: > As it turns out populate_memnodemap() so far "relied" on > extract_lsb_from_nodes() setting memnodemapsize one too high in edge > cases. Correct the issue there as well, by changing "epdx" to be an > inclusive PDX and adjusting the respective relational operators. > > While there also limit the scope of both related variables. > > Fixes: b1f4b45d02ca ("x86/NUMA: correct off-by-1 in node map size calculation") > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Roger Pau Monné <roger.pau@citrix.com> Thanks, Roger.
Hi Jan, > -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Subject: [PATCH][4.17] x86/NUMA: correct off-by-1 in node map population > > As it turns out populate_memnodemap() so far "relied" on > extract_lsb_from_nodes() setting memnodemapsize one too high in edge > cases. Correct the issue there as well, by changing "epdx" to be an > inclusive PDX and adjusting the respective relational operators. > > While there also limit the scope of both related variables. > > Fixes: b1f4b45d02ca ("x86/NUMA: correct off-by-1 in node map size > calculation") > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com> Thanks for the patch. IIUC this patch is for a regression that we must fix for the 4.17, so: Release-acked-by: Henry Wang <Henry.Wang@arm.com> Kind regards, Henry
On Tue, Oct 04, 2022 at 12:13:49PM +0200, Jan Beulich wrote: > As it turns out populate_memnodemap() so far "relied" on > extract_lsb_from_nodes() setting memnodemapsize one too high in edge > cases. Correct the issue there as well, by changing "epdx" to be an > inclusive PDX and adjusting the respective relational operators. > > While there also limit the scope of both related variables. > > Fixes: b1f4b45d02ca ("x86/NUMA: correct off-by-1 in node map size calculation") > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com> With this fix sabro boxes now report: Oct 4 12:05:25.087462 (XEN) System RAM: 32429MB (33208204kB) Oct 4 12:05:25.087482 (XEN) SRAT: Node 0 PXM 0 [0000000000000000, 000000007fffffff] Oct 4 12:05:25.171468 (XEN) SRAT: Node 0 PXM 0 [0000000100000000, 000000047fffffff] Oct 4 12:05:25.171489 (XEN) SRAT: Node 1 PXM 1 [0000000480000000, 000000087fffffff] Oct 4 12:05:25.183432 (XEN) NUMA: Using 19 for the hash shift. Oct 4 12:05:25.183453 (XEN) Domain heap initialised DMA width 32 bits Thanks, Roger.
--- a/xen/arch/x86/numa.c +++ b/xen/arch/x86/numa.c @@ -65,15 +65,15 @@ int srat_disabled(void) static int __init populate_memnodemap(const struct node *nodes, int numnodes, int shift, nodeid_t *nodeids) { - unsigned long spdx, epdx; int i, res = -1; memset(memnodemap, NUMA_NO_NODE, memnodemapsize * sizeof(*memnodemap)); for ( i = 0; i < numnodes; i++ ) { - spdx = paddr_to_pdx(nodes[i].start); - epdx = paddr_to_pdx(nodes[i].end - 1) + 1; - if ( spdx >= epdx ) + unsigned long spdx = paddr_to_pdx(nodes[i].start); + unsigned long epdx = paddr_to_pdx(nodes[i].end - 1); + + if ( spdx > epdx ) continue; if ( (epdx >> shift) >= memnodemapsize ) return 0; @@ -88,7 +88,7 @@ static int __init populate_memnodemap(co memnodemap[spdx >> shift] = nodeids[i]; spdx += (1UL << shift); - } while ( spdx < epdx ); + } while ( spdx <= epdx ); res = 1; }
As it turns out populate_memnodemap() so far "relied" on extract_lsb_from_nodes() setting memnodemapsize one too high in edge cases. Correct the issue there as well, by changing "epdx" to be an inclusive PDX and adjusting the respective relational operators. While there also limit the scope of both related variables. Fixes: b1f4b45d02ca ("x86/NUMA: correct off-by-1 in node map size calculation") Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> --- For symmetry it might be helpful to change "epdx" to be inclusive in extract_lsb_from_nodes() as well. I actually had it that way first in the earlier change, but then thought the smaller diff would be better.