WIP: memory: topology: expose per-numa memory

This patch wants to add memory informations on topology.Node,
so we can have per-NUMA-zone memory information.

WIP:
- do we need more tests?
- doc is missing,
- API is provisional, names to be reviewed.

The basic concepts are there, though.

Fixes: jaypipes#200
Signed-off-by: Francesco Romani <[email protected]>

cmd/ghwc/commands/topology.go

@@ -37,6 +37,11 @@ func showTopology(cmd *cobra.Command, args []string) error {
 			for _, cache := range node.Caches {
 				fmt.Printf("  %v\n", cache)
 			}
+			fmt.Printf("  %v\n", node.Memory)
+			fmt.Printf("  distances\n")
+			for nodeID, dist := range node.Distances {
+				fmt.Printf("    to node #%d %v\n", nodeID, dist)
+			}
 		}
 	case outputFormatJSON:
 		fmt.Printf("%s\n", topology.JSONString(pretty))

pkg/memory/memory.go

@@ -25,13 +25,36 @@ type Module struct {
 	Vendor       string `json:"vendor"`
 }
 
-type Info struct {
-	ctx                *context.Context
+// TODO review the name
+type Area struct {
 	TotalPhysicalBytes int64 `json:"total_physical_bytes"`
 	TotalUsableBytes   int64 `json:"total_usable_bytes"`
+}
+
+func (a *Area) String() string {
+	tpbs := util.UNKNOWN
+	if a.TotalPhysicalBytes > 0 {
+		tpb := a.TotalPhysicalBytes
+		unit, unitStr := unitutil.AmountString(tpb)
+		tpb = int64(math.Ceil(float64(a.TotalPhysicalBytes) / float64(unit)))
+		tpbs = fmt.Sprintf("%d%s", tpb, unitStr)
+	}
+	tubs := util.UNKNOWN
+	if a.TotalUsableBytes > 0 {
+		tub := a.TotalUsableBytes
+		unit, unitStr := unitutil.AmountString(tub)
+		tub = int64(math.Ceil(float64(a.TotalUsableBytes) / float64(unit)))
+		tubs = fmt.Sprintf("%d%s", tub, unitStr)
+	}
+	return fmt.Sprintf("memory (%s physical, %s usable)", tpbs, tubs)
+}
+
+type Info struct {
+	ctx *context.Context
 	// An array of sizes, in bytes, of memory pages supported by the host
 	SupportedPageSizes []uint64  `json:"supported_page_sizes"`
 	Modules            []*Module `json:"modules"`
+	Area
 }
 
 func New(opts ...*option.Option) (*Info, error) {
@@ -44,21 +67,7 @@ func New(opts ...*option.Option) (*Info, error) {
 }
 
 func (i *Info) String() string {
-	tpbs := util.UNKNOWN
-	if i.TotalPhysicalBytes > 0 {
-		tpb := i.TotalPhysicalBytes
-		unit, unitStr := unitutil.AmountString(tpb)
-		tpb = int64(math.Ceil(float64(i.TotalPhysicalBytes) / float64(unit)))
-		tpbs = fmt.Sprintf("%d%s", tpb, unitStr)
-	}
-	tubs := util.UNKNOWN
-	if i.TotalUsableBytes > 0 {
-		tub := i.TotalUsableBytes
-		unit, unitStr := unitutil.AmountString(tub)
-		tub = int64(math.Ceil(float64(i.TotalUsableBytes) / float64(unit)))
-		tubs = fmt.Sprintf("%d%s", tub, unitStr)
-	}
-	return fmt.Sprintf("memory (%s physical, %s usable)", tpbs, tubs)
+	return i.Area.String()
 }
 
 // simple private struct used to encapsulate memory information in a top-level

pkg/memory/memory_linux.go

@@ -17,6 +17,7 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/jaypipes/ghw/pkg/context"
 	"github.com/jaypipes/ghw/pkg/linuxpath"
 	"github.com/jaypipes/ghw/pkg/unitutil"
 	"github.com/jaypipes/ghw/pkg/util"
@@ -56,6 +57,45 @@ func (i *Info) load() error {
 	return nil
 }
 
+func AreaForNode(ctx *context.Context, nodeID int) (*Area, error) {
+	paths := linuxpath.New(ctx)
+	path := filepath.Join(
+		paths.SysDevicesSystemNode,
+		fmt.Sprintf("node%d", nodeID),
+	)
+
+	blockSizeBytes, err := memoryBlockSizeBytes(paths.SysDevicesSystemMemory)
+	if err != nil {
+		return nil, err
+	}
+
+	totPhys, err := memoryTotalPhysicalBytesFromPath(path, blockSizeBytes)
+	if err != nil {
+		return nil, err
+	}
+
+	totUsable, err := memoryTotalUsableBytesFromPath(filepath.Join(path, "meminfo"))
+	if err != nil {
+		return nil, err
+	}
+
+	return &Area{
+		TotalPhysicalBytes: totPhys,
+		TotalUsableBytes:   totUsable,
+	}, nil
+}
+
+func memoryBlockSizeBytes(dir string) (uint64, error) {
+	// get the memory block size in byte in hexadecimal notation
+	blockSize := filepath.Join(dir, "block_size_bytes")
+
+	d, err := ioutil.ReadFile(blockSize)
+	if err != nil {
+		return 0, err
+	}
+	return strconv.ParseUint(strings.TrimSpace(string(d)), 16, 64)
+}
+
 func memTotalPhysicalBytes(paths *linuxpath.Paths) (total int64) {
 	defer func() {
 		// fallback to the syslog file approach in case of error
@@ -66,42 +106,48 @@ func memTotalPhysicalBytes(paths *linuxpath.Paths) (total int64) {
 
 	// detect physical memory from /sys/devices/system/memory
 	dir := paths.SysDevicesSystemMemory
-
-	// get the memory block size in byte in hexadecimal notation
-	blockSize := filepath.Join(dir, "block_size_bytes")
-
-	d, err := ioutil.ReadFile(blockSize)
+	blockSizeBytes, err := memoryBlockSizeBytes(dir)
 	if err != nil {
-		return -1
+		total = -1
+		return total
 	}
-	blockSizeBytes, err := strconv.ParseUint(strings.TrimSpace(string(d)), 16, 64)
+
+	total, err = memoryTotalPhysicalBytesFromPath(dir, blockSizeBytes)
 	if err != nil {
-		return -1
+		total = -1
 	}
+	return total
+}
 
-	// iterate over memory's block /sys/devices/system/memory/memory*,
+func memoryTotalPhysicalBytesFromPath(dir string, blockSizeBytes uint64) (int64, error) {
+	// iterate over memory's block /sys/.../memory*,
 	// if the memory block state is 'online' we increment the total
 	// with the memory block size to determine the amount of physical
-	// memory available on this system
+	// memory available on this system.
+	// This works for both system-wide:
+	// /sys/devices/system/memory/memory*
+	// and for per-numa-node report:
+	// /sys/devices/system/node/node*/memory*
+
 	sysMemory, err := filepath.Glob(filepath.Join(dir, "memory*"))
 	if err != nil {
-		return -1
+		return -1, err
 	} else if sysMemory == nil {
-		return -1
+		return -1, fmt.Errorf("cannot find memory entries in %q", dir)
 	}
 
+	var total int64
 	for _, path := range sysMemory {
 		s, err := ioutil.ReadFile(filepath.Join(path, "state"))
 		if err != nil {
-			return -1
+			return -1, err
 		}
 		if strings.TrimSpace(string(s)) != "online" {
 			continue
 		}
 		total += int64(blockSizeBytes)
 	}
-
-	return total
+	return total, nil
 }
 
 func memTotalPhysicalBytesFromSyslog(paths *linuxpath.Paths) int64 {
@@ -161,7 +207,17 @@ func memTotalPhysicalBytesFromSyslog(paths *linuxpath.Paths) int64 {
 }
 
 func memTotalUsableBytes(paths *linuxpath.Paths) int64 {
-	// In Linux, /proc/meminfo contains a set of memory-related amounts, with
+	amount, err := memoryTotalUsableBytesFromPath(paths.ProcMeminfo)
+	if err != nil {
+		return -1
+	}
+	return amount
+}
+
+func memoryTotalUsableBytesFromPath(meminfoPath string) (int64, error) {
+	// In Linux, /proc/meminfo or its close relative
+	// /sys/devices/system/node/node*/meminfo
+	// contains a set of memory-related amounts, with
 	// lines looking like the following:
 	//
 	// $ cat /proc/meminfo
@@ -179,36 +235,41 @@ func memTotalUsableBytes(paths *linuxpath.Paths) int64 {
 	// "theoretical" information. For instance, on the above system, I have
 	// 24GB of RAM but MemTotal is indicating only around 23GB. This is because
 	// MemTotal contains the exact amount of *usable* memory after accounting
-	// for the kernel's resident memory size and a few reserved bits. For more
-	// information, see:
+	// for the kernel's resident memory size and a few reserved bits.
+	// Please note GHW cares about the subset of lines shared between system-wide
+	// and per-NUMA-node meminfos. For more information, see:
 	//
 	//  https://www.kernel.org/doc/Documentation/filesystems/proc.txt
-	filePath := paths.ProcMeminfo
-	r, err := os.Open(filePath)
+	r, err := os.Open(meminfoPath)
 	if err != nil {
-		return -1
+		return -1, err
 	}
 	defer util.SafeClose(r)
 
+	splitFields := func(c rune) bool {
+		return c == ':'
+	}
+
 	scanner := bufio.NewScanner(r)
 	for scanner.Scan() {
 		line := scanner.Text()
-		parts := strings.Fields(line)
-		key := strings.Trim(parts[0], ": \t")
-		if key != "MemTotal" {
+		parts := strings.FieldsFunc(line, splitFields)
+		key := parts[0]
+		if !strings.Contains(key, "MemTotal") {
 			continue
 		}
-		value, err := strconv.Atoi(strings.TrimSpace(parts[1]))
+		rawValue := parts[1]
+		inKb := strings.HasSuffix(rawValue, "kB")
+		value, err := strconv.Atoi(strings.TrimSpace(strings.TrimSuffix(rawValue, "kB")))
 		if err != nil {
-			return -1
+			return -1, err
 		}
-		inKb := (len(parts) == 3 && strings.TrimSpace(parts[2]) == "kB")
 		if inKb {
 			value = value * int(unitutil.KB)
 		}
-		return int64(value)
+		return int64(value), nil
 	}
-	return -1
+	return -1, fmt.Errorf("failed to find MemTotal entry in path %q", meminfoPath)
 }
 
 func memSupportedPageSizes(paths *linuxpath.Paths) []uint64 {

pkg/memory/memory_test.go

@@ -26,7 +26,12 @@ func TestMemory(t *testing.T) {
 
 	tpb := mem.TotalPhysicalBytes
 	tub := mem.TotalUsableBytes
-
+	if tpb == 0 {
+		t.Fatalf("Total physical bytes reported zero")
+	}
+	if tub == 0 {
+		t.Fatalf("Total usable bytes reported zero")
+	}
 	if tpb < tub {
 		t.Fatalf("Total physical bytes < total usable bytes. %d < %d",
 			tpb, tub)

pkg/snapshot/clonetree_linux.go

@@ -44,6 +44,8 @@ func ExpectedCloneStaticContent() []string {
 		"/sys/devices/system/node/possible",
 		"/sys/devices/system/node/node*/cpu*",
 		"/sys/devices/system/node/node*/distance",
+		"/sys/devices/system/node/node*/meminfo",
+		"/sys/devices/system/node/node*/memory*",
 	}
 }
 

pkg/topology/topology.go

@@ -59,6 +59,7 @@ type Node struct {
 	Cores     []*cpu.ProcessorCore `json:"cores"`
 	Caches    []*memory.Cache      `json:"caches"`
 	Distances []int                `json:"distances"`
+	Memory    *memory.Area         `json:"memory"`
 }
 
 func (n *Node) String() string {

pkg/topology/topology_linux.go

@@ -69,6 +69,13 @@ func topologyNodes(ctx *context.Context) []*Node {
 		}
 		node.Distances = distances
 
+		area, err := memory.AreaForNode(ctx, nodeID)
+		if err != nil {
+			ctx.Warn("failed to determine memory area for node: %s\n", err)
+			return nodes
+		}
+		node.Memory = area
+
 		nodes = append(nodes, node)
 	}
 	return nodes

pkg/topology/topology_linux_test.go

@@ -10,6 +10,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/jaypipes/ghw/pkg/memory"
 	"github.com/jaypipes/ghw/pkg/option"
 	"github.com/jaypipes/ghw/pkg/topology"
 
@@ -58,3 +59,65 @@ func TestTopologyNUMADistances(t *testing.T) {
 		t.Fatalf("Expected symmetric distance to the other node, got %v and %v", info.Nodes[0].Distances, info.Nodes[1].Distances)
 	}
 }
+
+// nolint: gocyclo
+func TestTopologyPerNUMAMemory(t *testing.T) {
+	testdataPath, err := testdata.SnapshotsDirectory()
+	if err != nil {
+		t.Fatalf("Expected nil err, but got %v", err)
+	}
+
+	multiNumaSnapshot := filepath.Join(testdataPath, "linux-amd64-intel-xeon-L5640.tar.gz")
+	// from now on we use constants reflecting the content of the snapshot we requested,
+	// which we reviewed beforehand. IOW, you need to know the content of the
+	// snapshot to fully understand this test. Inspect it using
+	// GHW_SNAPSHOT_PATH="/path/to/linux-amd64-intel-xeon-L5640.tar.gz" ghwc topology
+
+	memInfo, err := memory.New(option.WithSnapshot(option.SnapshotOptions{
+		Path: multiNumaSnapshot,
+	}))
+
+	if err != nil {
+		t.Fatalf("Expected nil err, but got %v", err)
+	}
+	if memInfo == nil {
+		t.Fatalf("Expected non-nil MemoryInfo, but got nil")
+	}
+
+	info, err := topology.New(option.WithSnapshot(option.SnapshotOptions{
+		Path: multiNumaSnapshot,
+	}))
+
+	if err != nil {
+		t.Fatalf("Expected nil err, but got %v", err)
+	}
+	if info == nil {
+		t.Fatalf("Expected non-nil TopologyInfo, but got nil")
+	}
+
+	if len(info.Nodes) != 2 {
+		t.Fatalf("Expected 2 nodes but got 0.")
+	}
+
+	for _, node := range info.Nodes {
+		if node.Memory == nil {
+			t.Fatalf("missing memory information for node %d", node.ID)
+		}
+
+		if node.Memory.TotalPhysicalBytes <= 0 {
+			t.Fatalf("negative physical size for node %d", node.ID)
+		}
+		if node.Memory.TotalPhysicalBytes > memInfo.TotalPhysicalBytes {
+			t.Fatalf("physical size for node %d exceeds system's", node.ID)
+		}
+		if node.Memory.TotalUsableBytes <= 0 {
+			t.Fatalf("negative usable size for node %d", node.ID)
+		}
+		if node.Memory.TotalUsableBytes > memInfo.TotalUsableBytes {
+			t.Fatalf("usable size for node %d exceeds system's", node.ID)
+		}
+		if node.Memory.TotalUsableBytes > node.Memory.TotalPhysicalBytes {
+			t.Fatalf("excessive usable size for node %d", node.ID)
+		}
+	}
+}