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 to the kernel messages.

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   | 181 +++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 192 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 +1 -0

index bb857421c2e8..5d361e4e3405 100644
@@ -80,6 +80,7 @@ obj-$(CONFIG_ACPI_PROCESSOR)  += processor.o
 obj-$(CONFIG_ACPI)             += container.o
 obj-$(CONFIG_ACPI_THERMAL)     += thermal.o
 obj-$(CONFIG_ACPI_NFIT)                += nfit/
+obj-$(CONFIG_ACPI_HMAT)                += hmat/
 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 +8 -0

new file mode 100644
index 000000000000..c9637e2e7514
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0
+config ACPI_HMAT
+       bool "ACPI Heterogeneous Memory Attribute Table Support"
+       depends on ACPI_NUMA
+       help
+        If set, this option causes the kernel to set the memory NUMA node
+        relationships and access attributes in accordance with ACPI HMAT
+        (Heterogeneous Memory Attributes Table).

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 +181 -1

new file mode 100644
index 000000000000..1741bf30d87f
@@ -0,0 +1,181 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019, Intel Corporation.
+ *
+ * Heterogeneous Memory Attributes Table (HMAT) representation
+ *
+ * This program parses and reports the platform's HMAT tables, and registers
+ * the applicable attributes with the node's interfaces.
+ */
+
+#include <linux/acpi.h>
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/node.h>
+#include <linux/sysfs.h>
+
+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)
+{
+       struct acpi_hmat_locality *hmat_loc = (void *)header;
+       unsigned int init, targ, total_size, ipds, tpds;
+       u32 *inits, *targs, value;
+       u16 *entries;
+       u8 type;
+
+       if (hmat_loc->header.length < sizeof(*hmat_loc)) {
+               pr_debug("HMAT: Unexpected locality header length: %d\n",
+                        hmat_loc->header.length);
+               return -EINVAL;
+       }
+
+       type = hmat_loc->data_type;
+       ipds = hmat_loc->number_of_initiator_Pds;
+       tpds = hmat_loc->number_of_target_Pds;
+       total_size = sizeof(*hmat_loc) + sizeof(*entries) * ipds * tpds +
+                    sizeof(*inits) * ipds + sizeof(*targs) * tpds;
+       if (hmat_loc->header.length < total_size) {
+               pr_debug("HMAT: Unexpected locality header length:%d, minimum required:%d\n",
+                        hmat_loc->header.length, total_size);
+               return -EINVAL;
+       }
+
+       pr_info("HMAT: Locality: Flags:%02x Type:%s Initiator Domains:%d Target Domains:%d Base:%lld\n",
+               hmat_loc->flags, hmat_data_type(type), ipds, tpds,
+               hmat_loc->entry_base_unit);
+
+       inits = (u32 *)(hmat_loc + 1);
+       targs = &inits[ipds];
+       entries = (u16 *)(&targs[tpds]);
+       for (init = 0; init < ipds; init++) {
+               for (targ = 0; targ < tpds; targ++) {
+                       value = entries[init * tpds + targ];
+                       value = (value * hmat_loc->entry_base_unit) / 10;
+                       pr_info("  Initiator-Target[%d-%d]:%d%s\n",
+                               inits[init], targs[targ], value,
+                               hmat_data_type_suffix(type));
+               }
+       }
+
+       return 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_debug("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_debug("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,
+                                            sizeof(struct acpi_table_hmat), i,
+                                            hmat_parse_subtable, 0) < 0)
+                       goto out_put;
+       }
+out_put:
+       acpi_put_table(tbl);
+       return 0;
+}
+subsys_initcall(hmat_init);
--

2.14.4

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 to the kernel messages.

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   | 181 +++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 192 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 +1 -0

index bb857421c2e8..5d361e4e3405 100644
@@ -80,6 +80,7 @@ obj-$(CONFIG_ACPI_PROCESSOR)	+= processor.o
 obj-$(CONFIG_ACPI)		+= container.o
 obj-$(CONFIG_ACPI_THERMAL)	+= thermal.o
 obj-$(CONFIG_ACPI_NFIT)		+= nfit/
+obj-$(CONFIG_ACPI_HMAT)		+= hmat/
 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 +8 -0

new file mode 100644
index 000000000000..c9637e2e7514
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0
+config ACPI_HMAT
+	bool "ACPI Heterogeneous Memory Attribute Table Support"
+	depends on ACPI_NUMA
+	help
+	 If set, this option causes the kernel to set the memory NUMA node
+	 relationships and access attributes in accordance with ACPI HMAT
+	 (Heterogeneous Memory Attributes Table).

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 +76 -0

new file mode 100644
index 000000000000..1741bf30d87f
@@ -0,0 +1,181 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019, Intel Corporation.
+ *
+ * Heterogeneous Memory Attributes Table (HMAT) representation
+ *
+ * This program parses and reports the platform's HMAT tables, and registers
+ * the applicable attributes with the node's interfaces.
+ */
+
+#include <linux/acpi.h>
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/list.h>
+#include <linux/node.h>
+#include <linux/sysfs.h>
+
+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)
+{
+	struct acpi_hmat_locality *hmat_loc = (void *)header;
+	unsigned int init, targ, total_size, ipds, tpds;
+	u32 *inits, *targs, value;
+	u16 *entries;
+	u8 type;
+
+	if (hmat_loc->header.length < sizeof(*hmat_loc)) {
+		pr_debug("HMAT: Unexpected locality header length: %d\n",
+			 hmat_loc->header.length);
+		return -EINVAL;
+	}
+
+	type = hmat_loc->data_type;
+	ipds = hmat_loc->number_of_initiator_Pds;
+	tpds = hmat_loc->number_of_target_Pds;
+	total_size = sizeof(*hmat_loc) + sizeof(*entries) * ipds * tpds +
+		     sizeof(*inits) * ipds + sizeof(*targs) * tpds;
+	if (hmat_loc->header.length < total_size) {
+		pr_debug("HMAT: Unexpected locality header length:%d, minimum required:%d\n",

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

+		}
+	}
+
+	return 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;
+

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

+		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,
+					     sizeof(struct acpi_table_hmat), i,
+					     hmat_parse_subtable, 0) < 0)
+			goto out_put;
+	}
+out_put:
+	acpi_put_table(tbl);
+	return 0;
+}
+subsys_initcall(hmat_init);

