| Message ID | 20241029202041.25334-4-gourry@gourry.net |
|---|---|
| State | Superseded |
| Headers | show |
| Series | memory,x86,acpi: hotplug memory alignment advisement | expand |
On 29.10.24 21:20, Gregory Price wrote: > Capacity is stranded when CFMWS regions are not aligned to block size. > On x86, block size increases with capacity (2G blocks @ 64G capacity). > > Use CFMWS base/size to report memory block size alignment advice. > > Suggested-by: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: Gregory Price <gourry@gourry.net> > --- > drivers/acpi/numa/srat.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c > index 44f91f2c6c5d..a24aff38c465 100644 > --- a/drivers/acpi/numa/srat.c > +++ b/drivers/acpi/numa/srat.c > @@ -14,6 +14,7 @@ > #include <linux/errno.h> > #include <linux/acpi.h> > #include <linux/memblock.h> > +#include <linux/memory.h> > #include <linux/numa.h> > #include <linux/nodemask.h> > #include <linux/topology.h> > @@ -338,12 +339,26 @@ static int __init acpi_parse_cfmws(union acpi_subtable_headers *header, > { > struct acpi_cedt_cfmws *cfmws; > int *fake_pxm = arg; > - u64 start, end; > + u64 start, end, align, size; > int node; > > cfmws = (struct acpi_cedt_cfmws *)header; > start = cfmws->base_hpa; > - end = cfmws->base_hpa + cfmws->window_size; > + size = cfmws->window_size; > + end = cfmws->base_hpa + size; > + > + /* Align memblock size to CFMW regions if possible */ > + for (align = SZ_64T; align >= SZ_256M; align >>= 1) { > + if (IS_ALIGNED(start, align) && IS_ALIGNED(size, align)) > + break; > + } Are there maybe some nice tricks bi-tricks to avoid the loop and these hardcoded limits? :) align = 1UL << __ffs(start | end)); Assuming "unsigned long" is sufficient in this code (64bit) and "start | end" will never be 0.
On Wed, Oct 30, 2024 at 11:40:08AM +0100, David Hildenbrand wrote: > On 29.10.24 21:20, Gregory Price wrote: > > Capacity is stranded when CFMWS regions are not aligned to block size. > > On x86, block size increases with capacity (2G blocks @ 64G capacity). > > > > Use CFMWS base/size to report memory block size alignment advice. > > > > Suggested-by: Dan Williams <dan.j.williams@intel.com> > > Signed-off-by: Gregory Price <gourry@gourry.net> > > --- > > drivers/acpi/numa/srat.c | 19 +++++++++++++++++-- > > 1 file changed, 17 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c > > index 44f91f2c6c5d..a24aff38c465 100644 > > --- a/drivers/acpi/numa/srat.c > > +++ b/drivers/acpi/numa/srat.c > > @@ -14,6 +14,7 @@ > > #include <linux/errno.h> > > #include <linux/acpi.h> > > #include <linux/memblock.h> > > +#include <linux/memory.h> > > #include <linux/numa.h> > > #include <linux/nodemask.h> > > #include <linux/topology.h> > > @@ -338,12 +339,26 @@ static int __init acpi_parse_cfmws(union acpi_subtable_headers *header, > > { > > struct acpi_cedt_cfmws *cfmws; > > int *fake_pxm = arg; > > - u64 start, end; > > + u64 start, end, align, size; > > int node; > > cfmws = (struct acpi_cedt_cfmws *)header; > > start = cfmws->base_hpa; > > - end = cfmws->base_hpa + cfmws->window_size; > > + size = cfmws->window_size; > > + end = cfmws->base_hpa + size; > > + > > + /* Align memblock size to CFMW regions if possible */ > > + for (align = SZ_64T; align >= SZ_256M; align >>= 1) { > > + if (IS_ALIGNED(start, align) && IS_ALIGNED(size, align)) > > + break; > > + } > > Are there maybe some nice tricks bi-tricks to avoid the loop and these > hardcoded limits? :) > > align = 1UL << __ffs(start | end)); > > Assuming "unsigned long" is sufficient in this code (64bit) and "start | > end" will never be 0. > I don't think 0 itself is necessarily invalid, but it would be strange. I can look a bit. > -- > Cheers, > > David / dhildenb >
On Wed, Oct 30, 2024 at 11:40:08AM +0100, David Hildenbrand wrote: > On 29.10.24 21:20, Gregory Price wrote: > > Capacity is stranded when CFMWS regions are not aligned to block size. > > On x86, block size increases with capacity (2G blocks @ 64G capacity). > > > > Use CFMWS base/size to report memory block size alignment advice. > > > > Suggested-by: Dan Williams <dan.j.williams@intel.com> > > Signed-off-by: Gregory Price <gourry@gourry.net> > > --- > > drivers/acpi/numa/srat.c | 19 +++++++++++++++++-- > > 1 file changed, 17 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c > > index 44f91f2c6c5d..a24aff38c465 100644 > > --- a/drivers/acpi/numa/srat.c > > +++ b/drivers/acpi/numa/srat.c > > @@ -14,6 +14,7 @@ > > #include <linux/errno.h> > > #include <linux/acpi.h> > > #include <linux/memblock.h> > > +#include <linux/memory.h> > > #include <linux/numa.h> > > #include <linux/nodemask.h> > > #include <linux/topology.h> > > @@ -338,12 +339,26 @@ static int __init acpi_parse_cfmws(union acpi_subtable_headers *header, > > { > > struct acpi_cedt_cfmws *cfmws; > > int *fake_pxm = arg; > > - u64 start, end; > > + u64 start, end, align, size; > > int node; > > cfmws = (struct acpi_cedt_cfmws *)header; > > start = cfmws->base_hpa; > > - end = cfmws->base_hpa + cfmws->window_size; > > + size = cfmws->window_size; > > + end = cfmws->base_hpa + size; > > + > > + /* Align memblock size to CFMW regions if possible */ > > + for (align = SZ_64T; align >= SZ_256M; align >>= 1) { > > + if (IS_ALIGNED(start, align) && IS_ALIGNED(size, align)) > > + break; > > + } > > Are there maybe some nice tricks bi-tricks to avoid the loop and these > hardcoded limits? :) > > align = 1UL << __ffs(start | end)); > > Assuming "unsigned long" is sufficient in this code (64bit) and "start | > end" will never be 0. > This will work, if start | end is < 256MB, the ACPI table is invalid by definition since either the block itself is <256MB or the size is 0 (which is nonsense). So yeah i can simplify here. Ack. will push v5 once i get KLP to clear another warning. > -- > Cheers, > > David / dhildenb >
diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c index 44f91f2c6c5d..a24aff38c465 100644 --- a/drivers/acpi/numa/srat.c +++ b/drivers/acpi/numa/srat.c @@ -14,6 +14,7 @@ #include <linux/errno.h> #include <linux/acpi.h> #include <linux/memblock.h> +#include <linux/memory.h> #include <linux/numa.h> #include <linux/nodemask.h> #include <linux/topology.h> @@ -338,12 +339,26 @@ static int __init acpi_parse_cfmws(union acpi_subtable_headers *header, { struct acpi_cedt_cfmws *cfmws; int *fake_pxm = arg; - u64 start, end; + u64 start, end, align, size; int node; cfmws = (struct acpi_cedt_cfmws *)header; start = cfmws->base_hpa; - end = cfmws->base_hpa + cfmws->window_size; + size = cfmws->window_size; + end = cfmws->base_hpa + size; + + /* Align memblock size to CFMW regions if possible */ + for (align = SZ_64T; align >= SZ_256M; align >>= 1) { + if (IS_ALIGNED(start, align) && IS_ALIGNED(size, align)) + break; + } + + if (align >= SZ_256M) { + if (memory_block_advise_max_size(align) < 0) + pr_warn("CFMWS: memblock size advise failed\n"); + } else { + pr_err("CFMWS: [BIOS BUG] base/size alignment violates spec\n"); + } /* * The SRAT may have already described NUMA details for all,
Capacity is stranded when CFMWS regions are not aligned to block size. On x86, block size increases with capacity (2G blocks @ 64G capacity). Use CFMWS base/size to report memory block size alignment advice. Suggested-by: Dan Williams <dan.j.williams@intel.com> Signed-off-by: Gregory Price <gourry@gourry.net> --- drivers/acpi/numa/srat.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)