Heterogeneuos memory node attributes

Systems may provide different memory types and export this information in the ACPI Heterogeneous Memory Attribute Table (HMAT). Parse these tables provided by the platform and report the memory access and caching attributes.

Signed-off-by Keith Busch <[email protected]>

---
 drivers/acpi/Kconfig       |   1 +
 drivers/acpi/Makefile      |   1 +
 drivers/acpi/hmat/Kconfig  |   8 ++
 drivers/acpi/hmat/Makefile |   1 +
 drivers/acpi/hmat/hmat.c   | 180 +++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 191 insertions(+)
 create mode 100644 drivers/acpi/hmat/Kconfig
 create mode 100644 drivers/acpi/hmat/Makefile
 create mode 100644 drivers/acpi/hmat/hmat.c

drivers/acpi/Kconfig +1 -0

index 90ff0a47c12e..b377f970adfd 100644
@@ -465,6 +465,7 @@ config ACPI_REDUCED_HARDWARE_ONLY
          If you are unsure what to do, do not enable this option.
 
 source "drivers/acpi/nfit/Kconfig"
+source "drivers/acpi/hmat/Kconfig"
 
 source "drivers/acpi/apei/Kconfig"
 source "drivers/acpi/dptf/Kconfig"

drivers/acpi/Makefile +0 -0

index 7c6afc111d76..bff8fbe5a6ab 100644

drivers/acpi/Makefile +0 -0

 obj-$(CONFIG_ACPI)             += acpi_memhotplug.o
 obj-$(CONFIG_ACPI_HOTPLUG_IOAPIC) += ioapic.o
 obj-$(CONFIG_ACPI_BATTERY)     += battery.o

drivers/acpi/hmat/Kconfig new file +0 -0

new file mode 100644
index 000000000000..a4034d37a311

drivers/acpi/hmat/Makefile new file +1 -0

new file mode 100644
index 000000000000..e909051d3d00
@@ -0,0 +1 @@
+obj-$(CONFIG_ACPI_HMAT) := hmat.o

drivers/acpi/hmat/hmat.c new file +0 -0

new file mode 100644
index 000000000000..833a783868d5

drivers/acpi/hmat/hmat.c new file +39 -0

+
+static __init const char *hmat_data_type(u8 type)
+{
+       switch (type) {
+       case ACPI_HMAT_ACCESS_LATENCY:
+               return "Access Latency";
+       case ACPI_HMAT_READ_LATENCY:
+               return "Read Latency";
+       case ACPI_HMAT_WRITE_LATENCY:
+               return "Write Latency";
+       case ACPI_HMAT_ACCESS_BANDWIDTH:
+               return "Access Bandwidth";
+       case ACPI_HMAT_READ_BANDWIDTH:
+               return "Read Bandwidth";
+       case ACPI_HMAT_WRITE_BANDWIDTH:
+               return "Write Bandwidth";
+       default:
+               return "Reserved";
+       };
+}
+
+static __init const char *hmat_data_type_suffix(u8 type)
+{
+       switch (type) {
+       case ACPI_HMAT_ACCESS_LATENCY:
+       case ACPI_HMAT_READ_LATENCY:
+       case ACPI_HMAT_WRITE_LATENCY:
+               return " nsec";
+       case ACPI_HMAT_ACCESS_BANDWIDTH:
+       case ACPI_HMAT_READ_BANDWIDTH:
+       case ACPI_HMAT_WRITE_BANDWIDTH:
+               return " MB/s";
+       default:
+               return "";
+       };
+}
+
+static __init int hmat_parse_locality(union acpi_subtable_headers *header,
+                                     const unsigned long end)

drivers/acpi/hmat/hmat.c new file +22 -0

