341 lines
12 KiB
Diff
341 lines
12 KiB
Diff
From 3e9e0c7f5d1a54b7688924243cc14707906a14cf Mon Sep 17 00:00:00 2001
|
|
From: eho <2220386943@qq.com>
|
|
Date: Wed, 25 Sep 2024 16:34:45 +0800
|
|
Subject: [PATCH 03/20] fix after pmuopen delete evtList exception if you
|
|
delete the pointer pointed to by attr.evtList immediately after pmuuopen is
|
|
successful, the event will be garbled.
|
|
|
|
---
|
|
build/common.sh | 4 ++
|
|
pmu/pmu.cpp | 108 +++++++++++++++++++++++-----------
|
|
pmu/pmu_list.cpp | 26 +++++---
|
|
pmu/pmu_list.h | 6 +-
|
|
test/test_perf/test_count.cpp | 57 ++++++++++++++++++
|
|
5 files changed, 155 insertions(+), 46 deletions(-)
|
|
|
|
diff --git a/build/common.sh b/build/common.sh
|
|
index 9cc16b0..f48b64e 100644
|
|
--- a/build/common.sh
|
|
+++ b/build/common.sh
|
|
@@ -15,6 +15,10 @@ set -e
|
|
|
|
cpu_core_num=$(($(nproc)-1))
|
|
|
|
+if [ "$cpu_core_num" -eq 0 ];then
|
|
+ cpu_core_num=1
|
|
+fi
|
|
+
|
|
creat_dir(){
|
|
local target_dir="$1"
|
|
if [ -d "${target_dir}" ];then
|
|
diff --git a/pmu/pmu.cpp b/pmu/pmu.cpp
|
|
index b7507ec..207d4fb 100644
|
|
--- a/pmu/pmu.cpp
|
|
+++ b/pmu/pmu.cpp
|
|
@@ -160,17 +160,29 @@ static int CheckAttr(enum PmuTaskType collectType, struct PmuAttr *attr)
|
|
return LIBPERF_ERR_INVALID_SAMPLE_RATE;
|
|
}
|
|
|
|
- if ((collectType == SAMPLING || collectType == COUNTING) && attr->evtAttr == nullptr) {
|
|
- struct EvtAttr *evtAttr = new struct EvtAttr[attr->numEvt];
|
|
+ return SUCCESS;
|
|
+}
|
|
+
|
|
+static void CopyAttrData(PmuAttr* newAttr, PmuAttr* inputAttr, enum PmuTaskType collectType)
|
|
+{
|
|
+ //Coping event data to prevent delete exceptions
|
|
+ if (inputAttr->numEvt > 0) {
|
|
+ char **newEvtList = new char *[inputAttr->numEvt];
|
|
+ for (int i = 0; i < inputAttr->numEvt; ++i) {
|
|
+ newEvtList[i] = new char[strlen(inputAttr->evtList[i]) + 1];
|
|
+ strcpy(newEvtList[i], inputAttr->evtList[i]);
|
|
+ }
|
|
+ newAttr->evtList = newEvtList;
|
|
+ }
|
|
+
|
|
+ if ((collectType == SAMPLING || collectType == COUNTING) && inputAttr->evtAttr == nullptr) {
|
|
+ struct EvtAttr *evtAttr = new struct EvtAttr[inputAttr->numEvt];
|
|
// handle event group id. -1 means that it doesn't run event group feature.
|
|
- for (int i = 0; i < attr->numEvt; ++i) {
|
|
+ for (int i = 0; i < inputAttr->numEvt; ++i) {
|
|
evtAttr[i].group_id = -1;
|
|
}
|
|
- attr->evtAttr = evtAttr;
|
|
-
|
|
+ newAttr->evtAttr = evtAttr;
|
|
}
|
|
-
|
|
- return SUCCESS;
|
|
}
|
|
|
|
static bool FreeEvtAttr(struct PmuAttr *attr)
|
|
@@ -196,6 +208,21 @@ static bool FreeEvtAttr(struct PmuAttr *attr)
|
|
return SUCCESS;
|
|
}
|
|
|
|
+static void FreeEvtList(unsigned evtNum, char** evtList)
|
|
+{
|
|
+ if (!evtList) {
|
|
+ return;
|
|
+ }
|
|
+ for (int i = 0; i < evtNum; i++) {
|
|
+ if (evtList[i]) {
|
|
+ delete[] evtList[i];
|
|
+ evtList[i] = nullptr;
|
|
+ }
|
|
+ }
|
|
+ delete[] evtList;
|
|
+ evtList = nullptr;
|
|
+}
|
|
+
|
|
static bool AppendChildEvents(char* evt, unordered_map<string, char*>& eventSplitMap)
|
|
{
|
|
string strName(evt);
|
|
@@ -304,51 +331,64 @@ static void PmuTaskAttrFree(PmuTaskAttr *taskAttr)
|
|
int PmuOpen(enum PmuTaskType collectType, struct PmuAttr *attr)
|
|
{
|
|
SetWarn(SUCCESS);
|
|
+ PmuAttr copiedAttr = {*attr};
|
|
+ pair<unsigned, char**> previousEventList = {0, nullptr};
|
|
try {
|
|
auto err = CheckAttr(collectType, attr);
|
|
if (err != SUCCESS) {
|
|
return -1;
|
|
}
|
|
+ CopyAttrData(&copiedAttr, attr, collectType);
|
|
+ previousEventList = make_pair(copiedAttr.numEvt, copiedAttr.evtList);
|
|
+ int pd = -1;
|
|
unordered_map<string, char*> eventSplitMap;
|
|
- if (!SplitUncoreEvent(attr, eventSplitMap)) {
|
|
- return -1;
|
|
- }
|
|
- auto previousEventList = make_pair(attr->numEvt, attr->evtList);
|
|
- vector<char*> newEvtlist;
|
|
- vector<struct EvtAttr> newEvtAttrList;
|
|
- auto numEvt = GenerateSplitList(eventSplitMap, newEvtlist, attr, newEvtAttrList);
|
|
- FreeEvtAttr(attr);
|
|
- attr->numEvt = numEvt;
|
|
- attr->evtList = newEvtlist.data();
|
|
- attr->evtAttr = newEvtAttrList.data();
|
|
-
|
|
- auto pTaskAttr = AssignPmuTaskParam(collectType, attr);
|
|
- if (pTaskAttr == nullptr) {
|
|
- return -1;
|
|
- }
|
|
- unique_ptr<PmuTaskAttr, void (*)(PmuTaskAttr*)> taskAttr(pTaskAttr, PmuTaskAttrFree);
|
|
+ do {
|
|
+ if (!SplitUncoreEvent(&copiedAttr, eventSplitMap)) {
|
|
+ break;
|
|
+ }
|
|
+ vector<char *> newEvtlist;
|
|
+ vector<struct EvtAttr> newEvtAttrList;
|
|
+ auto numEvt = GenerateSplitList(eventSplitMap, newEvtlist, &copiedAttr, newEvtAttrList);
|
|
+ FreeEvtAttr(&copiedAttr);
|
|
+ copiedAttr.numEvt = numEvt;
|
|
+ copiedAttr.evtList = newEvtlist.data();
|
|
+ copiedAttr.evtAttr = newEvtAttrList.data();
|
|
+
|
|
+ auto pTaskAttr = AssignPmuTaskParam(collectType, &copiedAttr);
|
|
+ if (pTaskAttr == nullptr) {
|
|
+ break;
|
|
+ }
|
|
+ unique_ptr<PmuTaskAttr, void (*)(PmuTaskAttr *)> taskAttr(pTaskAttr, PmuTaskAttrFree);
|
|
+
|
|
+ pd = KUNPENG_PMU::PmuList::GetInstance()->NewPd();
|
|
+ if (pd == -1) {
|
|
+ New(LIBPERF_ERR_NO_AVAIL_PD);
|
|
+ break;
|
|
+ }
|
|
+
|
|
+ KUNPENG_PMU::PmuList::GetInstance()->SetSymbolMode(pd, attr->symbolMode);
|
|
+ err = KUNPENG_PMU::PmuList::GetInstance()->Register(pd, taskAttr.get());
|
|
+ if (err != SUCCESS) {
|
|
+ PmuList::GetInstance()->Close(pd);
|
|
+ pd = -1;
|
|
+ }
|
|
+ New(err);
|
|
+ } while(false);
|
|
|
|
- auto pd = KUNPENG_PMU::PmuList::GetInstance()->NewPd();
|
|
if (pd == -1) {
|
|
- New(LIBPERF_ERR_NO_AVAIL_PD);
|
|
+ FreeEvtList(previousEventList.first, previousEventList.second);
|
|
return -1;
|
|
}
|
|
-
|
|
- KUNPENG_PMU::PmuList::GetInstance()->SetSymbolMode(pd, attr->symbolMode);
|
|
- err = KUNPENG_PMU::PmuList::GetInstance()->Register(pd, taskAttr.get());
|
|
- if (err != SUCCESS) {
|
|
- PmuList::GetInstance()->Close(pd);
|
|
- pd = -1;
|
|
- }
|
|
// store eventList provided by user and the mapping relationship between the user eventList and the split
|
|
// eventList into buff
|
|
KUNPENG_PMU::PmuList::GetInstance()->StoreSplitData(pd, previousEventList, eventSplitMap);
|
|
- New(err);
|
|
return pd;
|
|
} catch (std::bad_alloc&) {
|
|
+ FreeEvtList(previousEventList.first, previousEventList.second);
|
|
New(COMMON_ERR_NOMEM);
|
|
return -1;
|
|
} catch (exception& ex) {
|
|
+ FreeEvtList(previousEventList.first, previousEventList.second);
|
|
New(UNKNOWN_ERROR, ex.what());
|
|
return -1;
|
|
}
|
|
diff --git a/pmu/pmu_list.cpp b/pmu/pmu_list.cpp
|
|
index 0caba7b..4f293a7 100644
|
|
--- a/pmu/pmu_list.cpp
|
|
+++ b/pmu/pmu_list.cpp
|
|
@@ -283,8 +283,8 @@ namespace KUNPENG_PMU {
|
|
return SUCCESS;
|
|
}
|
|
|
|
- void PmuList::StoreSplitData(const unsigned pd, pair<unsigned, char**> previousEventList,
|
|
- unordered_map<string, char*> eventSplitMap)
|
|
+ void PmuList::StoreSplitData(const unsigned pd, pair<unsigned, char**>& previousEventList,
|
|
+ unordered_map<string, char*>& eventSplitMap)
|
|
{
|
|
lock_guard<mutex> lg(dataParentMtx);
|
|
parentEventMap.emplace(pd, move(eventSplitMap));
|
|
@@ -303,7 +303,7 @@ namespace KUNPENG_PMU {
|
|
RemoveEpollFd(pd);
|
|
EraseSpeCpu(pd);
|
|
EraseDummyEvent(pd);
|
|
- EraseParentEventMap();
|
|
+ EraseParentEventMap(pd);
|
|
SymResolverDestroy();
|
|
PmuEventListFree();
|
|
PointerPasser::FreeRawFieldMap();
|
|
@@ -409,15 +409,23 @@ namespace KUNPENG_PMU {
|
|
return dataEvtGroupList[pd];
|
|
}
|
|
|
|
- void PmuList::EraseParentEventMap()
|
|
+ void PmuList::EraseParentEventMap(const unsigned pd)
|
|
{
|
|
lock_guard<mutex> lg(dataParentMtx);
|
|
- for (auto& pair: parentEventMap) {
|
|
- auto& innerMap = pair.second;
|
|
- innerMap.clear();
|
|
+ auto iter = parentEventMap.find(pd);
|
|
+ if (iter != parentEventMap.end()) {
|
|
+ parentEventMap.at(pd).clear();
|
|
+ parentEventMap.erase(iter);
|
|
+ }
|
|
+ auto preIter = previousEventMap.find(pd);
|
|
+ if (preIter != previousEventMap.end()) {
|
|
+ auto pair = previousEventMap.at(pd);
|
|
+ for (int i = 0; i < pair.first; i++) {
|
|
+ delete[] pair.second[i];
|
|
+ }
|
|
+ delete[] pair.second;
|
|
+ previousEventMap.erase(preIter);
|
|
}
|
|
- parentEventMap.clear();
|
|
- previousEventMap.clear();
|
|
}
|
|
|
|
PmuList::EventData& PmuList::GetDataList(const unsigned pd)
|
|
diff --git a/pmu/pmu_list.h b/pmu/pmu_list.h
|
|
index 80346eb..13724f8 100644
|
|
--- a/pmu/pmu_list.h
|
|
+++ b/pmu/pmu_list.h
|
|
@@ -71,8 +71,8 @@ public:
|
|
int NewPd();
|
|
|
|
int GetHistoryData(const int pd, std::vector<PmuData>& pmuData);
|
|
- void StoreSplitData(unsigned pd, std::pair<unsigned, char**> previousEventList,
|
|
- std::unordered_map<std::string, char*> eventSplitMap);
|
|
+ void StoreSplitData(unsigned pd, std::pair<unsigned, char**>& previousEventList,
|
|
+ std::unordered_map<std::string, char*>& eventSplitMap);
|
|
bool IsAllPidExit(const unsigned pd);
|
|
|
|
private:
|
|
@@ -95,7 +95,7 @@ private:
|
|
void InsertEvtList(const unsigned pd, std::shared_ptr<EvtList> evtList);
|
|
std::vector<std::shared_ptr<EvtList>>& GetEvtList(const unsigned pd);
|
|
void EraseEvtList(const unsigned pd);
|
|
- void EraseParentEventMap();
|
|
+ void EraseParentEventMap(const unsigned pd);
|
|
|
|
int EvtInit(const bool groupEnable, const std::shared_ptr<EvtList> evtLeader, const int pd, const std::shared_ptr<EvtList> &evtList);
|
|
int Init(const int pd);
|
|
diff --git a/test/test_perf/test_count.cpp b/test/test_perf/test_count.cpp
|
|
index 1fda697..1b64497 100644
|
|
--- a/test/test_perf/test_count.cpp
|
|
+++ b/test/test_perf/test_count.cpp
|
|
@@ -13,6 +13,7 @@
|
|
* Description: Unit test for counting.
|
|
******************************************************************************/
|
|
#include "test_common.h"
|
|
+#include <dirent.h>
|
|
|
|
using namespace std;
|
|
|
|
@@ -307,4 +308,60 @@ TEST_F(TestCount, SimdRatio)
|
|
ASSERT_EQ(evtMap.size(), evts.size());
|
|
auto simdRatio = (double)evtMap[aseSpec]/evtMap[instSpec];
|
|
ASSERT_GT(simdRatio, 0.1);
|
|
+}
|
|
+
|
|
+static std::vector<string> GetHHADirs() {
|
|
+ vector<string> hhaEvents;
|
|
+ unique_ptr<DIR, decltype(&closedir)> dir(opendir("/sys/devices"), &closedir);
|
|
+ if(!dir) {
|
|
+ return hhaEvents;
|
|
+ }
|
|
+
|
|
+ struct dirent* dt;
|
|
+ while((dt = readdir(dir.get())) != nullptr) {
|
|
+ std::string name = dt->d_name;
|
|
+ if(name == "." || name == "..") {
|
|
+ continue;
|
|
+ }
|
|
+
|
|
+ if(dt->d_type == DT_DIR && strstr(name.c_str(), "hha") != nullptr) {
|
|
+ hhaEvents.push_back(name + "/");
|
|
+ }
|
|
+ }
|
|
+ return hhaEvents;
|
|
+}
|
|
+
|
|
+
|
|
+TEST_F(TestCount, DeleteEvtAfterOpenPmuu) {
|
|
+ struct PmuAttr attr = {nullptr};
|
|
+ vector<string> hhaEvents = GetHHADirs();
|
|
+ ASSERT_TRUE(hhaEvents.size() > 0);
|
|
+ set<string> evtNames;
|
|
+ vector<string> eventsStr = {"rx_outer", "rx_sccl", "rx_ops_num"};
|
|
+ int evtNum = hhaEvents.size() * eventsStr.size();
|
|
+ char **evtList = new char *[evtNum];
|
|
+ for (int i = 0; i < hhaEvents.size(); ++i) {
|
|
+ for (int j = 0; j < eventsStr.size(); ++j) {
|
|
+ int evtLen = hhaEvents[i].size() + eventsStr[j].size() + 2;
|
|
+ evtList[i * eventsStr.size() + j] = new char[evtLen];
|
|
+ snprintf(evtList[i * eventsStr.size() + j], evtLen, "%s%s/", hhaEvents[i].c_str(), eventsStr[j].c_str());
|
|
+ evtNames.emplace(hhaEvents[i] + eventsStr[j] + "/");
|
|
+ }
|
|
+ }
|
|
+ attr.evtList = evtList;
|
|
+ attr.numEvt = evtNum;
|
|
+ int pd = PmuOpen(COUNTING, &attr);
|
|
+ for (int i = 0; i < evtNum; i++) {
|
|
+ delete[] evtList[i];
|
|
+ }
|
|
+ delete[] evtList;
|
|
+ PmuEnable(pd);
|
|
+ sleep(1);
|
|
+ PmuDisable(pd);
|
|
+ PmuData* pmuData = nullptr;
|
|
+ int len = PmuRead(pd, &pmuData);
|
|
+ for(int i = 0; i < len; i++) {
|
|
+ ASSERT_TRUE(evtNames.find(pmuData[i].evt) != evtNames.end());
|
|
+ }
|
|
+ PmuClose(pd);
|
|
}
|
|
\ No newline at end of file
|
|
--
|
|
2.43.0
|
|
|