From 0174b24e6101b22a93a2c350d68bbf6dc2a26ffb Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 18 May 2020 14:43:13 -0700 Subject: [PATCH 1/7] cpuinfo_test: fix test data Real /proc/cpuinfo files do not contain empty lines at the start or at the end of file. Signed-off-by: Kir Kolyshkin --- cpuinfo_test.go | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/cpuinfo_test.go b/cpuinfo_test.go index b6869acee..ef55b21d1 100644 --- a/cpuinfo_test.go +++ b/cpuinfo_test.go @@ -18,8 +18,7 @@ package procfs import "testing" const ( - cpuinfoArm7 = ` -Processor : ARMv7 Processor rev 5 (v7l) + cpuinfoArm7 = `Processor : ARMv7 Processor rev 5 (v7l) processor : 0 BogoMIPS : 2400.00 @@ -37,8 +36,7 @@ Hardware : sun8i Revision : 0000 Serial : 5400503583203c3c040e` - cpuinfoS390x = ` -vendor_id : IBM/S390 + cpuinfoS390x = `vendor_id : IBM/S390 # processors : 4 bogomips per cpu: 3033.00 max thread id : 0 @@ -69,11 +67,9 @@ cpu MHz static : 5000 cpu number : 3 cpu MHz dynamic : 5000 -cpu MHz static : 5000 -` +cpu MHz static : 5000` - cpuinfoPpc64 = ` -processor : 0 + cpuinfoPpc64 = `processor : 0 cpu : POWER7 (architected), altivec supported clock : 3000.000000MHz revision : 2.1 (pvr 003f 0201) From 928d8ea1fea2405d17948f49ef4dfbd911430fff Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 18 May 2020 15:00:38 -0700 Subject: [PATCH 2/7] parseCPUInfoARM: fix for kernel 3.8+ Since Linux kernel 3.8-rc1 (commit b4b8f770eb10a "ARM: kernel: update cpuinfo to print all online CPUs features", Sep 10 2012) the kernel does not print "Processor" as the first line of /proc/cpuinfo. Instead, it adds a line named "model name" with similar contents. Fix the code for this case, and add more tests. Signed-off-by: Kir Kolyshkin --- cpuinfo.go | 27 +++++++++++---------- cpuinfo_test.go | 63 ++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 75 insertions(+), 15 deletions(-) diff --git a/cpuinfo.go b/cpuinfo.go index bb3be99ff..a80ef9eb6 100644 --- a/cpuinfo.go +++ b/cpuinfo.go @@ -189,16 +189,10 @@ func parseCPUInfoX86(info []byte) ([]CPUInfo, error) { func parseCPUInfoARM(info []byte) ([]CPUInfo, error) { scanner := bufio.NewScanner(bytes.NewReader(info)) - firstLine := firstNonEmptyLine(scanner) - if !strings.HasPrefix(firstLine, "Processor") || !strings.Contains(firstLine, ":") { - return nil, errors.New("invalid cpuinfo file: " + firstLine) - } - field := strings.SplitN(firstLine, ": ", 2) - commonCPUInfo := CPUInfo{VendorID: field[1]} - - cpuinfo := []CPUInfo{} - i := -1 + cpuinfo := []CPUInfo{{}} + i := 0 featuresLine := "" + modelName := "" for scanner.Scan() { line := scanner.Text() @@ -207,14 +201,23 @@ func parseCPUInfoARM(info []byte) ([]CPUInfo, error) { } field := strings.SplitN(line, ": ", 2) switch strings.TrimSpace(field[0]) { + case "Processor": + // this appears at the first line in kernels < 3.8 + modelName = field[1] + cpuinfo[i].VendorID = field[1] case "processor": - cpuinfo = append(cpuinfo, commonCPUInfo) // start of the next processor - i++ v, err := strconv.ParseUint(field[1], 0, 32) if err != nil { return nil, err } - cpuinfo[i].Processor = uint(v) + if v != 0 { + cpuinfo = append(cpuinfo, CPUInfo{VendorID: modelName}) // start of the next processor + i++ + cpuinfo[i].Processor = uint(v) + } + case "model name": + // this is what kernel 3.8+ gives instead of "Processor:" + cpuinfo[i].VendorID = field[1] case "BogoMIPS": v, err := strconv.ParseFloat(field[1], 64) if err != nil { diff --git a/cpuinfo_test.go b/cpuinfo_test.go index ef55b21d1..f4966869c 100644 --- a/cpuinfo_test.go +++ b/cpuinfo_test.go @@ -18,7 +18,23 @@ package procfs import "testing" const ( - cpuinfoArm7 = `Processor : ARMv7 Processor rev 5 (v7l) + // non-SMP system before kernel v3.8, commit + // https://github.com/torvalds/linux/commit/b4b8f770eb + cpuinfoArm6 = `Processor : ARMv6-compatible processor rev 5 (v6l) +BogoMIPS : 791.34 +Features : swp half thumb fastmult vfp edsp java +CPU implementer : 0x41 +CPU architecture: 6TEJ +CPU variant : 0x1 +CPU part : 0xb36 +CPU revision : 5 + +Hardware : IMAPX200 +Revision : 0000 +Serial : 0000000000000000` + + // SMP system before kernel 3.8 + cpuinfoArm7old = `Processor : ARMv7 Processor rev 5 (v7l) processor : 0 BogoMIPS : 2400.00 @@ -36,6 +52,17 @@ Hardware : sun8i Revision : 0000 Serial : 5400503583203c3c040e` + // kernel v3.8+ + cpuinfoArm7 = `processor : 0 +model name : ARMv7 Processor rev 2 (v7l) +BogoMIPS : 50.00 +Features : half thumb fastmult vfp edsp thumbee vfpv3 tls idiva idivt vfpd32 lpae +CPU implementer : 0x56 +CPU architecture: 7 +CPU variant : 0x2 +CPU part : 0x584 +CPU revision : 2` + cpuinfoS390x = `vendor_id : IBM/S390 # processors : 4 bogomips per cpu: 3033.00 @@ -149,8 +176,23 @@ func TestCPUInfoX86(t *testing.T) { } } -func TestCPUInfoParseARM(t *testing.T) { - cpuinfo, err := parseCPUInfoARM([]byte(cpuinfoArm7)) +func TestCPUInfoParseARM6(t *testing.T) { + cpuinfo, err := parseCPUInfoARM([]byte(cpuinfoArm6)) + if err != nil || cpuinfo == nil { + t.Fatalf("unable to parse arm cpu info: %v", err) + } + if want, have := 1, len(cpuinfo); want != have { + t.Errorf("want number of processors %v, have %v", want, have) + } + if want, have := "ARMv6-compatible processor rev 5 (v6l)", cpuinfo[0].VendorID; want != have { + t.Errorf("want vendor %v, have %v", want, have) + } + if want, have := "thumb", cpuinfo[0].Flags[2]; want != have { + t.Errorf("want flag %v, have %v", want, have) + } +} +func TestCPUInfoParseARM7old(t *testing.T) { + cpuinfo, err := parseCPUInfoARM([]byte(cpuinfoArm7old)) if err != nil || cpuinfo == nil { t.Fatalf("unable to parse arm cpu info: %v", err) } @@ -165,6 +207,21 @@ func TestCPUInfoParseARM(t *testing.T) { } } +func TestCPUInfoParseARM(t *testing.T) { + cpuinfo, err := parseCPUInfoARM([]byte(cpuinfoArm7)) + if err != nil || cpuinfo == nil { + t.Fatalf("unable to parse arm cpu info: %v", err) + } + if want, have := 1, len(cpuinfo); want != have { + t.Errorf("want number of processors %v, have %v", want, have) + } + if want, have := "ARMv7 Processor rev 2 (v7l)", cpuinfo[0].VendorID; want != have { + t.Errorf("want vendor %v, have %v", want, have) + } + if want, have := "fastmult", cpuinfo[0].Flags[2]; want != have { + t.Errorf("want flag %v, have %v", want, have) + } +} func TestCPUInfoParseS390X(t *testing.T) { cpuinfo, err := parseCPUInfoS390X([]byte(cpuinfoS390x)) if err != nil || cpuinfo == nil { From 611c66455ea19c43d5ce5aab3169ce4bb0f44e6e Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 18 May 2020 14:26:41 -0700 Subject: [PATCH 3/7] parseCPUInfoARM: only parse Features once The `Features` line is the same for all CPUs, and yet it is assigned to and parsed many times. Optimize the code so it is only parsed once and assigned in place. Signed-off-by: Kir Kolyshkin --- cpuinfo.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/cpuinfo.go b/cpuinfo.go index a80ef9eb6..6dd54ddaa 100644 --- a/cpuinfo.go +++ b/cpuinfo.go @@ -191,7 +191,7 @@ func parseCPUInfoARM(info []byte) ([]CPUInfo, error) { cpuinfo := []CPUInfo{{}} i := 0 - featuresLine := "" + var features []string modelName := "" for scanner.Scan() { @@ -225,13 +225,14 @@ func parseCPUInfoARM(info []byte) ([]CPUInfo, error) { } cpuinfo[i].BogoMips = v case "Features": - featuresLine = line + if len(features) == 0 { + // only parse once + features = strings.Fields(field[1]) + } + cpuinfo[i].Flags = features } } - fields := strings.SplitN(featuresLine, ": ", 2) - for i := range cpuinfo { - cpuinfo[i].Flags = strings.Fields(fields[1]) - } + return cpuinfo, nil } From c2b7b68d38efb2cf9fd3f2443f6fbb393e96d16e Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 18 May 2020 14:38:26 -0700 Subject: [PATCH 4/7] parseCPUInfo: optimize line scan Avoid double check of each line for ":" -- since we're splitting the line anyway, we can reuse the result. This also prevents a potential panic of referencing non-existen field[1] in case input contains a ":" with no space after. Signed-off-by: Kir Kolyshkin --- cpuinfo.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cpuinfo.go b/cpuinfo.go index 6dd54ddaa..ddb05f3f4 100644 --- a/cpuinfo.go +++ b/cpuinfo.go @@ -90,10 +90,10 @@ func parseCPUInfoX86(info []byte) ([]CPUInfo, error) { for scanner.Scan() { line := scanner.Text() - if !strings.Contains(line, ":") { + field := strings.SplitN(line, ": ", 2) + if len(field) != 2 { continue } - field := strings.SplitN(line, ": ", 2) switch strings.TrimSpace(field[0]) { case "processor": cpuinfo = append(cpuinfo, CPUInfo{}) // start of the next processor @@ -249,10 +249,10 @@ func parseCPUInfoS390X(info []byte) ([]CPUInfo, error) { for scanner.Scan() { line := scanner.Text() - if !strings.Contains(line, ":") { + field := strings.SplitN(line, ": ", 2) + if len(field) != 2 { continue } - field := strings.SplitN(line, ": ", 2) switch strings.TrimSpace(field[0]) { case "bogomips per cpu": v, err := strconv.ParseFloat(field[1], 64) @@ -322,10 +322,10 @@ func parseCPUInfoPPC(info []byte) ([]CPUInfo, error) { for scanner.Scan() { line := scanner.Text() - if !strings.Contains(line, ":") { + field := strings.SplitN(line, ": ", 2) + if len(field) != 2 { continue } - field := strings.SplitN(line, ": ", 2) switch strings.TrimSpace(field[0]) { case "processor": cpuinfo = append(cpuinfo, CPUInfo{}) // start of the next processor From a1759167bc23f0fc92dba7e47994a7c15c828b5a Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 18 May 2020 15:16:46 -0700 Subject: [PATCH 5/7] cpuinfo: read data in place Instead of reading the data to the buffer, then making a reader and a scanner out of that buffer, let's analyze the data as we read it line by line. Signed-off-by: Kir Kolyshkin --- cpuinfo.go | 26 +++++++++++++------------- cpuinfo_test.go | 15 +++++++++------ 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/cpuinfo.go b/cpuinfo.go index ddb05f3f4..054b8befd 100644 --- a/cpuinfo.go +++ b/cpuinfo.go @@ -17,13 +17,12 @@ package procfs import ( "bufio" - "bytes" "errors" + "io" + "os" "regexp" "strconv" "strings" - - "github.com/prometheus/procfs/internal/util" ) // CPUInfo contains general information about a system CPU found in /proc/cpuinfo @@ -64,15 +63,16 @@ var ( // CPUInfo returns information about current system CPUs. // See https://www.kernel.org/doc/Documentation/filesystems/proc.txt func (fs FS) CPUInfo() ([]CPUInfo, error) { - data, err := util.ReadFileNoStat(fs.proc.Path("cpuinfo")) + file, err := os.Open(fs.proc.Path("cpuinfo")) if err != nil { return nil, err } - return parseCPUInfo(data) + defer file.Close() + return parseCPUInfo(file) } -func parseCPUInfoX86(info []byte) ([]CPUInfo, error) { - scanner := bufio.NewScanner(bytes.NewReader(info)) +func parseCPUInfoX86(info io.Reader) ([]CPUInfo, error) { + scanner := bufio.NewScanner(info) // find the first "processor" line firstLine := firstNonEmptyLine(scanner) @@ -186,8 +186,8 @@ func parseCPUInfoX86(info []byte) ([]CPUInfo, error) { return cpuinfo, nil } -func parseCPUInfoARM(info []byte) ([]CPUInfo, error) { - scanner := bufio.NewScanner(bytes.NewReader(info)) +func parseCPUInfoARM(info io.Reader) ([]CPUInfo, error) { + scanner := bufio.NewScanner(info) cpuinfo := []CPUInfo{{}} i := 0 @@ -236,8 +236,8 @@ func parseCPUInfoARM(info []byte) ([]CPUInfo, error) { return cpuinfo, nil } -func parseCPUInfoS390X(info []byte) ([]CPUInfo, error) { - scanner := bufio.NewScanner(bytes.NewReader(info)) +func parseCPUInfoS390X(info io.Reader) ([]CPUInfo, error) { + scanner := bufio.NewScanner(info) firstLine := firstNonEmptyLine(scanner) if !strings.HasPrefix(firstLine, "vendor_id") || !strings.Contains(firstLine, ":") { @@ -304,8 +304,8 @@ func parseCPUInfoS390X(info []byte) ([]CPUInfo, error) { return cpuinfo, nil } -func parseCPUInfoPPC(info []byte) ([]CPUInfo, error) { - scanner := bufio.NewScanner(bytes.NewReader(info)) +func parseCPUInfoPPC(info io.Reader) ([]CPUInfo, error) { + scanner := bufio.NewScanner(info) firstLine := firstNonEmptyLine(scanner) if !strings.HasPrefix(firstLine, "processor") || !strings.Contains(firstLine, ":") { diff --git a/cpuinfo_test.go b/cpuinfo_test.go index f4966869c..492a1a8b3 100644 --- a/cpuinfo_test.go +++ b/cpuinfo_test.go @@ -15,7 +15,10 @@ package procfs -import "testing" +import ( + "strings" + "testing" +) const ( // non-SMP system before kernel v3.8, commit @@ -177,7 +180,7 @@ func TestCPUInfoX86(t *testing.T) { } func TestCPUInfoParseARM6(t *testing.T) { - cpuinfo, err := parseCPUInfoARM([]byte(cpuinfoArm6)) + cpuinfo, err := parseCPUInfoARM(strings.NewReader(cpuinfoArm6)) if err != nil || cpuinfo == nil { t.Fatalf("unable to parse arm cpu info: %v", err) } @@ -192,7 +195,7 @@ func TestCPUInfoParseARM6(t *testing.T) { } } func TestCPUInfoParseARM7old(t *testing.T) { - cpuinfo, err := parseCPUInfoARM([]byte(cpuinfoArm7old)) + cpuinfo, err := parseCPUInfoARM(strings.NewReader(cpuinfoArm7old)) if err != nil || cpuinfo == nil { t.Fatalf("unable to parse arm cpu info: %v", err) } @@ -208,7 +211,7 @@ func TestCPUInfoParseARM7old(t *testing.T) { } func TestCPUInfoParseARM(t *testing.T) { - cpuinfo, err := parseCPUInfoARM([]byte(cpuinfoArm7)) + cpuinfo, err := parseCPUInfoARM(strings.NewReader(cpuinfoArm7)) if err != nil || cpuinfo == nil { t.Fatalf("unable to parse arm cpu info: %v", err) } @@ -223,7 +226,7 @@ func TestCPUInfoParseARM(t *testing.T) { } } func TestCPUInfoParseS390X(t *testing.T) { - cpuinfo, err := parseCPUInfoS390X([]byte(cpuinfoS390x)) + cpuinfo, err := parseCPUInfoS390X(strings.NewReader(cpuinfoS390x)) if err != nil || cpuinfo == nil { t.Fatalf("unable to parse s390x cpu info: %v", err) } @@ -242,7 +245,7 @@ func TestCPUInfoParseS390X(t *testing.T) { } func TestCPUInfoParsePPC(t *testing.T) { - cpuinfo, err := parseCPUInfoPPC([]byte(cpuinfoPpc64)) + cpuinfo, err := parseCPUInfoPPC(strings.NewReader(cpuinfoPpc64)) if err != nil || cpuinfo == nil { t.Fatalf("unable to parse ppc cpu info: %v", err) } From a6effd3423c5d8a7fb64a443c02a107ee66fb64f Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 18 May 2020 15:19:44 -0700 Subject: [PATCH 6/7] parseCPUInfo*: add error checking In case the input can't be read, scanner.Scan() stops and the code is supposed to check for error using scanner.Err(). Do that. In other words, instead of returning half-read cpuinfo, return an error. Signed-off-by: Kir Kolyshkin --- cpuinfo.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cpuinfo.go b/cpuinfo.go index 054b8befd..83b04991e 100644 --- a/cpuinfo.go +++ b/cpuinfo.go @@ -183,7 +183,7 @@ func parseCPUInfoX86(info io.Reader) ([]CPUInfo, error) { cpuinfo[i].PowerManagement = field[1] } } - return cpuinfo, nil + return cpuinfo, scanner.Err() } func parseCPUInfoARM(info io.Reader) ([]CPUInfo, error) { @@ -233,7 +233,7 @@ func parseCPUInfoARM(info io.Reader) ([]CPUInfo, error) { } } - return cpuinfo, nil + return cpuinfo, scanner.Err() } func parseCPUInfoS390X(info io.Reader) ([]CPUInfo, error) { @@ -301,7 +301,7 @@ func parseCPUInfoS390X(info io.Reader) ([]CPUInfo, error) { } } - return cpuinfo, nil + return cpuinfo, scanner.Err() } func parseCPUInfoPPC(info io.Reader) ([]CPUInfo, error) { @@ -346,7 +346,7 @@ func parseCPUInfoPPC(info io.Reader) ([]CPUInfo, error) { cpuinfo[i].CPUMHz = v } } - return cpuinfo, nil + return cpuinfo, scanner.Err() } // firstNonEmptyLine advances the scanner to the first non-empty line From e1d29d85f9b6a587b6c7394ddcab8b06b169be3e Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Mon, 18 May 2020 15:26:03 -0700 Subject: [PATCH 7/7] cpuinfo: rm GetFirstNonEmptyLine, simplify scan There is no sense to skip empty lines at the beginning of the file since there are none (there were in test data but it is fixed by an earlier commit). Logic is simpler now. Signed-off-by: Kir Kolyshkin --- cpuinfo.go | 50 +++++++------------------------------------------- 1 file changed, 7 insertions(+), 43 deletions(-) diff --git a/cpuinfo.go b/cpuinfo.go index 83b04991e..8c299516b 100644 --- a/cpuinfo.go +++ b/cpuinfo.go @@ -74,19 +74,8 @@ func (fs FS) CPUInfo() ([]CPUInfo, error) { func parseCPUInfoX86(info io.Reader) ([]CPUInfo, error) { scanner := bufio.NewScanner(info) - // find the first "processor" line - firstLine := firstNonEmptyLine(scanner) - if !strings.HasPrefix(firstLine, "processor") || !strings.Contains(firstLine, ":") { - return nil, errors.New("invalid cpuinfo file: " + firstLine) - } - field := strings.SplitN(firstLine, ": ", 2) - v, err := strconv.ParseUint(field[1], 0, 32) - if err != nil { - return nil, err - } - firstcpu := CPUInfo{Processor: uint(v)} - cpuinfo := []CPUInfo{firstcpu} - i := 0 + cpuinfo := []CPUInfo{} + i := -1 for scanner.Scan() { line := scanner.Text() @@ -239,13 +228,8 @@ func parseCPUInfoARM(info io.Reader) ([]CPUInfo, error) { func parseCPUInfoS390X(info io.Reader) ([]CPUInfo, error) { scanner := bufio.NewScanner(info) - firstLine := firstNonEmptyLine(scanner) - if !strings.HasPrefix(firstLine, "vendor_id") || !strings.Contains(firstLine, ":") { - return nil, errors.New("invalid cpuinfo file: " + firstLine) - } - field := strings.SplitN(firstLine, ": ", 2) cpuinfo := []CPUInfo{} - commonCPUInfo := CPUInfo{VendorID: field[1]} + commonCPUInfo := CPUInfo{} for scanner.Scan() { line := scanner.Text() @@ -254,6 +238,8 @@ func parseCPUInfoS390X(info io.Reader) ([]CPUInfo, error) { continue } switch strings.TrimSpace(field[0]) { + case "vendor_id": + commonCPUInfo.VendorID = field[1] case "bogomips per cpu": v, err := strconv.ParseFloat(field[1], 64) if err != nil { @@ -307,18 +293,8 @@ func parseCPUInfoS390X(info io.Reader) ([]CPUInfo, error) { func parseCPUInfoPPC(info io.Reader) ([]CPUInfo, error) { scanner := bufio.NewScanner(info) - firstLine := firstNonEmptyLine(scanner) - if !strings.HasPrefix(firstLine, "processor") || !strings.Contains(firstLine, ":") { - return nil, errors.New("invalid cpuinfo file: " + firstLine) - } - field := strings.SplitN(firstLine, ": ", 2) - v, err := strconv.ParseUint(field[1], 0, 32) - if err != nil { - return nil, err - } - firstcpu := CPUInfo{Processor: uint(v)} - cpuinfo := []CPUInfo{firstcpu} - i := 0 + cpuinfo := []CPUInfo{} + i := -1 for scanner.Scan() { line := scanner.Text() @@ -348,15 +324,3 @@ func parseCPUInfoPPC(info io.Reader) ([]CPUInfo, error) { } return cpuinfo, scanner.Err() } - -// firstNonEmptyLine advances the scanner to the first non-empty line -// and returns the contents of that line -func firstNonEmptyLine(scanner *bufio.Scanner) string { - for scanner.Scan() { - line := scanner.Text() - if strings.TrimSpace(line) != "" { - return line - } - } - return "" -}