+               return -EINVAL;
+       }
+
+       type = loc->data_type;
+       ipds = loc->number_of_initiator_Pds;
+       tpds = loc->number_of_target_Pds;
+       total_size = sizeof(*loc) + sizeof(*entries) * ipds * tpds +
+                    sizeof(*inits) * ipds + sizeof(*targs) * tpds;
+       if (loc->header.length < total_size) {
+               pr_err("HMAT: Unexpected locality header length:%d, minimum required:%d\n",
+                       loc->header.length, total_size);
+               return -EINVAL;
+       }
+
+       pr_info("HMAT: Locality: Flags:%02x Type:%s Initiator Domains:%d Target Domains:%d Base:%lld\n",
+               loc->flags, hmat_data_type(type), ipds, tpds,
+               loc->entry_base_unit);
+
+       inits = (u32 *)(loc + 1);
+       targs = &inits[ipds];
+       entries = (u16 *)(&targs[tpds]);
+       for (targ = 0; targ < tpds; targ++) {

drivers/acpi/hmat/hmat.c new file +72 -0

+
+static __init int hmat_parse_cache(union acpi_subtable_headers *header,
+                                  const unsigned long end)
+{
+       struct acpi_hmat_cache *cache = (void *)header;
+       u32 attrs;
+
+       if (cache->header.length < sizeof(*cache)) {
+               pr_err("HMAT: Unexpected cache header length: %d\n",
+                       cache->header.length);
+               return -EINVAL;
+       }
+
+       attrs = cache->cache_attributes;
+       pr_info("HMAT: Cache: Domain:%d Size:%llu Attrs:%08x SMBIOS Handles:%d\n",
+               cache->memory_PD, cache->cache_size, attrs,
+               cache->number_of_SMBIOShandles);
+
+       return 0;
+}
+
+static int __init hmat_parse_address_range(union acpi_subtable_headers *header,
+                                          const unsigned long end)
+{
+       struct acpi_hmat_address_range *spa = (void *)header;
+
+       if (spa->header.length != sizeof(*spa)) {
+               pr_err("HMAT: Unexpected address range header length: %d\n",
+                       spa->header.length);
+               return -EINVAL;
+       }
+       pr_info("HMAT: Memory (%#llx length %#llx) Flags:%04x Processor Domain:%d Memory Domain:%d\n",
+               spa->physical_address_base, spa->physical_address_length,
+               spa->flags, spa->processor_PD, spa->memory_PD);
+       return 0;
+}
+
+static int __init hmat_parse_subtable(union acpi_subtable_headers *header,
+                                     const unsigned long end)
+{
+       struct acpi_hmat_structure *hdr = (void *)header;
+
+       if (!hdr)
+               return -EINVAL;
+
+       switch (hdr->type) {
+       case ACPI_HMAT_TYPE_ADDRESS_RANGE:
+               return hmat_parse_address_range(header, end);
+       case ACPI_HMAT_TYPE_LOCALITY:
+               return hmat_parse_locality(header, end);
+       case ACPI_HMAT_TYPE_CACHE:
+               return hmat_parse_cache(header, end);
+       default:
+               return -EINVAL;
+       }
+}
+
+static __init int hmat_init(void)
+{
+       struct acpi_table_header *tbl;
+       enum acpi_hmat_type i;
+       acpi_status status;
+
+       if (srat_disabled())
+               return 0;
+
+       status = acpi_get_table(ACPI_SIG_HMAT, 0, &tbl);
+       if (ACPI_FAILURE(status))
+               return 0;
+
+       for (i = ACPI_HMAT_TYPE_ADDRESS_RANGE; i < ACPI_HMAT_TYPE_RESERVED; i++) {
+               if (acpi_table_parse_entries(ACPI_SIG_HMAT,

Systems may be constructed with various specialized nodes. Some nodes may provide memory, some provide compute devices that access and use that memory, and others may provide both. Nodes that provide memory are referred to as memory targets, and nodes that can initiate memory access are referred to as memory initiators.

Memory targets will often have varying access characteristics from different initiators, and platforms may have ways to express those relationships. In preparation for these systems, provide interfaces for the kernel to export the memory relationship among different nodes memory targets and their initiators with symlinks to each other's nodes, and export node lists showing the same relationship.

If a system provides access locality for each initiator-target pair, nodes may be grouped into ranked access classes relative to other nodes. The new interface allows a subsystem to register relationships of varying classes if available and desired to be exported. A lower class number indicates a higher performing tier, with 0 being the best performing class.

A memory initiator may have multiple memory targets in the same access class. The initiator's memory targets in given class indicate the node's access characteristics perform better relative to other initiator nodes either unreported or in lower class numbers. The targets within an initiator's class, though, do not necessarily perform the same as each other.

A memory target node may have multiple memory initiators. All linked initiators in a target's class have the same access characteristics to that target.

The following example show the nodes' new sysfs hierarchy for a memory target node 'Y' with class 0 access from initiator node 'X':

  # symlinks -v /sys/devices/system/node/nodeX/class0/
  relative: /sys/devices/system/node/nodeX/class0/targetY -> ../../nodeY

  # symlinks -v /sys/devices/system/node/nodeY/class0/
  relative: /sys/devices/system/node/nodeY/class0/initiatorX -> ../../nodeX

And the same information is reflected in the nodelist:

  # cat /sys/devices/system/node/nodeX/class0/target_nodelist
  Y

  # cat /sys/devices/system/node/nodeY/class0/initiator_nodelist
  X

Signed-off-by Keith Busch <[email protected]>

---
 drivers/base/node.c  | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/node.h |   6 ++-
 2 files changed, 131 insertions(+), 2 deletions(-)

drivers/base/node.c +1 -0

index 86d6cd92ce3d..1da5072116ab 100644
@@ -17,6 +17,7 @@
 #include <linux/nodemask.h>
 #include <linux/cpu.h>
 #include <linux/device.h>
+#include <linux/pm_runtime.h>
 #include <linux/swap.h>
 #include <linux/slab.h>
 

Add entries for memory initiator and target node class attributes.

Signed-off-by Keith Busch <[email protected]>

---
 Documentation/ABI/stable/sysfs-devices-node | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

Documentation/ABI/stable/sysfs-devices-node +6 -1

index 3e90e1f3bf0a..a9c47b4b0eee 100644
@@ -90,4 +90,27 @@ Date:                December 2009
 Contact:       Lee Schermerhorn <[email protected]>
 Description:
                The node's huge page size control/query attributes.
-               See Documentation/admin-guide/mm/hugetlbpage.rst
\ No newline at end of file
+               See Documentation/admin-guide/mm/hugetlbpage.rst
+
+What:          /sys/devices/system/node/nodeX/classY/
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:

Quoted 4 lines · Keith Busch

Add entries for memory initiator and target node class attributes.

Signed-off-by Keith Busch <[email protected]>

I would recommend combining this with the previous patch, as the way it is now I need to look at two patches at the time. :-)

Quoted 27 lines

---
 Documentation/ABI/stable/sysfs-devices-node | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

Documentation/ABI/stable/sysfs-devices-node +13 -1

index 3e90e1f3bf0a..a9c47b4b0eee 100644
@@ -90,4 +90,27 @@ Date:                December 2009
 Contact:       Lee Schermerhorn <[email protected]>
 Description:
                The node's huge page size control/query attributes.
-               See Documentation/admin-guide/mm/hugetlbpage.rst
\ No newline at end of file
+               See Documentation/admin-guide/mm/hugetlbpage.rst
+
+What:          /sys/devices/system/node/nodeX/classY/
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The node's relationship to other nodes for access class "Y".
+
+What:          /sys/devices/system/node/nodeX/classY/initiator_nodelist
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The node list of memory initiators that have class "Y" access

Documentation/ABI/stable/sysfs-devices-node Date: December 2009 · line 108

Quoted 3 lines

Documentation/ABI/stable/sysfs-devices-node +3 -0

+               to this node's memory. CPUs and other memory initiators in
+               nodes not in the list accessing this node's memory may have
+               different performance.

This does not follow the general "one value per file" rule of sysfs (I know that there are other sysfs files with more than one value in them, but it is better to follow this rule as long as that makes sense).

Documentation/ABI/stable/sysfs-devices-node Date: December 2009

Quoted 9 lines

Documentation/ABI/stable/sysfs-devices-node +8 -1

+
+What:          /sys/devices/system/node/nodeX/classY/target_nodelist
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The node list of memory targets that this initiator node has
+               class "Y" access. Memory accesses from this node to nodes not
+               in this list may have differet performance.
--

Same here.

And if you follow the recommendation given in the previous message (add "initiators" and "targets" subdirs under "classX"), you won't even need the two files above.

And, of course, the symlinks part needs to be documented as well. I guess you can follow the Documentation/ABI/testing/sysfs-devices-power_resources_D0 with that.

Quoted 4 lines · Keith Busch

Add entries for memory initiator and target node class attributes.

Signed-off-by Keith Busch <[email protected]>

I would recommend combining this with the previous patch, as the way it is now I need to look at two patches at the time. :-)

Quoted 20 lines

---
 Documentation/ABI/stable/sysfs-devices-node | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

Documentation/ABI/stable/sysfs-devices-node +6 -1

index 3e90e1f3bf0a..a9c47b4b0eee 100644
@@ -90,4 +90,27 @@ Date:                December 2009
 Contact:       Lee Schermerhorn <[email protected]>
 Description:
                The node's huge page size control/query attributes.
-               See Documentation/admin-guide/mm/hugetlbpage.rst
\ No newline at end of file
+               See Documentation/admin-guide/mm/hugetlbpage.rst
+
+What:          /sys/devices/system/node/nodeX/classY/
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:

Documentation/ABI/stable/sysfs-devices-node Date: December 2009 · line 108

Quoted 10 lines

Documentation/ABI/stable/sysfs-devices-node +10 -0

+               The node's relationship to other nodes for access class "Y".
+
+What:          /sys/devices/system/node/nodeX/classY/initiator_nodelist
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The node list of memory initiators that have class "Y" access
+               to this node's memory. CPUs and other memory initiators in
+               nodes not in the list accessing this node's memory may have
+               different performance.

This does not follow the general "one value per file" rule of sysfs (I know that there are other sysfs files with more than one value in them, but it is better to follow this rule as long as that makes sense).

Documentation/ABI/stable/sysfs-devices-node Date: December 2009

Quoted 9 lines

Documentation/ABI/stable/sysfs-devices-node +8 -1

+
+What:          /sys/devices/system/node/nodeX/classY/target_nodelist
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The node list of memory targets that this initiator node has
+               class "Y" access. Memory accesses from this node to nodes not
+               in this list may have differet performance.
--

Same here.

And if you follow the recommendation given in the previous message (add "initiators" and "targets" subdirs under "classX"), you won't even need the two files above.

Quoted 58 lines · Rafael J. Wysocki

Quoted 4 lines · Keith Busch

Add entries for memory initiator and target node class attributes.

Signed-off-by Keith Busch <[email protected]>

I would recommend combining this with the previous patch, as the way it is now I need to look at two patches at the time. :-)

Quoted 20 lines

---
 Documentation/ABI/stable/sysfs-devices-node | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

Documentation/ABI/stable/sysfs-devices-node +6 -1

index 3e90e1f3bf0a..a9c47b4b0eee 100644
@@ -90,4 +90,27 @@ Date:                December 2009
 Contact:       Lee Schermerhorn <[email protected]>
 Description:
                The node's huge page size control/query attributes.
-               See Documentation/admin-guide/mm/hugetlbpage.rst
\ No newline at end of file
+               See Documentation/admin-guide/mm/hugetlbpage.rst
+
+What:          /sys/devices/system/node/nodeX/classY/
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:

Documentation/ABI/stable/sysfs-devices-node Date: December 2009 · line 108

Quoted 10 lines

Documentation/ABI/stable/sysfs-devices-node +10 -0

+               The node's relationship to other nodes for access class "Y".
+
+What:          /sys/devices/system/node/nodeX/classY/initiator_nodelist
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The node list of memory initiators that have class "Y" access
+               to this node's memory. CPUs and other memory initiators in
+               nodes not in the list accessing this node's memory may have
+               different performance.

This does not follow the general "one value per file" rule of sysfs (I know that there are other sysfs files with more than one value in them, but it is better to follow this rule as long as that makes sense).

Documentation/ABI/stable/sysfs-devices-node Date: December 2009

Quoted 9 lines

Documentation/ABI/stable/sysfs-devices-node +8 -1

+
+What:          /sys/devices/system/node/nodeX/classY/target_nodelist
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The node list of memory targets that this initiator node has
+               class "Y" access. Memory accesses from this node to nodes not
+               in this list may have differet performance.
--

Same here.

Quoted 3 lines

And if you follow the recommendation given in the previous message (add "initiators" and "targets" subdirs under "classX"), you won't even need the two files above.

This recommendation is in conflict with Greg's feedback about kobject usage. If these are just "vanity" subdirs I think it's better to have a multi-value sysfs file. This "list" style is already commonplace for the /sys/devices/system hierarchy.

Quoted 59 lines · Dan Williams

Quoted 58 lines · Rafael J. Wysocki

Quoted 4 lines · Keith Busch

Add entries for memory initiator and target node class attributes.

Signed-off-by Keith Busch <[email protected]>

I would recommend combining this with the previous patch, as the way it is now I need to look at two patches at the time. :-)

Quoted 20 lines

---
 Documentation/ABI/stable/sysfs-devices-node | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

Documentation/ABI/stable/sysfs-devices-node +6 -1

index 3e90e1f3bf0a..a9c47b4b0eee 100644
@@ -90,4 +90,27 @@ Date:                December 2009
 Contact:       Lee Schermerhorn <[email protected]>
 Description:
                The node's huge page size control/query attributes.
-               See Documentation/admin-guide/mm/hugetlbpage.rst
\ No newline at end of file
+               See Documentation/admin-guide/mm/hugetlbpage.rst
+
+What:          /sys/devices/system/node/nodeX/classY/
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:

Documentation/ABI/stable/sysfs-devices-node Date: December 2009 · line 108

Quoted 10 lines

Documentation/ABI/stable/sysfs-devices-node +10 -0

+               The node's relationship to other nodes for access class "Y".
+
+What:          /sys/devices/system/node/nodeX/classY/initiator_nodelist
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The node list of memory initiators that have class "Y" access
+               to this node's memory. CPUs and other memory initiators in
+               nodes not in the list accessing this node's memory may have
+               different performance.

This does not follow the general "one value per file" rule of sysfs (I know that there are other sysfs files with more than one value in them, but it is better to follow this rule as long as that makes sense).

Documentation/ABI/stable/sysfs-devices-node Date: December 2009

Quoted 9 lines

Documentation/ABI/stable/sysfs-devices-node +8 -1

+
+What:          /sys/devices/system/node/nodeX/classY/target_nodelist
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The node list of memory targets that this initiator node has
+               class "Y" access. Memory accesses from this node to nodes not
+               in this list may have differet performance.
--

Same here.

Quoted 8 lines

Quoted 3 lines

And if you follow the recommendation given in the previous message (add "initiators" and "targets" subdirs under "classX"), you won't even need the two files above.

This recommendation is in conflict with Greg's feedback about kobject usage. If these are just "vanity" subdirs I think it's better to have a multi-value sysfs file. This "list" style is already commonplace for the /sys/devices/system hierarchy.

If you do a subdirectory "correctly" (i.e. a name for an attribute group), that's fine. Just do not ever create a kobject just for a subdir, that will mess up userspace.

And I hate the "multi-value" sysfs files, where at all possible, please do not copy past bad mistakes there. If you can make them individual files, please do that, it makes it easier to maintain and code the kernel for.

Quoted 70 lines · Greg Kroah-Hartman

Quoted 59 lines · Dan Williams

Quoted 58 lines · Rafael J. Wysocki

Quoted 4 lines · Keith Busch

Add entries for memory initiator and target node class attributes.

Signed-off-by Keith Busch <[email protected]>

I would recommend combining this with the previous patch, as the way it is now I need to look at two patches at the time. :-)