drivers/acpi/hmat/hmat.c line 86

Quoted 6 lines · On Thu, 24 Jan 2019 16:07:17 -0700 Keith Busch <[email protected]> wrote:

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

+	pr_info("HMAT: Locality: Flags:%02x Type:%s Initiator Domains:%d Target Domains:%d Base:%lld\n",
+		hmat_loc->flags, hmat_data_type(type), ipds, tpds,
+		hmat_loc->entry_base_unit);
+
+	inits = (u32 *)(hmat_loc + 1);
+	targs = &inits[ipds];

This line is a bit of an oddity as it's indexing off the end of the data.

	targs = inits + ipds;

would be nicer to my mind as doesn't even hint that we are in inits still.

drivers/acpi/hmat/hmat.c line 87

Quoted 1 line

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

+	entries = (u16 *)(&targs[tpds]);

As above I'd prefer we did the pointer arithmetic explicitly rather than used an index off the end of the array.

drivers/acpi/hmat/hmat.c line 94

Quoted 7 lines

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

+	for (init = 0; init < ipds; init++) {
+		for (targ = 0; targ < tpds; targ++) {
+			value = entries[init * tpds + targ];
+			value = (value * hmat_loc->entry_base_unit) / 10;
+			pr_info("  Initiator-Target[%d-%d]:%d%s\n",
+				inits[init], targs[targ], value,
+				hmat_data_type_suffix(type));

Worth checking at this early stage that the domains exist in SRAT?
+ screaming if they don't.

Quoted 16 lines

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

+		}
+	}
+
+	return 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_debug("HMAT: Unexpected cache header length: %d\n",
+			 cache->header.length);
+		return -EINVAL;

drivers/acpi/hmat/hmat.c line 116

Quoted 6 lines

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

+	}
+
+	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);

Can we sanity check those smbios handles actually match anything?

drivers/acpi/hmat/hmat.c line 128

Quoted 12 lines

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

+
+	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_debug("HMAT: Unexpected address range header length: %d\n",
+			 spa->header.length);

My gut feeling is that it's much more useful to make this always print rather than debug. Same with other error paths above. Given the number of times broken ACPI tables show up, it's nice to complain really loudly!

Perhaps others prefer to not do so though so I'll defer to subsystem norms.

Quoted 17 lines

drivers/acpi/hmat/hmat.c line 128

Quoted 12 lines

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

+
+	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_debug("HMAT: Unexpected address range header length: %d\n",
+			 spa->header.length);  

My gut feeling is that it's much more useful to make this always print rather than debug. Same with other error paths above. Given the number of times broken ACPI tables show up, it's nice to complain really loudly!

Quoted 1 line

Perhaps others prefer to not do so though so I'll defer to subsystem norms.

Yeah, I demoted these to debug based on earlier feedback. We should still be operational even with broken HMAT, so I don't want to create unnecessary panic if its broken, but I agree something should be immediately noticable if the firmware tables are incorrect. Maybe like what bad_srat() provides.

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.

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 memory initiator may have multiple memory targets in the same access class. The target memory's initiators in a given class indicate the nodes access characteristics share the same performance relative to other linked initiator nodes. Each target within an initiator's access 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 access class 0 from initiator node 'X':

  # symlinks -v /sys/devices/system/node/nodeX/access0/
  relative: /sys/devices/system/node/nodeX/access0/targets/nodeY -> ../../nodeY

  # symlinks -v /sys/devices/system/node/nodeY/access0/
  relative: /sys/devices/system/node/nodeY/access0/initiators/nodeX -> ../../nodeX

The new attributes are added to the sysfs stable documentation.

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

---
 Documentation/ABI/stable/sysfs-devices-node |  25 ++++-
 drivers/base/node.c                         | 142 +++++++++++++++++++++++++++-
 include/linux/node.h                        |   7 +-
 3 files changed, 171 insertions(+), 3 deletions(-)

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

index 3e90e1f3bf0a..fb843222a281 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/accessY/
+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/accessY/initiators/
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The directory containing symlinks to memory initiator
+               nodes that have class "Y" access to this target node's
+               memory. CPUs and other memory initiators in nodes not in
+               the list accessing this node's memory may have different
+               performance.
+
+What:          /sys/devices/system/node/nodeX/classY/targets/
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The directory containing symlinks to memory targets that
+               this initiator node has class "Y" access.

drivers/base/node.c +1 -0

index 86d6cd92ce3d..6f4097680580 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>
 

