diff --git a/perf/collector_libpfm.go b/perf/collector_libpfm.go index 09d271eb029..e4ee08a36a6 100644 --- a/perf/collector_libpfm.go +++ b/perf/collector_libpfm.go @@ -46,6 +46,10 @@ type collector struct { onlineCPUs []int eventToCustomEvent map[Event]*CustomEvent uncore stats.Collector + + // Handle for mocking purposes. + perfEventOpen func(attr *unix.PerfEventAttr, pid int, cpu int, groupFd int, flags int) (fd int, err error) + ioctlSetInt func(fd int, req uint, value int) error } type group struct { @@ -75,7 +79,7 @@ func init() { } func newCollector(cgroupPath string, events PerfEvents, onlineCPUs []int, cpuToSocket map[int]int) *collector { - collector := &collector{cgroupPath: cgroupPath, events: events, onlineCPUs: onlineCPUs, cpuFiles: map[int]group{}, uncore: NewUncoreCollector(cgroupPath, events, cpuToSocket)} + collector := &collector{cgroupPath: cgroupPath, events: events, onlineCPUs: onlineCPUs, cpuFiles: map[int]group{}, uncore: NewUncoreCollector(cgroupPath, events, cpuToSocket), perfEventOpen: unix.PerfEventOpen, ioctlSetInt: unix.IoctlSetInt} mapEventsToCustomEvents(collector) return collector } @@ -184,44 +188,29 @@ func (c *collector) setup() error { c.cpuFilesLock.Lock() defer c.cpuFilesLock.Unlock() cgroupFd := int(cgroup.Fd()) - for i, group := range c.events.Core.Events { + groupIndex := 0 + for _, group := range c.events.Core.Events { // CPUs file descriptors of group leader needed for perf_event_open. leaderFileDescriptors := make(map[int]int, len(c.onlineCPUs)) for _, cpu := range c.onlineCPUs { leaderFileDescriptors[cpu] = groupLeaderFileDescriptor } - for j, event := range group.events { - // First element is group leader. - isGroupLeader := j == 0 - customEvent, ok := c.eventToCustomEvent[event] - if ok { - config := c.createConfigFromRawEvent(customEvent) - leaderFileDescriptors, err = c.registerEvent(eventInfo{string(customEvent.Name), config, cgroupFd, i, isGroupLeader}, leaderFileDescriptors) - if err != nil { - return err - } - } else { - config, err := c.createConfigFromEvent(event) - if err != nil { - return err - } - leaderFileDescriptors, err = c.registerEvent(eventInfo{string(event), config, cgroupFd, i, isGroupLeader}, leaderFileDescriptors) - if err != nil { - return err - } - // Clean memory allocated by C code. - C.free(unsafe.Pointer(config)) - } + leaderFileDescriptors, err := c.createLeaderFileDescriptors(group.events, cgroupFd, groupIndex, leaderFileDescriptors) + if err != nil { + klog.Error(err) + continue + } else { + groupIndex++ } // Group is prepared so we should reset and enable counting. for _, fd := range leaderFileDescriptors { - err = unix.IoctlSetInt(fd, unix.PERF_EVENT_IOC_RESET, 0) + err = c.ioctlSetInt(fd, unix.PERF_EVENT_IOC_RESET, 0) if err != nil { return err } - err = unix.IoctlSetInt(fd, unix.PERF_EVENT_IOC_ENABLE, 0) + err = c.ioctlSetInt(fd, unix.PERF_EVENT_IOC_ENABLE, 0) if err != nil { return err } @@ -231,6 +220,35 @@ func (c *collector) setup() error { return nil } +func (c *collector) createLeaderFileDescriptors(events []Event, cgroupFd int, groupIndex int, leaderFileDescriptors map[int]int) (map[int]int, error) { + for j, event := range events { + // First element is group leader. + isGroupLeader := j == 0 + customEvent, ok := c.eventToCustomEvent[event] + var err error + if ok { + config := c.createConfigFromRawEvent(customEvent) + leaderFileDescriptors, err = c.registerEvent(eventInfo{string(customEvent.Name), config, cgroupFd, groupIndex, isGroupLeader}, leaderFileDescriptors) + if err != nil { + return nil, fmt.Errorf("unable to register perf event %v", err) + } + } else { + config, err := c.createConfigFromEvent(event) + if err != nil { + return nil, fmt.Errorf("unable create config from perf event %v", err) + + } + leaderFileDescriptors, err = c.registerEvent(eventInfo{string(event), config, cgroupFd, groupIndex, isGroupLeader}, leaderFileDescriptors) + if err != nil { + return nil, fmt.Errorf("unable to register perf event %v", err) + } + // Clean memory allocated by C code. + C.free(unsafe.Pointer(config)) + } + } + return leaderFileDescriptors, nil +} + func readPerfEventAttr(name string) (*unix.PerfEventAttr, error) { perfEventAttrMemory := C.malloc(C.ulong(unsafe.Sizeof(unix.PerfEventAttr{}))) event := pfmPerfEncodeArgT{} @@ -269,13 +287,13 @@ func (c *collector) registerEvent(event eventInfo, leaderFileDescriptors map[int setAttributes(event.config, event.isGroupLeader) for _, cpu := range c.onlineCPUs { - fd, err := unix.PerfEventOpen(event.config, pid, cpu, leaderFileDescriptors[cpu], flags) + fd, err := c.perfEventOpen(event.config, pid, cpu, leaderFileDescriptors[cpu], flags) if err != nil { - return nil, fmt.Errorf("setting up perf event %#v failed: %q", event.config, err) + return leaderFileDescriptors, fmt.Errorf("setting up perf event %#v failed: %q", event.config, err) } perfFile := os.NewFile(uintptr(fd), event.name) if perfFile == nil { - return nil, fmt.Errorf("unable to create os.File from file descriptor %#v", fd) + return leaderFileDescriptors, fmt.Errorf("unable to create os.File from file descriptor %#v", fd) } c.addEventFile(event.groupIndex, event.name, cpu, perfFile) diff --git a/perf/collector_libpfm_test.go b/perf/collector_libpfm_test.go index 4f43f0d6821..e2598383f05 100644 --- a/perf/collector_libpfm_test.go +++ b/perf/collector_libpfm_test.go @@ -20,6 +20,8 @@ package perf import ( "bytes" "encoding/binary" + "golang.org/x/sys/unix" + "os" "testing" "github.com/stretchr/testify/assert" @@ -197,6 +199,34 @@ func TestNewCollector(t *testing.T) { assert.Same(t, &perfCollector.events.Core.CustomEvents[0], perfCollector.eventToCustomEvent[Event("event_2")]) } +func TestCollector_Setup(t *testing.T) { + path, err := mockSystemDevices() + assert.Nil(t, err) + defer func() { + err := os.RemoveAll(path) + assert.Nil(t, err) + }() + events := PerfEvents{ + Core: Events{ + Events: []Group{ + {[]Event{"cache-misses"}, false}, + {[]Event{"non-existing-event"}, false}, + }, + }, + } + c := newCollector(path, events, []int{0}, map[int]int{0: 0}) + c.perfEventOpen = func(attr *unix.PerfEventAttr, pid int, cpu int, groupFd int, flags int) (fd int, err error) { + return int(attr.Config), nil + } + c.ioctlSetInt = func(fd int, req uint, value int) error { + return nil + } + err = c.setup() + assert.Nil(t, err) + assert.Equal(t, 1, len(c.cpuFiles)) + assert.Equal(t, []string{"cache-misses"}, c.cpuFiles[0].names) +} + var readGroupPerfStatCases = []struct { test string file GroupReadFormat diff --git a/perf/uncore_libpfm.go b/perf/uncore_libpfm.go index 2e4cf20d53a..338587e6156 100644 --- a/perf/uncore_libpfm.go +++ b/perf/uncore_libpfm.go @@ -202,7 +202,7 @@ func (c *uncoreCollector) setup(events PerfEvents, devicesPath string) error { } if err != nil { - return err + klog.Error(err) } } diff --git a/perf/uncore_libpfm_test.go b/perf/uncore_libpfm_test.go index d5b2ab89578..490f528ad58 100644 --- a/perf/uncore_libpfm_test.go +++ b/perf/uncore_libpfm_test.go @@ -114,6 +114,7 @@ func TestUncoreCollectorSetup(t *testing.T) { Uncore: Events{ Events: []Group{ {[]Event{"uncore_imc_1/cas_count_read"}, false}, + {[]Event{"uncore_imc_1/non_existing_event"}, false}, {[]Event{"uncore_imc_0/cas_count_write", "uncore_imc_0/cas_count_read"}, true}, }, CustomEvents: []CustomEvent{ @@ -133,6 +134,11 @@ func TestUncoreCollectorSetup(t *testing.T) { } err = collector.setup(events, path) + assert.Equal(t, []string{"uncore_imc_1/cas_count_read"}, + getMapKeys(collector.cpuFiles[0]["uncore_imc_1"].cpuFiles)) + assert.ElementsMatch(t, []string{"uncore_imc_0/cas_count_write", "uncore_imc_0/cas_count_read"}, + getMapKeys(collector.cpuFiles[2]["uncore_imc_0"].cpuFiles)) + // There are no errors. assert.Nil(t, err) } @@ -295,3 +301,11 @@ func TestReadPerfUncoreStat(t *testing.T) { assert.NoError(t, err) assert.Equal(t, expectedStat, stat) } + +func getMapKeys(someMap map[string]map[int]readerCloser) []string { + var keys []string + for key, _ := range someMap { + keys = append(keys, key) + } + return keys +}