Quoted 20 lines

---
 Documentation/ABI/stable/sysfs-devices-node | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

Documentation/ABI/stable/sysfs-devices-node +6 -1

index 3e90e1f3bf0a..a9c47b4b0eee 100644
@@ -90,4 +90,27 @@ Date:                December 2009
 Contact:       Lee Schermerhorn <[email protected]>
 Description:
                The node's huge page size control/query attributes.
-               See Documentation/admin-guide/mm/hugetlbpage.rst
\ No newline at end of file
+               See Documentation/admin-guide/mm/hugetlbpage.rst
+
+What:          /sys/devices/system/node/nodeX/classY/
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:

Documentation/ABI/stable/sysfs-devices-node Date: December 2009 · line 108

Quoted 10 lines

Documentation/ABI/stable/sysfs-devices-node +10 -0

+               The node's relationship to other nodes for access class "Y".
+
+What:          /sys/devices/system/node/nodeX/classY/initiator_nodelist
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The node list of memory initiators that have class "Y" access
+               to this node's memory. CPUs and other memory initiators in
+               nodes not in the list accessing this node's memory may have
+               different performance.

This does not follow the general "one value per file" rule of sysfs (I know that there are other sysfs files with more than one value in them, but it is better to follow this rule as long as that makes sense).

Documentation/ABI/stable/sysfs-devices-node Date: December 2009

Quoted 9 lines

Documentation/ABI/stable/sysfs-devices-node +8 -1

+
+What:          /sys/devices/system/node/nodeX/classY/target_nodelist
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The node list of memory targets that this initiator node has
+               class "Y" access. Memory accesses from this node to nodes not
+               in this list may have differet performance.
--

Same here.

Quoted 8 lines

Quoted 3 lines

And if you follow the recommendation given in the previous message (add "initiators" and "targets" subdirs under "classX"), you won't even need the two files above.

This recommendation is in conflict with Greg's feedback about kobject usage. If these are just "vanity" subdirs I think it's better to have a multi-value sysfs file. This "list" style is already commonplace for the /sys/devices/system hierarchy.

Quoted 8 lines

If you do a subdirectory "correctly" (i.e. a name for an attribute group), that's fine. Just do not ever create a kobject just for a subdir, that will mess up userspace.

And I hate the "multi-value" sysfs files, where at all possible, please do not copy past bad mistakes there. If you can make them individual files, please do that, it makes it easier to maintain and code the kernel for.

