mbox series

[RFC,0/3] Apply SRAT defined PXM to entire CFMWS

Message ID cover.1683742429.git.alison.schofield@intel.com
Headers show
Series Apply SRAT defined PXM to entire CFMWS | expand

Message

Alison Schofield May 10, 2023, 6:44 p.m. UTC
From: Alison Schofield <alison.schofield@intel.com>

The patch that created fake NUMA nodes for CFMWS windows
not defined in the SRAT, made wrong assumptions about the
SRAT defined entries. Specifically, it assumed an SRAT entry
using cfmws->start, uses the entire CMFWS range, start through
end. The assumption is wrong, and so the ACPI driver, needs
to examine the SRAT created memblks more closely to discover
partial definitions of the HPA range.

This work-in-progress addresses that issue. The first 2 patches
introduce numa helpers that are used in the 3rd patch, where the
ACPI drivers parsing of the CFMWS is updated.

The patch commit logs, especially Patch 3, describes more
of the approach as well as other approaches considered, and
questions. So, perhaps, scan 1 & 2, and dive into #3 and 
confirm or refute this approach.

I did not include our NUMA or ACPI friends in this posting,
because I want to get a direction check from CXL folks before
addressing how the helpers can get merged into the NUMA arch.

Thanks for looking!

Alison Schofield (3):
  x86/numa: Introduce numa_find_node(start, end)
  x86/numa: Introduce numa_remove_memblks(node, start, end)
  ACPI: NUMA: Apply SRAT proximity domain to entire CFMWS window

 arch/x86/include/asm/numa.h |  2 ++
 arch/x86/mm/numa.c          | 36 ++++++++++++++++++++++++++++++++++++
 drivers/acpi/numa/srat.c    | 32 ++++++++++++++++++++++++++------
 3 files changed, 64 insertions(+), 6 deletions(-)

Comments

Dan Williams May 11, 2023, 10:42 p.m. UTC | #1
alison.schofield@ wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> The patch that created fake NUMA nodes for CFMWS windows

"fake" seems the wrong word to me. These are first class Linux NUMA
nodes in the end. So to me it's the "patch that extended SRAT proximity
domains by the potential performance-class windows in the CFMWS", or
something like that.

> not defined in the SRAT, made wrong assumptions about the
> SRAT defined entries. Specifically, it assumed an SRAT entry
> using cfmws->start, uses the entire CMFWS range, start through
> end. The assumption is wrong, and so the ACPI driver, needs

I also wouldn't say it was wrong, just incomplete. I.e. this is just
extending the heuristic to say that if the BIOS describes *any* portion
of a CFMWS window with a proximity domain, might as well reuse that
proximity domain for the entire window since everything in that window
is expected to be of a similar performance class.

> to examine the SRAT created memblks more closely to discover
> partial definitions of the HPA range.
> 
> This work-in-progress addresses that issue. The first 2 patches
> introduce numa helpers that are used in the 3rd patch, where the
> ACPI drivers parsing of the CFMWS is updated.
> 
> The patch commit logs, especially Patch 3, describes more
> of the approach as well as other approaches considered, and
> questions. So, perhaps, scan 1 & 2, and dive into #3 and 
> confirm or refute this approach.
> 
> I did not include our NUMA or ACPI friends in this posting,
> because I want to get a direction check from CXL folks before
> addressing how the helpers can get merged into the NUMA arch.
> 
> Thanks for looking!
> 
> Alison Schofield (3):
>   x86/numa: Introduce numa_find_node(start, end)
>   x86/numa: Introduce numa_remove_memblks(node, start, end)
>   ACPI: NUMA: Apply SRAT proximity domain to entire CFMWS window
> 
>  arch/x86/include/asm/numa.h |  2 ++
>  arch/x86/mm/numa.c          | 36 ++++++++++++++++++++++++++++++++++++
>  drivers/acpi/numa/srat.c    | 32 ++++++++++++++++++++++++++------
>  3 files changed, 64 insertions(+), 6 deletions(-)
> 
> -- 
> 2.37.3
>
Jonathan Cameron May 12, 2023, 4:33 p.m. UTC | #2
On Thu, 11 May 2023 15:42:39 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> alison.schofield@ wrote:
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> > The patch that created fake NUMA nodes for CFMWS windows  
> 
> "fake" seems the wrong word to me. These are first class Linux NUMA
> nodes in the end. So to me it's the "patch that extended SRAT proximity
> domains by the potential performance-class windows in the CFMWS", or
> something like that.
> 
> > not defined in the SRAT, made wrong assumptions about the
> > SRAT defined entries. Specifically, it assumed an SRAT entry
> > using cfmws->start, uses the entire CMFWS range, start through
> > end. The assumption is wrong, and so the ACPI driver, needs  
> 
> I also wouldn't say it was wrong, just incomplete. I.e. this is just
> extending the heuristic to say that if the BIOS describes *any* portion
> of a CFMWS window with a proximity domain, might as well reuse that
> proximity domain for the entire window since everything in that window
> is expected to be of a similar performance class.

Some strong assumptions in this statement.

A platform might not care about QTG groups because it's not doing any fancy
QOS stuff (or at least not enough of them to cover 'similar' performance).
At that point, you could have just a few CFMWS with a wide range
of different characteristics.  At somepoint I think we'll need to do
something more sophisticated to cover that case.  Maybe not yet though.
The moment we have platforms doing interleave or not in the host bridges
we will get very different bandwidth at least, potentially to different
parts of identical memory on the same device.  What fun ;)

Jonathan

> 
> > to examine the SRAT created memblks more closely to discover
> > partial definitions of the HPA range.
> > 
> > This work-in-progress addresses that issue. The first 2 patches
> > introduce numa helpers that are used in the 3rd patch, where the
> > ACPI drivers parsing of the CFMWS is updated.
> > 
> > The patch commit logs, especially Patch 3, describes more
> > of the approach as well as other approaches considered, and
> > questions. So, perhaps, scan 1 & 2, and dive into #3 and 
> > confirm or refute this approach.
> > 
> > I did not include our NUMA or ACPI friends in this posting,
> > because I want to get a direction check from CXL folks before
> > addressing how the helpers can get merged into the NUMA arch.
> > 
> > Thanks for looking!
> > 
> > Alison Schofield (3):
> >   x86/numa: Introduce numa_find_node(start, end)
> >   x86/numa: Introduce numa_remove_memblks(node, start, end)
> >   ACPI: NUMA: Apply SRAT proximity domain to entire CFMWS window
> > 
> >  arch/x86/include/asm/numa.h |  2 ++
> >  arch/x86/mm/numa.c          | 36 ++++++++++++++++++++++++++++++++++++
> >  drivers/acpi/numa/srat.c    | 32 ++++++++++++++++++++++++++------
> >  3 files changed, 64 insertions(+), 6 deletions(-)
> > 
> > -- 
> > 2.37.3
> >   
> 
>