@@ -59,6 +60,94 @@ static inline ssize_t node_read_cpulist(struct device *dev,
 static DEVICE_ATTR(cpumap,  S_IRUGO, node_read_cpumask, NULL);
 static DEVICE_ATTR(cpulist, S_IRUGO, node_read_cpulist, NULL);

drivers/base/node.c static inline ssize_t node_read_cpulist(struct device *dev, · line 71

Quoted 9 lines · Keith Busch

drivers/base/node.c +9 -0

+/**
+ * struct node_access_nodes - Access class device to hold user visible
+ *                           relationships to other nodes.
+ * @dev:       Device for this memory access class
+ * @list_node: List element in the node's access list
+ * @access:    The access class rank
+ */
+struct node_access_nodes {
+       struct device           dev;

I'm not sure if the entire struct device is needed here.

It looks like what you need is the kobject part of it only and you can use a kobject directly here:

struct kobject kobj;

Then, you can register that under the node's kobject using kobject_init_and_add() and you can create attr groups under a kobject using sysfs_create_groups(), which is exactly what device_add_groups() does.

That would allow you to avoid allocating extra memory to hold the entire device structure and the extra empty "power" subdirectory added by device registration would not be there.

drivers/base/node.c static inline ssize_t node_read_cpulist(struct device *dev, · line 71

Quoted 9 lines

drivers/base/node.c +9 -0

+/**
+ * struct node_access_nodes - Access class device to hold user visible
+ *                           relationships to other nodes.
+ * @dev:       Device for this memory access class
+ * @list_node: List element in the node's access list
+ * @access:    The access class rank
+ */
+struct node_access_nodes {
+       struct device           dev;

I'm not sure if the entire struct device is needed here.

It looks like what you need is the kobject part of it only and you can use a kobject directly here:

struct kobject kobj;

Then, you can register that under the node's kobject using kobject_init_and_add() and you can create attr groups under a kobject using sysfs_create_groups(), which is exactly what device_add_groups() does.

That would allow you to avoid allocating extra memory to hold the entire device structure and the extra empty "power" subdirectory added by device registration would not be there.

Quoted 22 lines · Rafael J. Wysocki

drivers/base/node.c static inline ssize_t node_read_cpulist(struct device *dev, · line 71

Quoted 9 lines

drivers/base/node.c +9 -0

+/**
+ * struct node_access_nodes - Access class device to hold user visible
+ *                           relationships to other nodes.
+ * @dev:       Device for this memory access class
+ * @list_node: List element in the node's access list
+ * @access:    The access class rank
+ */
+struct node_access_nodes {
+       struct device           dev;

I'm not sure if the entire struct device is needed here.

It looks like what you need is the kobject part of it only and you can use a kobject directly here:

struct kobject kobj;

Then, you can register that under the node's kobject using kobject_init_and_add() and you can create attr groups under a kobject using sysfs_create_groups(), which is exactly what device_add_groups() does.

Quoted 3 lines

That would allow you to avoid allocating extra memory to hold the entire device structure and the extra empty "power" subdirectory added by device registration would not be there.

When you use a "raw" kobject then userspace tools do not see the devices and attributes in libraries like udev.

So unless userspace does not care about this at all,

you should use a 'struct device' where ever
possible. The memory "savings" usually just isn't worth it unless you have a _lot_ of objects being created here.

Who is going to use all of this new information?

Quoted 3 lines · <[email protected]> wrote:

When you use a "raw" kobject then userspace tools do not see the devices and attributes in libraries like udev.

And why would they need it in this particular case?

Quoted 1 line

So unless userspace does not care about this at all,

Which I think is the case here, isn't it?

Quoted 5 lines

you should use a 'struct device' where ever
possible. The memory "savings" usually just isn't worth it unless you have a _lot_ of objects being created here.

Who is going to use all of this new information?

Somebody who wants to know how the memory in the system is laid out AFAICS.

Quoted 11 lines · Rafael J. Wysocki

Quoted 3 lines · <[email protected]> wrote:

When you use a "raw" kobject then userspace tools do not see the devices and attributes in libraries like udev.

And why would they need it in this particular case?

Quoted 1 line

So unless userspace does not care about this at all,

Which I think is the case here, isn't it?

Quoted 7 lines

Quoted 5 lines

you should use a 'struct device' where ever
possible. The memory "savings" usually just isn't worth it unless you have a _lot_ of objects being created here.

Who is going to use all of this new information?

Somebody who wants to know how the memory in the system is laid out AFAICS.

Yes, this is for user space to make informed decisions about where it wants to allocate/relocate hot and cold data with respect to particular compute domains. So user space should care about these attributes, and they won't always be static when memory hotplug support for these attributes is added.

Does that change anything, or still recommending kobject? I don't have a strong opinion either way and have both options coded and ready to submit new version once I know which direction is most acceptable.

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.

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 memory initiator may have multiple memory targets in the same access class. The target memory's initiators in a given class indicate the nodes access characteristics share the same performance relative to other linked initiator nodes. Each target within an initiator's access 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 access class 0 from initiator node 'X':

  # symlinks -v /sys/devices/system/node/nodeX/access0/
  relative: /sys/devices/system/node/nodeX/access0/targets/nodeY -> ../../nodeY

  # symlinks -v /sys/devices/system/node/nodeY/access0/
  relative: /sys/devices/system/node/nodeY/access0/initiators/nodeX -> ../../nodeX

The new attributes are added to the sysfs stable documentation.

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

---
 Documentation/ABI/stable/sysfs-devices-node |  25 ++++-
 drivers/base/node.c                         | 142 +++++++++++++++++++++++++++-
 include/linux/node.h                        |   7 +-
 3 files changed, 171 insertions(+), 3 deletions(-)

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

index 3e90e1f3bf0a..fb843222a281 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/accessY/
+Date:		December 2018
+Contact:	Keith Busch <[email protected]>
+Description:
+		The node's relationship to other nodes for access class "Y".

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

+
+What:		/sys/devices/system/node/nodeX/classY/targets/
+Date:		December 2018
+Contact:	Keith Busch <[email protected]>
+Description:
+		The directory containing symlinks to memory targets that
+		this initiator node has class "Y" access.

drivers/base/node.c +90 -1

index 86d6cd92ce3d..6f4097680580 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>
 

@@ -59,6 +60,94 @@ static inline ssize_t node_read_cpulist(struct device *dev,
 static DEVICE_ATTR(cpumap,  S_IRUGO, node_read_cpumask, NULL);
 static DEVICE_ATTR(cpulist, S_IRUGO, node_read_cpulist, NULL);
 
+/**
+ * struct node_access_nodes - Access class device to hold user visible
+ * 			      relationships to other nodes.
+ * @dev:	Device for this memory access class
+ * @list_node:	List element in the node's access list
+ * @access:	The access class rank
+ */
+struct node_access_nodes {
+	struct device		dev;
+	struct list_head	list_node;
+	unsigned		access;
+};
+#define to_access_nodes(dev) container_of(dev, struct node_access_nodes, dev)
+
+static struct attribute *node_init_access_node_attrs[] = {
+	NULL,
+};
+
+static struct attribute *node_targ_access_node_attrs[] = {
+	NULL,
+};
+
+static const struct attribute_group initiators = {
+	.name	= "initiators",
+	.attrs	= node_init_access_node_attrs,
+};
+
+static const struct attribute_group targets = {
+	.name	= "targets",
+	.attrs	= node_targ_access_node_attrs,
+};
+
+static const struct attribute_group *node_access_node_groups[] = {
+	&initiators,
+	&targets,
+	NULL,
+};
+
+static void node_remove_accesses(struct node *node)
+{
+	struct node_access_nodes *c, *cnext;
+
+	list_for_each_entry_safe(c, cnext, &node->access_list, list_node) {
+		list_del(&c->list_node);
+		device_unregister(&c->dev);
+	}
+}
+
+static void node_access_release(struct device *dev)
+{
+	kfree(to_access_nodes(dev));
+}
+
+static struct node_access_nodes *node_init_node_access(struct node *node,
+						       unsigned access)
+{
+	struct node_access_nodes *access_node;
+	struct device *dev;
+
+	list_for_each_entry(access_node, &node->access_list, list_node)
+		if (access_node->access == access)
+			return access_node;
+
+	access_node = kzalloc(sizeof(*access_node), GFP_KERNEL);
+	if (!access_node)
+		return NULL;
+
+	access_node->access = access;
+	dev = &access_node->dev;
+	dev->parent = &node->dev;
+	dev->release = node_access_release;
+	dev->groups = node_access_node_groups;
+	if (dev_set_name(dev, "access%u", access))
+		goto free;
+
+	if (device_register(dev))
+		goto free_name;
+
+	pm_runtime_no_callbacks(dev);
+	list_add_tail(&access_node->list_node, &node->access_list);
+	return access_node;
+free_name:
+	kfree_const(dev->kobj.name);
+free:
+	kfree(access_node);
+	return NULL;
+}
+
 #define K(x) ((x) << (PAGE_SHIFT - 10))
 static ssize_t node_read_meminfo(struct device *dev,
 			struct device_attribute *attr, char *buf)

@@ -340,7 +429,7 @@ static int register_node(struct node *node, int num)
 void unregister_node(struct node *node)
 {
 	hugetlb_unregister_node(node);		/* no-op, if memoryless node */
-
+	node_remove_accesses(node);
 	device_unregister(&node->dev);
 }
 

@@ -372,6 +461,56 @@ int register_cpu_under_node(unsigned int cpu, unsigned int nid)
 				 kobject_name(&node_devices[nid]->dev.kobj));
 }

drivers/base/node.c +4 -0

+ * 	This function will export node relationships linking which memory
+ * 	initiator nodes can access memory targets at a given ranked access
+ * 	class.
+ */

drivers/base/node.c +21 -0

+
+	ret = sysfs_add_link_to_group(&initiator->dev.kobj, "targets",
+				      &targ_node->dev.kobj,
+				      dev_name(&targ_node->dev));
+	if (ret)
+		return ret;
+
+	ret = sysfs_add_link_to_group(&target->dev.kobj, "initiators",
+				      &init_node->dev.kobj,
+				      dev_name(&init_node->dev));
+	if (ret)
+		goto err;
+
+	return 0;
+ err:
+	sysfs_remove_link_from_group(&initiator->dev.kobj, "targets",
+				     dev_name(&targ_node->dev));
+	return ret;
+}
+
 int unregister_cpu_under_node(unsigned int cpu, unsigned int nid)
 {
 	struct device *obj;

@@ -580,6 +719,7 @@ int __register_one_node(int nid)
 			register_cpu_under_node(cpu, nid);
 	}
 
+	INIT_LIST_HEAD(&node_devices[nid]->access_list);
 	/* initialize work queue for memory hot plug */
 	init_node_hugetlb_work(nid);
 

include/linux/node.h +6 -1

index 257bb3d6d014..f34688a203c1 100644
@@ -17,11 +17,12 @@
 
 #include <linux/device.h>
 #include <linux/cpumask.h>
+#include <linux/list.h>
 #include <linux/workqueue.h>
 
 struct node {
 	struct device	dev;
-
+	struct list_head access_list;
 #if defined(CONFIG_MEMORY_HOTPLUG_SPARSE) && defined(CONFIG_HUGETLBFS)
 	struct work_struct	node_work;
 #endif

@@ -75,6 +76,10 @@ extern int register_mem_sect_under_node(struct memory_block *mem_blk,
 extern int unregister_mem_sect_under_nodes(struct memory_block *mem_blk,
 					   unsigned long phys_index);
 
+extern int register_memory_node_under_compute_node(unsigned int mem_nid,
+						   unsigned int cpu_nid,
+						   unsigned access);
+
 #ifdef CONFIG_HUGETLBFS
 extern void register_hugetlbfs_with_node(node_registration_func_t doregister,
 					 node_registration_func_t unregister);

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

Quoted 9 lines · On Thu, 24 Jan 2019 16:07:18 -0700 Keith Busch <[email protected]> wrote:

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

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

Also seems to contain the characteristics of those accesses.

Quoted 4 lines

+ * This function will export node relationships linking which memory
+ * initiator nodes can access memory targets at a given ranked access
+ * class.
+ */

drivers/base/node.c int register_cpu_under_node(unsigned int cpu, unsigned int nid) · line 486

Quoted 10 lines

drivers/base/node.c +10 -0

+int register_memory_node_under_compute_node(unsigned int mem_nid,
+					    unsigned int cpu_nid,
+					    unsigned access)
+{
+	struct node *init_node, *targ_node;
+	struct node_access_nodes *initiator, *target;
+	int ret;
+
+	if (!node_online(cpu_nid) || !node_online(mem_nid))
+		return -ENODEV;

What do we do under memory/node hotplug? More than likely that will apply in such systems (it does in mine for starters). Clearly to do the full story we would need _HMT support etc but we can do the prebaked version by having hmat entries for nodes that aren't online yet (like we do for SRAT).

Perhaps one for a follow up patch set. However, I'd like an pr_info to indicate that the node is listed but not online currently.

drivers/base/node.c int register_cpu_under_node(unsigned int cpu, unsigned int nid) · line 493

Quoted 7 lines

drivers/base/node.c +7 -0

+
+	init_node = node_devices[cpu_nid];
+	targ_node = node_devices[mem_nid];
+	initiator = node_init_node_access(init_node, access);
+	target = node_init_node_access(targ_node, access);
+	if (!initiator || !target)
+		return -ENOMEM;

If one of these fails and the other doesn't + the one that succeeded did an init, don't we end up leaking a device here? I'd expect this function to not leave things hanging if it has an error. It should unwind anything it has done. It has been added to the list so could be cleaned up later, but I'm not seeing that code.

These only get cleaned up when the node is removed.

Quoted 13 lines · Jonathan Cameron

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

Quoted 9 lines · On Thu, 24 Jan 2019 16:07:18 -0700 Keith Busch <[email protected]> wrote:

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

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

Also seems to contain the characteristics of those accesses.

Right, but not in this patch. I will append the description for access characterisitics in the patch that adds those attributes.

Quoted 21 lines

Quoted 4 lines

+ * This function will export node relationships linking which memory
+ * initiator nodes can access memory targets at a given ranked access
+ * class.
+ */

drivers/base/node.c int register_cpu_under_node(unsigned int cpu, unsigned int nid) · line 486

Quoted 10 lines

drivers/base/node.c +10 -0

+int register_memory_node_under_compute_node(unsigned int mem_nid,
+					    unsigned int cpu_nid,
+					    unsigned access)
+{
+	struct node *init_node, *targ_node;
+	struct node_access_nodes *initiator, *target;
+	int ret;
+
+	if (!node_online(cpu_nid) || !node_online(mem_nid))
+		return -ENODEV;  

What do we do under memory/node hotplug? More than likely that will apply in such systems (it does in mine for starters). Clearly to do the full story we would need _HMT support etc but we can do the prebaked version by having hmat entries for nodes that aren't online yet (like we do for SRAT).

Quoted 2 lines

Perhaps one for a follow up patch set. However, I'd like an pr_info to indicate that the node is listed but not online currently.

Yes, hot plug is planned to follow on to this series.

Quoted 14 lines

drivers/base/node.c int register_cpu_under_node(unsigned int cpu, unsigned int nid) · line 493

Quoted 7 lines

drivers/base/node.c +7 -0

+
+	init_node = node_devices[cpu_nid];
+	targ_node = node_devices[mem_nid];
+	initiator = node_init_node_access(init_node, access);
+	target = node_init_node_access(targ_node, access);
+	if (!initiator || !target)
+		return -ENOMEM;  

If one of these fails and the other doesn't + the one that succeeded did an init, don't we end up leaking a device here? I'd expect this function to not leave things hanging if it has an error. It should unwind anything it has done. It has been added to the list so could be cleaned up later, but I'm not seeing that code.

Quoted 1 line

These only get cleaned up when the node is removed.

The intiator-target relationship is many-to-many, so we don't want to free it just because we couldn't allocate its pairing node. The exisiting one may still be paired to others we were able to allocate.

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.

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 memory initiator may have multiple memory targets in the same access class. The target memory's initiators in a given class indicate the nodes access characteristics share the same performance relative to other linked initiator nodes. Each target within an initiator's access 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 access class 0 from initiator node 'X':

  # symlinks -v /sys/devices/system/node/nodeX/access0/
  relative: /sys/devices/system/node/nodeX/access0/targets/nodeY -> ../../nodeY

  # symlinks -v /sys/devices/system/node/nodeY/access0/
  relative: /sys/devices/system/node/nodeY/access0/initiators/nodeX -> ../../nodeX

The new attributes are added to the sysfs stable documentation.

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

---
 Documentation/ABI/stable/sysfs-devices-node |  25 ++++-
 drivers/base/node.c                         | 142 +++++++++++++++++++++++++++-
 include/linux/node.h                        |   7 +-
 3 files changed, 171 insertions(+), 3 deletions(-)

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

index 3e90e1f3bf0a..fb843222a281 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/accessY/
+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/accessY/initiators/
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The directory containing symlinks to memory initiator
+               nodes that have class "Y" access to this target node's
+               memory. CPUs and other memory initiators in nodes not in
+               the list accessing this node's memory may have different
+               performance.
+
+What:          /sys/devices/system/node/nodeX/classY/targets/
+Date:          December 2018
+Contact:       Keith Busch <[email protected]>
+Description:
+               The directory containing symlinks to memory targets that
+               this initiator node has class "Y" access.

drivers/base/node.c +141 -1

index 86d6cd92ce3d..6f4097680580 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>
 

@@ -59,6 +60,94 @@ static inline ssize_t node_read_cpulist(struct device *dev,
 static DEVICE_ATTR(cpumap,  S_IRUGO, node_read_cpumask, NULL);
 static DEVICE_ATTR(cpulist, S_IRUGO, node_read_cpulist, NULL);
 
+/**
+ * struct node_access_nodes - Access class device to hold user visible
+ *                           relationships to other nodes.
+ * @dev:       Device for this memory access class
+ * @list_node: List element in the node's access list
+ * @access:    The access class rank
+ */
+struct node_access_nodes {
+       struct device           dev;
+       struct list_head        list_node;
+       unsigned                access;
+};
+#define to_access_nodes(dev) container_of(dev, struct node_access_nodes, dev)
+
+static struct attribute *node_init_access_node_attrs[] = {
+       NULL,
+};
+
+static struct attribute *node_targ_access_node_attrs[] = {
+       NULL,
+};
+
+static const struct attribute_group initiators = {
+       .name   = "initiators",
+       .attrs  = node_init_access_node_attrs,
+};
+
+static const struct attribute_group targets = {
+       .name   = "targets",
+       .attrs  = node_targ_access_node_attrs,
+};
+
+static const struct attribute_group *node_access_node_groups[] = {
+       &initiators,
+       &targets,
+       NULL,
+};
+
+static void node_remove_accesses(struct node *node)
+{
+       struct node_access_nodes *c, *cnext;
+
+       list_for_each_entry_safe(c, cnext, &node->access_list, list_node) {
+               list_del(&c->list_node);
+               device_unregister(&c->dev);
+       }
+}
+
+static void node_access_release(struct device *dev)
+{
+       kfree(to_access_nodes(dev));
+}
+
+static struct node_access_nodes *node_init_node_access(struct node *node,
+                                                      unsigned access)
+{
+       struct node_access_nodes *access_node;
+       struct device *dev;
+
+       list_for_each_entry(access_node, &node->access_list, list_node)
+               if (access_node->access == access)
+                       return access_node;
+
+       access_node = kzalloc(sizeof(*access_node), GFP_KERNEL);
+       if (!access_node)
+               return NULL;
+
+       access_node->access = access;
+       dev = &access_node->dev;
+       dev->parent = &node->dev;
+       dev->release = node_access_release;
+       dev->groups = node_access_node_groups;
+       if (dev_set_name(dev, "access%u", access))
+               goto free;
+
+       if (device_register(dev))
+               goto free_name;
+
+       pm_runtime_no_callbacks(dev);
+       list_add_tail(&access_node->list_node, &node->access_list);
+       return access_node;
+free_name:
+       kfree_const(dev->kobj.name);
+free:
+       kfree(access_node);
+       return NULL;
+}
+
 #define K(x) ((x) << (PAGE_SHIFT - 10))
 static ssize_t node_read_meminfo(struct device *dev,
                        struct device_attribute *attr, char *buf)

@@ -340,7 +429,7 @@ static int register_node(struct node *node, int num)
 void unregister_node(struct node *node)
 {
        hugetlb_unregister_node(node);          /* no-op, if memoryless node */
-
+       node_remove_accesses(node);
        device_unregister(&node->dev);
 }
 

@@ -372,6 +461,56 @@ int register_cpu_under_node(unsigned int cpu, unsigned int nid)
                                 kobject_name(&node_devices[nid]->dev.kobj));
 }
 
+/**
+ * register_memory_node_under_compute_node - link memory node to its compute
+ *                                          node for a given access class.
+ * @mem_node:  Memory node number
+ * @cpu_node:  Cpu  node number
+ * @access:    Access class to register
+ *
+ * Description:
+ *     For use with platforms that may have separate memory and compute nodes.
+ *     This function will export node relationships linking which memory
+ *     initiator nodes can access memory targets at a given ranked access
+ *     class.
+ */
+int register_memory_node_under_compute_node(unsigned int mem_nid,
+                                           unsigned int cpu_nid,
+                                           unsigned access)
+{
+       struct node *init_node, *targ_node;
+       struct node_access_nodes *initiator, *target;
+       int ret;
+
+       if (!node_online(cpu_nid) || !node_online(mem_nid))
+               return -ENODEV;
+
+       init_node = node_devices[cpu_nid];
+       targ_node = node_devices[mem_nid];
+       initiator = node_init_node_access(init_node, access);
+       target = node_init_node_access(targ_node, access);
+       if (!initiator || !target)
+               return -ENOMEM;
+
+       ret = sysfs_add_link_to_group(&initiator->dev.kobj, "targets",
+                                     &targ_node->dev.kobj,
+                                     dev_name(&targ_node->dev));
+       if (ret)
+               return ret;
+
+       ret = sysfs_add_link_to_group(&target->dev.kobj, "initiators",
+                                     &init_node->dev.kobj,
+                                     dev_name(&init_node->dev));
+       if (ret)
+               goto err;
+
+       return 0;
+ err:
+       sysfs_remove_link_from_group(&initiator->dev.kobj, "targets",
+                                    dev_name(&targ_node->dev));
+       return ret;
+}
+
 int unregister_cpu_under_node(unsigned int cpu, unsigned int nid)
 {
        struct device *obj;

@@ -580,6 +719,7 @@ int __register_one_node(int nid)
                        register_cpu_under_node(cpu, nid);
        }
 
+       INIT_LIST_HEAD(&node_devices[nid]->access_list);
        /* initialize work queue for memory hot plug */
        init_node_hugetlb_work(nid);
 

include/linux/node.h +6 -2

index 257bb3d6d014..f34688a203c1 100644
@@ -17,11 +17,12 @@
 
 #include <linux/device.h>
 #include <linux/cpumask.h>
+#include <linux/list.h>
 #include <linux/workqueue.h>
 
 struct node {
        struct device   dev;
-
+       struct list_head access_list;
 #if defined(CONFIG_MEMORY_HOTPLUG_SPARSE) && defined(CONFIG_HUGETLBFS)
        struct work_struct      node_work;
 #endif

@@ -75,6 +76,10 @@ extern int register_mem_sect_under_node(struct memory_block *mem_blk,
 extern int unregister_mem_sect_under_nodes(struct memory_block *mem_blk,
                                           unsigned long phys_index);
 
+extern int register_memory_node_under_compute_node(unsigned int mem_nid,
+                                                  unsigned int cpu_nid,
+                                                  unsigned access);
+
 #ifdef CONFIG_HUGETLBFS
 extern void register_hugetlbfs_with_node(node_registration_func_t doregister,
                                         node_registration_func_t unregister);
--

2.14.4

If the HMAT Subsystem Address Range provides a valid processor proximity domain for a memory domain, or a processor domain matches the performance access of the valid processor proximity domain, 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 | 310 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 310 insertions(+)

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

index 1741bf30d87f..85fd835c2e23 100644
@@ -16,6 +16,91 @@
 #include <linux/node.h>
 #include <linux/sysfs.h>
 
+static __initdata LIST_HEAD(targets);
+static __initdata LIST_HEAD(initiators);
+static __initdata LIST_HEAD(localities);
+
+struct memory_target {
+	struct list_head node;
+	unsigned int memory_pxm;
+	unsigned int processor_pxm;
+	unsigned int read_bandwidth;
+	unsigned int write_bandwidth;
+	unsigned int read_latency;
+	unsigned int write_latency;
+};
+
+struct memory_initiator {
+	struct list_head node;
+	unsigned int processor_pxm;
+};
+
+struct memory_locality {
+	struct list_head node;
+	struct acpi_hmat_locality *hmat_loc;
+};
+
+static __init struct memory_initiator *find_mem_initiator(unsigned int cpu_pxm)
+{
+	struct memory_initiator *intitator;
+
+	list_for_each_entry(intitator, &initiators, node)
+		if (intitator->processor_pxm == cpu_pxm)
+			return intitator;
+	return NULL;
+}
+
+static __init struct memory_target *find_mem_target(unsigned int mem_pxm)
+{
+	struct memory_target *target;
+
+	list_for_each_entry(target, &targets, node)
+		if (target->memory_pxm == mem_pxm)
+			return target;
+	return NULL;
+}
+
+static __init struct memory_initiator *alloc_memory_initiator(
+							unsigned int cpu_pxm)
+{
+	struct memory_initiator *intitator;
+
+	if (pxm_to_node(cpu_pxm) == NUMA_NO_NODE)
+		return NULL;
+
+	intitator = find_mem_initiator(cpu_pxm);
+	if (intitator)
+		return intitator;
+
+	intitator = kzalloc(sizeof(*intitator), GFP_KERNEL);
+	if (!intitator)
+		return NULL;
+
+	intitator->processor_pxm = cpu_pxm;
+	list_add_tail(&intitator->node, &initiators);
+	return intitator;
+}
+
+static __init void alloc_memory_target(unsigned int mem_pxm)
+{
+	struct memory_target *target;
+
+	if (pxm_to_node(mem_pxm) == NUMA_NO_NODE)
+		return;
+
+	target = find_mem_target(mem_pxm);
+	if (target)
+		return;
+
+	target = kzalloc(sizeof(*target), GFP_KERNEL);
+	if (!target)
+		return;
+
+	target->memory_pxm = mem_pxm;
+	target->processor_pxm = PXM_INVAL;
+	list_add_tail(&target->node, &targets);
+}
+
 static __init const char *hmat_data_type(u8 type)
 {
 	switch (type) {

@@ -52,13 +137,45 @@ static __init const char *hmat_data_type_suffix(u8 type)
 	};
 }
 
+static __init void hmat_update_target_access(struct memory_target *target,
+                                             u8 type, u32 value)
+{
+	switch (type) {
+	case ACPI_HMAT_ACCESS_LATENCY:
+		target->read_latency = value;
+		target->write_latency = value;
+		break;
+	case ACPI_HMAT_READ_LATENCY:
+		target->read_latency = value;
+		break;
+	case ACPI_HMAT_WRITE_LATENCY:
+		target->write_latency = value;
+		break;
+	case ACPI_HMAT_ACCESS_BANDWIDTH:
+		target->read_bandwidth = value;
+		target->write_bandwidth = value;
+		break;
+	case ACPI_HMAT_READ_BANDWIDTH:
+		target->read_bandwidth = value;
+		break;
+	case ACPI_HMAT_WRITE_BANDWIDTH:
+		target->write_bandwidth = value;
+		break;
+	default:
+		break;
+	};
+}
+
 static __init int hmat_parse_locality(union acpi_subtable_headers *header,
 				      const unsigned long end)
 {
 	struct acpi_hmat_locality *hmat_loc = (void *)header;
+	struct memory_target *target;
+	struct memory_initiator *initiator;
 	unsigned int init, targ, total_size, ipds, tpds;
 	u32 *inits, *targs, value;
 	u16 *entries;
+	bool report = false;
 	u8 type;
 
 	if (hmat_loc->header.length < sizeof(*hmat_loc)) {

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

 		for (targ = 0; targ < tpds; targ++) {
 			value = entries[init * tpds + targ];
 			value = (value * hmat_loc->entry_base_unit) / 10;
 			pr_info("  Initiator-Target[%d-%d]:%d%s\n",
 				inits[init], targs[targ], value,
 				hmat_data_type_suffix(type));
+
+			target = find_mem_target(targs[targ]);
+			if (target && report &&
+			    target->processor_pxm == initiator->processor_pxm)
+				hmat_update_target_access(target, type, value);
+		}

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

 	}
 

@@ -122,16 +265,35 @@ static int __init hmat_parse_address_range(union acpi_subtable_headers *header,
 					   const unsigned long end)

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

 	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);
 
+	if (spa->flags & ACPI_HMAT_MEMORY_PD_VALID) {
+		target = find_mem_target(spa->memory_PD);
+		if (!target) {
+			pr_debug("HMAT: Memory Domain missing from SRAT\n");
+			return -EINVAL;
+		}
+	}
+	if (target && spa->flags & ACPI_HMAT_PROCESSOR_PD_VALID) {
+		int p_node = pxm_to_node(spa->processor_PD);
+
+		if (p_node == NUMA_NO_NODE) {
+			pr_debug("HMAT: Invalid Processor Domain\n");
+			return -EINVAL;
+		}
+		target->processor_pxm = p_node;
+	}
+
 	return 0;
 }
 

@@ -155,6 +317,142 @@ 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 bool hmat_is_local(struct memory_target *target,
+                                 u8 type, u32 value)
+{
+	switch (type) {
+	case ACPI_HMAT_ACCESS_LATENCY:
+		return value == target->read_latency &&
+		       value == target->write_latency;
+	case ACPI_HMAT_READ_LATENCY:
+		return value == target->read_latency;
+	case ACPI_HMAT_WRITE_LATENCY:
+		return value == target->write_latency;
+	case ACPI_HMAT_ACCESS_BANDWIDTH:
+		return value == target->read_bandwidth &&
+		       value == target->write_bandwidth;
+	case ACPI_HMAT_READ_BANDWIDTH:
+		return value == target->read_bandwidth;
+	case ACPI_HMAT_WRITE_BANDWIDTH:
+		return value == target->write_bandwidth;
+	default:
+		return true;
+	};
+}
+
+static bool hmat_is_local_initiator(struct memory_target *target,
+				    struct memory_initiator *initiator,
+				    struct acpi_hmat_locality *hmat_loc)

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

+
+	for (i = 0; i < ipds; i++) {
+		if (inits[i] == initiator->processor_pxm) {
+			idx = i;
+			break;
+		}
+	}
+
+	if (i == ipds)
+		return false;
+
+	for (i = 0; i < tpds; i++) {

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

+
+	return hmat_is_local(target, hmat_loc->data_type, value);
+}
+
+static __init void hmat_register_if_local(struct memory_target *target,
+					  struct memory_initiator *initiator)
+{
+	unsigned int mem_nid, cpu_nid;
+	struct memory_locality *loc;
+
+	if (initiator->processor_pxm == target->processor_pxm)
+		return;
+
+	list_for_each_entry(loc, &localities, node)
+		if (!hmat_is_local_initiator(target, initiator, loc->hmat_loc))
+			return;
+
+	mem_nid = pxm_to_node(target->memory_pxm);
+	cpu_nid = pxm_to_node(initiator->processor_pxm);
+	register_memory_node_under_compute_node(mem_nid, cpu_nid, 0);
+}
+
+static __init void hmat_register_target_initiators(struct memory_target *target)

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

+		return;
+
+	if (list_empty(&localities))
+		return;
+
+	list_for_each_entry(initiator, &initiators, node)
+		hmat_register_if_local(target, initiator);
+}
+
+static __init void hmat_register_targets(void)
+{
+	struct memory_target *target, *tnext;
+	struct memory_locality *loc, *lnext;
+	struct memory_initiator *intitator, *inext;
+
+	list_for_each_entry_safe(target, tnext, &targets, node) {
+		list_del(&target->node);
+		hmat_register_target_initiators(target);
+		kfree(target);
+	}
+
+	list_for_each_entry_safe(intitator, inext, &initiators, node) {
+		list_del(&intitator->node);
+		kfree(intitator);
+	}
+
+	list_for_each_entry_safe(loc, lnext, &localities, node) {
+		list_del(&loc->node);
+		kfree(loc);
+	}
+}
+
 static __init int hmat_init(void)
 {
 	struct acpi_table_header *tbl;

@@ -164,6 +462,17 @@ static __init int hmat_init(void)
 	if (srat_disabled())
 		return 0;
 
+	status = acpi_get_table(ACPI_SIG_SRAT, 0, &tbl);
+	if (ACPI_FAILURE(status))
+		return 0;
+
+	if (acpi_table_parse_entries(ACPI_SIG_SRAT,
+				sizeof(struct acpi_table_srat),
+				ACPI_SRAT_TYPE_MEMORY_AFFINITY,
+				srat_parse_mem_affinity, 0) < 0)
+		goto out_put;
+	acpi_put_table(tbl);
+
 	status = acpi_get_table(ACPI_SIG_HMAT, 0, &tbl);
 	if (ACPI_FAILURE(status))
 		return 0;

@@ -174,6 +483,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;

Register the local attached performace access attributes 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 local and matching pairings at this time.

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

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

drivers/acpi/hmat/Kconfig +1 -0

index c9637e2e7514..08e972ead159 100644
@@ -2,6 +2,7 @@
 config ACPI_HMAT
 	bool "ACPI Heterogeneous Memory Attribute Table Support"
 	depends on ACPI_NUMA
+	select HMEM_REPORTING
 	help
 	 If set, this option causes the kernel to set the memory NUMA node
 	 relationships and access attributes in accordance with ACPI HMAT

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

index 85fd835c2e23..917e6122b3f0 100644
@@ -430,6 +430,19 @@ static __init void hmat_register_target_initiators(struct memory_target *target)
 		hmat_register_if_local(target, initiator);
 }
 
+static __init void hmat_register_target_perf(struct memory_target *target)
+{
+	unsigned mem_nid = pxm_to_node(target->memory_pxm);
+	struct node_hmem_attrs hmem_attrs = {
+		.read_bandwidth	= target->read_bandwidth,
+		.write_bandwidth= target->write_bandwidth,
+		.read_latency	= target->read_latency,
+		.write_latency	= target->write_latency,
+	};
+
+	node_set_perf_attrs(mem_nid, &hmem_attrs, 0);
+}
+
 static __init void hmat_register_targets(void)
 {
 	struct memory_target *target, *tnext;

@@ -439,6 +452,7 @@ static __init void hmat_register_targets(void)
 	list_for_each_entry_safe(target, tnext, &targets, node) {
 		list_del(&target->node);
 		hmat_register_target_initiators(target);
+		hmat_register_target_perf(target);
 		kfree(target);
 	}
 

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/<cpu>/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.

Add the attributes for the system memory side caches to sysfs stable documentation.

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

---
 Documentation/ABI/stable/sysfs-devices-node |  34 +++++++
 drivers/base/node.c                         | 153 ++++++++++++++++++++++++++++
 include/linux/node.h                        |  34 +++++++
 3 files changed, 221 insertions(+)

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

index 41cb9345e1e0..26327279b6b6 100644
@@ -142,3 +142,37 @@ Contact:	Keith Busch <[email protected]>
 Description:
 		This node's write latency in nanoseconds when access
 		from nodes found in this class's linked initiators.
+
+What:		/sys/devices/system/node/nodeX/side_cache/indexY/associativity
+Date:		December 2018
+Contact:	Keith Busch <[email protected]>

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

+
+What:		/sys/devices/system/node/nodeX/side_cache/indexY/line_size
+Date:		December 2018
+Contact:	Keith Busch <[email protected]>
+Description:
+		The number of bytes accessed from the next cache level on a
+		cache miss.
+
+What:		/sys/devices/system/node/nodeX/side_cache/indexY/size
+Date:		December 2018
+Contact:	Keith Busch <[email protected]>
+Description:
+		The size of this memory side cache in bytes.
+
+What:		/sys/devices/system/node/nodeX/side_cache/indexY/write_policy
+Date:		December 2018
+Contact:	Keith Busch <[email protected]>
+Description:
+		The cache write policy: 0 for write-back, 1 for write-through,
+		other or unknown.

drivers/base/node.c +67 -0

index 2de546a040a5..9b4cb29863ff 100644
@@ -205,6 +205,157 @@ void node_set_perf_attrs(unsigned int nid, struct node_hmem_attrs *hmem_attrs,
 		}
 	}
 }
+
+/**
+ * struct node_cache_info - Internal tracking for memory node caches
+ * @dev:	Device represeting the cache level
+ * @node:	List element for tracking in the node
+ * @cache_attrs:Attributes for this cache level
+ */
+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;
+	dev->release = node_cache_release;
+	if (dev_set_name(dev, "side_cache"))
+		goto free_dev;
+
+	if (device_register(dev))
+		goto free_name;
+
+	pm_runtime_no_callbacks(dev);