I agree in general about multi-value sysfs, but in this case we're talking about a mask. Masks are a single value. That said I can get on board with calling what 'cpulist' does a design mistake (human readable mask), but otherwise switching to one file per item in the mask is a mess for userspace to consume.

Quoted 82 lines · Dan Williams

Quoted 78 lines · Greg Kroah-Hartman

Quoted 59 lines · Dan Williams

Quoted 58 lines · Rafael J. Wysocki

Quoted 4 lines · Keith Busch

Add entries for memory initiator and target node class attributes.

Signed-off-by Keith Busch <[email protected]>

I would recommend combining this with the previous patch, as the way it is now I need to look at two patches at the time. :-)

Quoted 20 lines

---
 Documentation/ABI/stable/sysfs-devices-node | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

Documentation/ABI/stable/sysfs-devices-node +6 -1

index 3e90e1f3bf0a..a9c47b4b0eee 100644
@@ -90,4 +90,27 @@ Date:                December 2009
 Contact:       Lee Schermerhorn <[email protected]>
 Description:
                The node's huge page size control/query attributes.
-               See Documentation/admin-guide/mm/hugetlbpage.rst
\ No newline at end of file
+               See Documentation/admin-guide/mm/hugetlbpage.rst
+
+What:          /sys/devices/system/node/nodeX/classY/
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:

Documentation/ABI/stable/sysfs-devices-node Date: December 2009 · line 108

Quoted 10 lines

Documentation/ABI/stable/sysfs-devices-node +10 -0

+               The node's relationship to other nodes for access class "Y".
+
+What:          /sys/devices/system/node/nodeX/classY/initiator_nodelist
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The node list of memory initiators that have class "Y" access
+               to this node's memory. CPUs and other memory initiators in
+               nodes not in the list accessing this node's memory may have
+               different performance.

This does not follow the general "one value per file" rule of sysfs (I know that there are other sysfs files with more than one value in them, but it is better to follow this rule as long as that makes sense).

Documentation/ABI/stable/sysfs-devices-node Date: December 2009

Quoted 9 lines

Documentation/ABI/stable/sysfs-devices-node +8 -1

+
+What:          /sys/devices/system/node/nodeX/classY/target_nodelist
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The node list of memory targets that this initiator node has
+               class "Y" access. Memory accesses from this node to nodes not
+               in this list may have differet performance.
--

Same here.

Quoted 8 lines

Quoted 3 lines

And if you follow the recommendation given in the previous message (add "initiators" and "targets" subdirs under "classX"), you won't even need the two files above.

This recommendation is in conflict with Greg's feedback about kobject usage. If these are just "vanity" subdirs I think it's better to have a multi-value sysfs file. This "list" style is already commonplace for the /sys/devices/system hierarchy.

If you do a subdirectory "correctly" (i.e. a name for an attribute group), that's fine. Just do not ever create a kobject just for a subdir, that will mess up userspace.

And I hate the "multi-value" sysfs files, where at all possible, please do not copy past bad mistakes there. If you can make them individual files, please do that, it makes it easier to maintain and code the kernel for.

Quoted 5 lines

I agree in general about multi-value sysfs, but in this case we're talking about a mask. Masks are a single value. That said I can get on board with calling what 'cpulist' does a design mistake (human readable mask), but otherwise switching to one file per item in the mask is a mess for userspace to consume.

Can you please refer to my response to Keith?

If you have "initiators" and "targets" under "classX" and a list of symlinks in each of them, I don't see any kind of a mess here.

Quoted 84 lines · Rafael J. Wysocki

Quoted 82 lines · Dan Williams

Quoted 78 lines · Greg Kroah-Hartman

Quoted 59 lines · Dan Williams

Quoted 58 lines · Rafael J. Wysocki

Quoted 4 lines · Keith Busch

Add entries for memory initiator and target node class attributes.

Signed-off-by Keith Busch <[email protected]>

I would recommend combining this with the previous patch, as the way it is now I need to look at two patches at the time. :-)

Quoted 20 lines

---
 Documentation/ABI/stable/sysfs-devices-node | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

Documentation/ABI/stable/sysfs-devices-node +6 -1

index 3e90e1f3bf0a..a9c47b4b0eee 100644
@@ -90,4 +90,27 @@ Date:                December 2009
 Contact:       Lee Schermerhorn <[email protected]>
 Description:
                The node's huge page size control/query attributes.
-               See Documentation/admin-guide/mm/hugetlbpage.rst
\ No newline at end of file
+               See Documentation/admin-guide/mm/hugetlbpage.rst
+
+What:          /sys/devices/system/node/nodeX/classY/
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:

Documentation/ABI/stable/sysfs-devices-node Date: December 2009 · line 108

Quoted 10 lines

Documentation/ABI/stable/sysfs-devices-node +10 -0

+               The node's relationship to other nodes for access class "Y".
+
+What:          /sys/devices/system/node/nodeX/classY/initiator_nodelist
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The node list of memory initiators that have class "Y" access
+               to this node's memory. CPUs and other memory initiators in
+               nodes not in the list accessing this node's memory may have
+               different performance.

This does not follow the general "one value per file" rule of sysfs (I know that there are other sysfs files with more than one value in them, but it is better to follow this rule as long as that makes sense).

Documentation/ABI/stable/sysfs-devices-node Date: December 2009

Quoted 9 lines

Documentation/ABI/stable/sysfs-devices-node +8 -1

+
+What:          /sys/devices/system/node/nodeX/classY/target_nodelist
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The node list of memory targets that this initiator node has
+               class "Y" access. Memory accesses from this node to nodes not
+               in this list may have differet performance.
--

Same here.

Quoted 8 lines

Quoted 3 lines

And if you follow the recommendation given in the previous message (add "initiators" and "targets" subdirs under "classX"), you won't even need the two files above.

This recommendation is in conflict with Greg's feedback about kobject usage. If these are just "vanity" subdirs I think it's better to have a multi-value sysfs file. This "list" style is already commonplace for the /sys/devices/system hierarchy.

If you do a subdirectory "correctly" (i.e. a name for an attribute group), that's fine. Just do not ever create a kobject just for a subdir, that will mess up userspace.

And I hate the "multi-value" sysfs files, where at all possible, please do not copy past bad mistakes there. If you can make them individual files, please do that, it makes it easier to maintain and code the kernel for.

Quoted 7 lines

Quoted 5 lines

I agree in general about multi-value sysfs, but in this case we're talking about a mask. Masks are a single value. That said I can get on board with calling what 'cpulist' does a design mistake (human readable mask), but otherwise switching to one file per item in the mask is a mess for userspace to consume.

Can you please refer to my response to Keith?

Ah, ok I missed the patch4 comments and was reading this one in isolation... which also bolsters your comment about squashing these two patches together.

Quoted 2 lines

If you have "initiators" and "targets" under "classX" and a list of symlinks in each of them, I don't see any kind of a mess here.

In this instance, I think having symlinks at all is "messy" vs just having a mask. Yes, you're right, if we have the proposed symlinks from patch4 there is no need for these _nodelist attributes, and those symlinks would be better under "initiator" and "target" directories. However, I'm arguing going the other way, just have the 2 mask attributes and no symlinks. The HMAT erodes the concept of "numa nodes" typically being a single digit number space per platform. Given increasing numbers of memory target types and initiator devices its going to be cumbersome to have userspace walk multiple symlinks vs just reading a mask and opening the canonical path for a node directly.

This is also part of the rationale for only emitting one "class" (initiator / target performance profile) by default. There's an N-to-N initiator-target description in the HMAT. When / if we decide emit more classes the more work userspace would need to do to convert directory structures back into data.

Quoted 63 lines · Dan Williams

Quoted 61 lines · Rafael J. Wysocki

Quoted 4 lines · Keith Busch

Add entries for memory initiator and target node class attributes.

Signed-off-by Keith Busch <[email protected]>

I would recommend combining this with the previous patch, as the way it is now I need to look at two patches at the time. :-)

Quoted 20 lines

---
 Documentation/ABI/stable/sysfs-devices-node | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

Documentation/ABI/stable/sysfs-devices-node +6 -1

index 3e90e1f3bf0a..a9c47b4b0eee 100644
@@ -90,4 +90,27 @@ Date:                December 2009
 Contact:       Lee Schermerhorn <[email protected]>
 Description:
                The node's huge page size control/query attributes.
-               See Documentation/admin-guide/mm/hugetlbpage.rst
\ No newline at end of file
+               See Documentation/admin-guide/mm/hugetlbpage.rst
+
+What:          /sys/devices/system/node/nodeX/classY/
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:

Documentation/ABI/stable/sysfs-devices-node Date: December 2009 · line 108

Quoted 10 lines

Documentation/ABI/stable/sysfs-devices-node +10 -0

+               The node's relationship to other nodes for access class "Y".
+
+What:          /sys/devices/system/node/nodeX/classY/initiator_nodelist
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The node list of memory initiators that have class "Y" access
+               to this node's memory. CPUs and other memory initiators in
+               nodes not in the list accessing this node's memory may have
+               different performance.

This does not follow the general "one value per file" rule of sysfs (I know that there are other sysfs files with more than one value in them, but it is better to follow this rule as long as that makes sense).

Documentation/ABI/stable/sysfs-devices-node Date: December 2009

Quoted 9 lines

Documentation/ABI/stable/sysfs-devices-node +8 -1

+
+What:          /sys/devices/system/node/nodeX/classY/target_nodelist
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The node list of memory targets that this initiator node has
+               class "Y" access. Memory accesses from this node to nodes not
+               in this list may have differet performance.
--

Same here.

And if you follow the recommendation given in the previous message (add "initiators" and "targets" subdirs under "classX"), you won't even need the two files above.

Quoted 4 lines

This recommendation is in conflict with Greg's feedback about kobject usage. If these are just "vanity" subdirs I think it's better to have a multi-value sysfs file. This "list" style is already commonplace for the /sys/devices/system hierarchy.

If you do a subdirectory "correctly" (i.e. a name for an attribute group), that's fine.

Quoted 3 lines · Greg Kroah-Hartman

If you do a subdirectory "correctly" (i.e. a name for an attribute group), that's fine.

Yes, that's what I was thinking about: along the lines of the "power" group under device kobjects.

Quoted 3 lines · Rafael J. Wysocki

Quoted 1 line · Greg Kroah-Hartman

Quoted 5 lines

Quoted 2 lines

If you do a subdirectory "correctly" (i.e. a name for an attribute group), that's fine.

Yes, that's what I was thinking about: along the lines of the "power" group under device kobjects.

We can't append symlinks to an attribute group, though.

I'd need to create a lot of struct devices just to get the desired directory hiearchy.

And then each of those "devices" will have their own "power" group, which really doesn't make any sense for what we're trying to show. Is that really the right way to do this, or something else I'm missing?

Quoted 8 lines · Keith Busch

Quoted 6 lines · Rafael J. Wysocki

Quoted 3 lines · Greg Kroah-Hartman

If you do a subdirectory "correctly" (i.e. a name for an attribute group), that's fine.

Quoted 4 lines

Quoted 2 lines

Yes, that's what I was thinking about: along the lines of the "power" group under device kobjects.

We can't append symlinks to an attribute group, though.

That's right, unfortunately.

Add entries for memory initiator and target node class attributes.

Signed-off-by Keith Busch <[email protected]>

---
 Documentation/ABI/stable/sysfs-devices-node | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

Documentation/ABI/stable/sysfs-devices-node +14 -1

index 3e90e1f3bf0a..a9c47b4b0eee 100644
@@ -90,4 +90,27 @@ Date:		December 2009
 Contact:	Lee Schermerhorn <[email protected]>
 Description:
 		The node's huge page size control/query attributes.
-		See Documentation/admin-guide/mm/hugetlbpage.rst
\ No newline at end of file
+		See Documentation/admin-guide/mm/hugetlbpage.rst
+
+What:		/sys/devices/system/node/nodeX/classY/
+Date:		December 2018
+Contact:	Keith Busch <[email protected]>
+Description:
+		The node's relationship to other nodes for access class "Y".
+
+What:		/sys/devices/system/node/nodeX/classY/initiator_nodelist
+Date:		December 2018
+Contact:	Keith Busch <[email protected]>
+Description:
+		The node list of memory initiators that have class "Y" access
+		to this node's memory. CPUs and other memory initiators in

Quoted 31 lines · On Wed, 16 Jan 2019 10:57:56 -0700 Keith Busch <[email protected]> wrote:

Add entries for memory initiator and target node class attributes.

Signed-off-by Keith Busch <[email protected]>

---
 Documentation/ABI/stable/sysfs-devices-node | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

Documentation/ABI/stable/sysfs-devices-node +14 -1

index 3e90e1f3bf0a..a9c47b4b0eee 100644
@@ -90,4 +90,27 @@ Date:              December 2009
 Contact:     Lee Schermerhorn <[email protected]>
 Description:
              The node's huge page size control/query attributes.
-             See Documentation/admin-guide/mm/hugetlbpage.rst
\ No newline at end of file
+             See Documentation/admin-guide/mm/hugetlbpage.rst
+
+What:                /sys/devices/system/node/nodeX/classY/
+Date:                December 2018
+Contact:     Keith Busch <[email protected]>
+Description:
+             The node's relationship to other nodes for access class "Y".
+
+What:                /sys/devices/system/node/nodeX/classY/initiator_nodelist
+Date:                December 2018
+Contact:     Keith Busch <[email protected]>
+Description:
+             The node list of memory initiators that have class "Y" access
+             to this node's memory. CPUs and other memory initiators in

Documentation/ABI/stable/sysfs-devices-node Date: December 2009 · line 116

Quoted 10 lines

Documentation/ABI/stable/sysfs-devices-node +10 -0

+             nodes not in the list accessing this node's memory may have
+             different performance.
+
+What:                /sys/devices/system/node/nodeX/classY/target_nodelist
+Date:                December 2018
+Contact:     Keith Busch <[email protected]>
+Description:
+             The node list of memory targets that this initiator node has
+             class "Y" access. Memory accesses from this node to nodes not
+             in this list may have differet performance.

Different performance from what? In the other thread we established that these target_nodelists are kind of a backwards reference, they all have their characteristics anyway. Perhaps this just needs to say:
"Memory access from this node to these targets may have different performance"?

i.e. Don't make the assumption I did that they should all be the same!

If the HMAT Subsystem Address Range provides a valid processor proximity domain for a memory domain, or a processor domain with the highest performing access exists, register the memory target with that initiator so this relationship will be visible under the node's sysfs directory.

Since HMAT requires valid address ranges have an equivalent SRAT entry, verify each memory target satisfies this requirement.

Signed-off-by Keith Busch <[email protected]>

---
 drivers/acpi/hmat/hmat.c | 143 ++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 136 insertions(+), 7 deletions(-)

drivers/acpi/hmat/hmat.c +0 -0

index 833a783868d5..efb33c74d1a3 100644

drivers/acpi/hmat/hmat.c +28 -0

+{
+       struct memory_target *t;
+
+       list_for_each_entry(t, &targets, node)
+               if (t->memory_pxm == m)
+                       return t;
+       return NULL;
+}
+
+static __init void alloc_memory_target(unsigned int mem_pxm)
+{
+       struct memory_target *t;
+
+       if (pxm_to_node(mem_pxm) == NUMA_NO_NODE)
+               return;
+
+       t = find_mem_target(mem_pxm);
+       if (t)
+               return;
+
+       t = kzalloc(sizeof(*t), GFP_KERNEL);
+       if (!t)
+               return;
+
+       t->memory_pxm = mem_pxm;
+       list_add_tail(&t->node, &targets);
+}
+
 static __init const char *hmat_data_type(u8 type)
 {
        switch (type) {

drivers/acpi/hmat/hmat.c +11 -0

+{
+       switch (type) {
+       case ACPI_HMAT_ACCESS_LATENCY:
+       case ACPI_HMAT_READ_LATENCY:
+       case ACPI_HMAT_WRITE_LATENCY:
+               if (!*best || *best > value)
+                       *best = value;
+               break;
+       case ACPI_HMAT_ACCESS_BANDWIDTH:
+       case ACPI_HMAT_READ_BANDWIDTH:
+       case ACPI_HMAT_WRITE_BANDWIDTH:

drivers/acpi/hmat/hmat.c +24 -7

        struct acpi_hmat_locality *loc = (void *)header;
-       unsigned int init, targ, total_size, ipds, tpds;
+       unsigned int init, targ, pass, p_node, total_size, ipds, tpds;
        u32 *inits, *targs, value;
        u16 *entries;
        u8 type;

@@ -87,12 +143,28 @@ static __init int hmat_parse_locality(union acpi_subtable_headers *header,
        targs = &inits[ipds];
        entries = (u16 *)(&targs[tpds]);
        for (targ = 0; targ < tpds; targ++) {
-               for (init = 0; init < ipds; init++) {
-                       value = entries[init * tpds + targ];
-                       value = (value * loc->entry_base_unit) / 10;
-                       pr_info("  Initiator-Target[%d-%d]:%d%s\n",
-                               inits[init], targs[targ], value,
-                               hmat_data_type_suffix(type));
+               u32 best = 0;
+
+               t = find_mem_target(targs[targ]);
+               for (pass = 0; pass < 2; pass++) {
+                       for (init = 0; init < ipds; init++) {
+                               value = entries[init * tpds + targ];
+                               value = (value * loc->entry_base_unit) / 10;
+
+                               if (!pass) {
+                                       hmat_update_access(type, value, &best);
+                                       pr_info("  Initiator-Target[%d-%d]:%d%s\n",
+                                               inits[init], targs[targ], value,
+                                               hmat_data_type_suffix(type));
+                                       continue;
+                               }
+
+                               if (!t)
+                                       continue;
+                               p_node = pxm_to_node(inits[init]);
+                               if (p_node != NUMA_NO_NODE && value == best)
+                                       set_bit(p_node, t->p_nodes);
+                       }
                }
        }
        return 0;

@@ -122,6 +194,7 @@ static int __init hmat_parse_address_range(union acpi_subtable_headers *header,
                                           const unsigned long end)
 {
        struct acpi_hmat_address_range *spa = (void *)header;
+       struct memory_target *t = NULL;
 
        if (spa->header.length != sizeof(*spa)) {
                pr_err("HMAT: Unexpected address range header length: %d\n",

drivers/acpi/hmat/hmat.c +31 -0

+                       return -EINVAL;
+               }
+               set_bit(p_node, t->p_nodes);
+       }
        return 0;
 }
 

@@ -154,6 +244,33 @@ static int __init hmat_parse_subtable(union acpi_subtable_headers *header,
        }
 }
 
+static __init int srat_parse_mem_affinity(union acpi_subtable_headers *header,
+                                         const unsigned long end)
+{
+       struct acpi_srat_mem_affinity *ma = (void *)header;
+
+       if (!ma)
+               return -EINVAL;
+       if (!(ma->flags & ACPI_SRAT_MEM_ENABLED))
+               return 0;
+       alloc_memory_target(ma->proximity_domain);
+       return 0;
+}
+
+static __init void hmat_register_targets(void)
+{
+       struct memory_target *t, *next;
+       unsigned m, p;
+
+       list_for_each_entry_safe(t, next, &targets, node) {
+               list_del(&t->node);
+               m = pxm_to_node(t->memory_pxm);
+               for_each_set_bit(p, t->p_nodes, MAX_NUMNODES)
+                       register_memory_node_under_compute_node(m, p, 0);
+               kfree(t);
+       }
+}
+
 static __init int hmat_init(void)
 {
        struct acpi_table_header *tbl;

@@ -163,6 +280,17 @@ static __init int hmat_init(void)
        if (srat_disabled())

drivers/acpi/hmat/hmat.c +4 -1

+               goto out_put;
+       acpi_put_table(tbl);
+
        status = acpi_get_table(ACPI_SIG_HMAT, 0, &tbl);
        if (ACPI_FAILURE(status))
                return 0;

@@ -173,6 +301,7 @@ static __init int hmat_init(void)
                                             hmat_parse_subtable, 0) < 0)
                        goto out_put;
        }
+       hmat_register_targets();
 out_put:
        acpi_put_table(tbl);
        return 0;
--

Quoted 6 lines · Keith Busch

If the HMAT Subsystem Address Range provides a valid processor proximity domain for a memory domain, or a processor domain with the highest performing access exists, register the memory target with that initiator so this relationship will be visible under the node's sysfs directory.

Quoted 2 lines

Since HMAT requires valid address ranges have an equivalent SRAT entry, verify each memory target satisfies this requirement.

What exactly will happen after this patch?

There will be some new directories under /sys/devices/system/node/nodeX/ if all goes well. Anything else?

Heterogeneous memory systems provide memory nodes with different latency and bandwidth performance attributes. Provide a new kernel interface for subsystems to register the attributes under the memory target node's initiator access class. If the system provides this information, applications may query these attributes when deciding which node to request memory.

The following example shows the new sysfs hierarchy for a node exporting performance attributes:

  # tree -P "read*|write*" /sys/devices/system/node/nodeY/classZ/
  /sys/devices/system/node/nodeY/classZ/
  |-- read_bandwidth
  |-- read_latency
  |-- write_bandwidth
  `-- write_latency

The bandwidth is exported as MB/s and latency is reported in nanoseconds. Memory accesses from an initiator node that is not one of the memory's class "Z" initiator nodes may encounter different performance than reported here. When a subsystem makes use of this interface, initiators of a lower class number, "Z", have better performance relative to higher class numbers. When provided, class 0 is the highest performing access class.

Signed-off-by Keith Busch <[email protected]>

---
 drivers/base/Kconfig |  8 ++++++++
 drivers/base/node.c  | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/node.h | 25 +++++++++++++++++++++++++
 3 files changed, 81 insertions(+)

drivers/base/Kconfig +0 -0

index 3e63a900b330..6014980238e8 100644

drivers/base/Kconfig +1 -0

+
 source "drivers/base/test/Kconfig"
 
 config SYS_HYPERVISOR

drivers/base/node.c +17 -0

index 1da5072116ab..1e909f61e8b1 100644
@@ -66,6 +66,9 @@ struct node_class_nodes {
        unsigned                class;
        nodemask_t              initiator_nodes;
        nodemask_t              target_nodes;
+#ifdef CONFIG_HMEM_REPORTING
+       struct node_hmem_attrs  hmem_attrs;
+#endif
 };
 #define to_class_nodes(dev) container_of(dev, struct node_class_nodes, dev)
 

@@ -145,6 +148,51 @@ static struct node_class_nodes *node_init_node_class(struct device *parent,
        return NULL;
 }
 
+#ifdef CONFIG_HMEM_REPORTING
+#define ACCESS_ATTR(name)                                                 \
+static ssize_t name##_show(struct device *dev,                            \
+                          struct device_attribute *attr,                  \
+                          char *buf)                                      \
+{                                                                         \
+       return sprintf(buf, "%u\n", to_class_nodes(dev)->hmem_attrs.name); \
+}                                                                         \
+static DEVICE_ATTR_RO(name);
+
+ACCESS_ATTR(read_bandwidth)
+ACCESS_ATTR(read_latency)
+ACCESS_ATTR(write_bandwidth)
+ACCESS_ATTR(write_latency)

drivers/base/node.c +21 -0

+void node_set_perf_attrs(unsigned int nid, struct node_hmem_attrs *hmem_attrs,
+                        unsigned class)
+{
+       struct node_class_nodes *c;
+       struct node *node;
+
+       if (WARN_ON_ONCE(!node_online(nid)))
+               return;
+
+       node = node_devices[nid];
+       c = node_init_node_class(&node->dev, &node->class_list, class);
+       if (!c)
+               return;
+
+       c->hmem_attrs = *hmem_attrs;
+       if (sysfs_create_groups(&c->dev.kobj, access_groups))
+               pr_info("failed to add performance attribute group to node %d\n",
+                       nid);
+}
+#endif
+
 #define K(x) ((x) << (PAGE_SHIFT - 10))
 static ssize_t node_read_meminfo(struct device *dev,
                        struct device_attribute *attr, char *buf)

include/linux/node.h +13 -0

index 8e3666c12ef2..e22940a593c2 100644
@@ -20,6 +20,31 @@
 #include <linux/list.h>
 #include <linux/workqueue.h>
 
+#ifdef CONFIG_HMEM_REPORTING
+/**
+ * struct node_hmem_attrs - heterogeneous memory performance attributes
+ *
+ * @read_bandwidth:    Read bandwidth in MB/s
+ * @write_bandwidth:   Write bandwidth in MB/s
+ * @read_latency:      Read latency in nanoseconds
+ * @write_latency:     Write latency in nanoseconds
+ */
+struct node_hmem_attrs {
+       unsigned int read_bandwidth;
+       unsigned int write_bandwidth;
+       unsigned int read_latency;

include/linux/node.h +2 -1

+#endif
+
 struct node {
        struct device   dev;
        struct list_head class_list;
--

Quoted 40 lines · Keith Busch

Heterogeneous memory systems provide memory nodes with different latency and bandwidth performance attributes. Provide a new kernel interface for subsystems to register the attributes under the memory target node's initiator access class. If the system provides this information, applications may query these attributes when deciding which node to request memory.

The following example shows the new sysfs hierarchy for a node exporting performance attributes:

  # tree -P "read*|write*" /sys/devices/system/node/nodeY/classZ/
  /sys/devices/system/node/nodeY/classZ/
  |-- read_bandwidth
  |-- read_latency
  |-- write_bandwidth
  `-- write_latency

The bandwidth is exported as MB/s and latency is reported in nanoseconds. Memory accesses from an initiator node that is not one of the memory's class "Z" initiator nodes may encounter different performance than reported here. When a subsystem makes use of this interface, initiators of a lower class number, "Z", have better performance relative to higher class numbers. When provided, class 0 is the highest performing access class.

Signed-off-by Keith Busch <[email protected]>

---
 drivers/base/Kconfig |  8 ++++++++
 drivers/base/node.c  | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/node.h | 25 +++++++++++++++++++++++++
 3 files changed, 81 insertions(+)

drivers/base/Kconfig +0 -0

index 3e63a900b330..6014980238e8 100644
@@ -149,6 +149,14 @@ config DEBUG_TEST_DRIVER_REMOVE
          unusable. You should say N here unless you are explicitly looking to
          test this functionality.
 

drivers/base/Kconfig config DEBUG_TEST_DRIVER_REMOVE · line 154

Quoted 3 lines

drivers/base/Kconfig +3 -0

+config HMEM_REPORTING
+       bool
+       default y

Add descriptions for memory class initiator performance access attributes.

Signed-off-by Keith Busch <[email protected]>

---
 Documentation/ABI/stable/sysfs-devices-node | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

Documentation/ABI/stable/sysfs-devices-node +4 -0

index a9c47b4b0eee..2217557f29d3 100644
@@ -114,3 +114,31 @@ Description:
                The node list of memory targets that this initiator node has
                class "Y" access. Memory accesses from this node to nodes not
                in this list may have differet performance.
+
+What:          /sys/devices/system/node/nodeX/classY/read_bandwidth
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>

Documentation/ABI/stable/sysfs-devices-node +5 -0

+
+What:          /sys/devices/system/node/nodeX/classY/write_bandwidth
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:

Save the best performace access attributes and register these with the memory's node if HMAT provides the locality table. While HMAT does make it possible to know performance for all possible initiator-target pairings, we export only the best pairings at this time.

Signed-off-by Keith Busch <[email protected]>

---
 drivers/acpi/hmat/Kconfig |  1 +
 drivers/acpi/hmat/hmat.c  | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

drivers/acpi/hmat/Kconfig +0 -0

index a4034d37a311..20a0e96ba58a 100644

drivers/acpi/hmat/Kconfig +0 -0

        help
         Parses representation of the ACPI Heterogeneous Memory Attributes
         Table (HMAT) and set the memory node relationships and access

drivers/acpi/hmat/hmat.c +18 -0

index efb33c74d1a3..45e20dc677f9 100644
@@ -23,6 +23,8 @@ struct memory_target {
        struct list_head node;
        unsigned int memory_pxm;
        unsigned long p_nodes[BITS_TO_LONGS(MAX_NUMNODES)];
+       bool hmem_valid;
+       struct node_hmem_attrs hmem;
 };
 
 static __init struct memory_target *find_mem_target(unsigned int m)

@@ -108,6 +110,34 @@ static __init void hmat_update_access(u8 type, u32 value, u32 *best)
        }
 }
 
+static __init void hmat_update_target(struct memory_target *t, u8 type,
+                                     u32 value)
+{
+       switch (type) {
+       case ACPI_HMAT_ACCESS_LATENCY:
+               t->hmem.read_latency = value;
+               t->hmem.write_latency = value;
+               break;
+       case ACPI_HMAT_READ_LATENCY:
+               t->hmem.read_latency = value;
+               break;
+       case ACPI_HMAT_WRITE_LATENCY:
+               t->hmem.write_latency = value;
+               break;
+       case ACPI_HMAT_ACCESS_BANDWIDTH:
+               t->hmem.read_bandwidth = value;

drivers/acpi/hmat/hmat.c +6 -1

+}
+
 static __init int hmat_parse_locality(union acpi_subtable_headers *header,
                                      const unsigned long end)
 {

@@ -166,6 +196,8 @@ static __init int hmat_parse_locality(union acpi_subtable_headers *header,
                                        set_bit(p_node, t->p_nodes);
                        }
                }
+               if (t && best)
+                       hmat_update_target(t, type, best);
        }
        return 0;
 }

@@ -267,6 +299,8 @@ static __init void hmat_register_targets(void)
                m = pxm_to_node(t->memory_pxm);
                for_each_set_bit(p, t->p_nodes, MAX_NUMNODES)
                        register_memory_node_under_compute_node(m, p, 0);
+               if (t->hmem_valid)
+                       node_set_perf_attrs(m, &t->hmem, 0);
                kfree(t);
        }
 }
--

System memory may have side caches to help improve access speed to frequently requested address ranges. While the system provided cache is transparent to the software accessing these memory ranges, applications can optimize their own access based on cache attributes.

Provide a new API for the kernel to register these memory side caches under the memory node that provides it.

The new sysfs representation is modeled from the existing cpu cacheinfo attributes, as seen from /sys/devices/system/cpu/cpuX/side_cache/. Unlike CPU cacheinfo, though, the node cache level is reported from the view of the memory. A higher number is nearer to the CPU, while lower levels are closer to the backing memory. Also unlike CPU cache, it is assumed the system will handle flushing any dirty cached memory to the last level on a power failure if the range is persistent memory.

The attributes we export are the cache size, the line size, associativity, and write back policy.

Signed-off-by Keith Busch <[email protected]>

---
 drivers/base/node.c  | 142 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/node.h |  39 ++++++++++++++
 2 files changed, 181 insertions(+)

drivers/base/node.c +53 -0

index 1e909f61e8b1..7ff3ed566d7d 100644
@@ -191,6 +191,146 @@ void node_set_perf_attrs(unsigned int nid, struct node_hmem_attrs *hmem_attrs,
                pr_info("failed to add performance attribute group to node %d\n",
                        nid);
 }
+
+struct node_cache_info {
+       struct device dev;
+       struct list_head node;
+       struct node_cache_attrs cache_attrs;
+};
+#define to_cache_info(device) container_of(device, struct node_cache_info, dev)
+
+#define CACHE_ATTR(name, fmt)                                          \
+static ssize_t name##_show(struct device *dev,                         \
+                          struct device_attribute *attr,               \
+                          char *buf)                                   \
+{                                                                      \
+       return sprintf(buf, fmt "\n", to_cache_info(dev)->cache_attrs.name);\
+}                                                                      \
+DEVICE_ATTR_RO(name);
+
+CACHE_ATTR(size, "%llu")
+CACHE_ATTR(level, "%u")
+CACHE_ATTR(line_size, "%u")
+CACHE_ATTR(associativity, "%u")
+CACHE_ATTR(write_policy, "%u")
+
+static struct attribute *cache_attrs[] = {
+       &dev_attr_level.attr,
+       &dev_attr_associativity.attr,
+       &dev_attr_size.attr,
+       &dev_attr_line_size.attr,
+       &dev_attr_write_policy.attr,
+       NULL,
+};
+ATTRIBUTE_GROUPS(cache);
+
+static void node_cache_release(struct device *dev)
+{
+       kfree(dev);
+}
+
+static void node_cacheinfo_release(struct device *dev)
+{
+       struct node_cache_info *info = to_cache_info(dev);
+       kfree(info);
+}
+
+static void node_init_cache_dev(struct node *node)
+{
+       struct device *dev;
+
+       dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+       if (!dev)
+               return;
+
+       dev->parent = &node->dev;

drivers/base/node.c +11 -0

+free_name:
+       kfree_const(dev->kobj.name);
+free_dev:
+       kfree(dev);
+}
+
+void node_add_cache(unsigned int nid, struct node_cache_attrs *cache_attrs)
+{
+       struct node_cache_info *info;
+       struct device *dev;
+       struct node *node;

drivers/base/node.c +19 -0

+                       return;
+               }
+       }
+
+       if (!node->cache_dev)
+               node_init_cache_dev(node);
+       if (!node->cache_dev)
+               return;
+
+       info = kzalloc(sizeof(*info), GFP_KERNEL);
+       if (!info)
+               return;
+
+       dev = &info->dev;
+       dev->parent = node->cache_dev;
+       dev->release = node_cacheinfo_release;
+       dev->groups = cache_groups;
+       if (dev_set_name(dev, "index%d", cache_attrs->level))
+               goto free_cache;

drivers/base/node.c +29 -0

+free_name:
+       kfree_const(dev->kobj.name);
+free_cache:
+       kfree(info);
+}
+
+static void node_remove_caches(struct node *node)
+{
+       struct node_cache_info *info, *next;
+
+       if (!node->cache_dev)
+               return;
+
+       list_for_each_entry_safe(info, next, &node->cache_attrs, node) {
+               list_del(&info->node);
+               device_unregister(&info->dev);
+       }
+       device_unregister(node->cache_dev);
+}
+
+static void node_init_caches(unsigned int nid)
+{
+       INIT_LIST_HEAD(&node_devices[nid]->cache_attrs);
+}
+#else
+static void node_init_caches(unsigned int nid) { }
+static void node_remove_caches(struct node *node) { }
 #endif
 
 #define K(x) ((x) << (PAGE_SHIFT - 10))

@@ -475,6 +615,7 @@ void unregister_node(struct node *node)
 {
        hugetlb_unregister_node(node);          /* no-op, if memoryless node */
        node_remove_classes(node);
+       node_remove_caches(node);
        device_unregister(&node->dev);
 }
 

@@ -755,6 +896,7 @@ int __register_one_node(int nid)
        INIT_LIST_HEAD(&node_devices[nid]->class_list);
        /* initialize work queue for memory hot plug */
        init_node_hugetlb_work(nid);
+       node_init_caches(nid);
 
        return error;
 }

include/linux/node.h +30 -0

index e22940a593c2..8cdf2b2808e4 100644
@@ -37,12 +37,47 @@ struct node_hmem_attrs {
 };
 void node_set_perf_attrs(unsigned int nid, struct node_hmem_attrs *hmem_attrs,
                         unsigned class);
+
+enum cache_associativity {
+       NODE_CACHE_DIRECT_MAP,
+       NODE_CACHE_INDEXED,
+       NODE_CACHE_OTHER,
+};
+
+enum cache_write_policy {
+       NODE_CACHE_WRITE_BACK,
+       NODE_CACHE_WRITE_THROUGH,
+       NODE_CACHE_WRITE_OTHER,
+};
+
+/**
+ * struct node_cache_attrs - system memory caching attributes
+ *
+ * @associativity:     The ways memory blocks may be placed in cache
+ * @write_policy:      Write back or write through policy
+ * @size:              Total size of cache in bytes
+ * @line_size:         Number of bytes fetched on a cache miss
+ * @level:             Represents the cache hierarchy level
+ */
+struct node_cache_attrs {
+       enum cache_associativity associativity;
+       enum cache_write_policy write_policy;
+       u64 size;
+       u16 line_size;
+       u8  level;
+};
+void node_add_cache(unsigned int nid, struct node_cache_attrs *cache_attrs);
 #else

include/linux/node.h +4 -1

 #endif
 
 struct node {

@@ -51,6 +86,10 @@ struct node {
 #if defined(CONFIG_MEMORY_HOTPLUG_SPARSE) && defined(CONFIG_HUGETLBFS)
        struct work_struct      node_work;
 #endif
+#ifdef CONFIG_HMEM_REPORTING
+       struct list_head cache_attrs;
+       struct device *cache_dev;
+#endif
 };
 
 struct memory_block;
--