From de4789097cebff3b3f2fb53505da595b65dfebe5 Mon Sep 17 00:00:00 2001 From: Galaxy Date: Mon, 20 May 2024 03:55:07 -0700 Subject: [PATCH 1/2] Fix memleak of SPE. --- pmu/evt.h | 2 +- pmu/evt_list.cpp | 4 ++-- pmu/evt_list.h | 2 +- pmu/perf_counter.cpp | 2 +- pmu/perf_counter.h | 2 +- pmu/pmu_list.cpp | 5 ++++- pmu/pmu_list.h | 1 + pmu/sampler.cpp | 2 +- pmu/sampler.h | 2 +- pmu/spe_sampler.cpp | 12 ++++-------- pmu/spe_sampler.h | 6 ++---- 11 files changed, 19 insertions(+), 21 deletions(-) diff --git a/pmu/evt.h b/pmu/evt.h index ee27ac9..9eeda80 100644 --- a/pmu/evt.h +++ b/pmu/evt.h @@ -44,7 +44,7 @@ public: virtual int Init() = 0; - virtual int Read(std::vector &data, std::vector &sampleIps) = 0; + virtual int Read(std::vector &data, std::vector &sampleIps, std::vector &extPool) = 0; virtual int MapPerfAttr() = 0; diff --git a/pmu/evt_list.cpp b/pmu/evt_list.cpp index f489161..42d2789 100644 --- a/pmu/evt_list.cpp +++ b/pmu/evt_list.cpp @@ -152,7 +152,7 @@ void KUNPENG_PMU::EvtList::FillFields( } } -int KUNPENG_PMU::EvtList::Read(vector &data, std::vector &sampleIps) +int KUNPENG_PMU::EvtList::Read(vector &data, std::vector &sampleIps, std::vector &extPool) { for (unsigned int row = 0; row < numCpu; row++) { for (unsigned int col = 0; col < numPid; col++) { @@ -168,7 +168,7 @@ int KUNPENG_PMU::EvtList::Read(vector &data, std::vector auto cpuTopo = this->cpuList[row].get(); for (unsigned int col = 0; col < numPid; col++) { auto cnt = data.size(); - int err = this->xyCounterArray[row][col]->Read(data, sampleIps); + int err = this->xyCounterArray[row][col]->Read(data, sampleIps, extPool); if (err != SUCCESS) { return err; } diff --git a/pmu/evt_list.h b/pmu/evt_list.h index 7bb3595..a0aff4d 100644 --- a/pmu/evt_list.h +++ b/pmu/evt_list.h @@ -45,7 +45,7 @@ public: int Enable(); int Stop(); int Reset(); - int Read(std::vector &pmuData, std::vector &sampleIps); + int Read(std::vector &pmuData, std::vector &sampleIps, std::vector &extPool); void SetTimeStamp(const int64_t ×tamp) { diff --git a/pmu/perf_counter.cpp b/pmu/perf_counter.cpp index 2b25d7e..fb8da21 100644 --- a/pmu/perf_counter.cpp +++ b/pmu/perf_counter.cpp @@ -37,7 +37,7 @@ static constexpr int MAX_ATTR_SIZE = 120; * Right now we do not implement grouping logic, thus we ignore the * PERF_FORMAT_ID section for now */ -int KUNPENG_PMU::PerfCounter::Read(vector &data, std::vector &sampleIps) +int KUNPENG_PMU::PerfCounter::Read(vector &data, std::vector &sampleIps, std::vector &extPool) { struct ReadFormat perfCountValue; diff --git a/pmu/perf_counter.h b/pmu/perf_counter.h index 4d50ed4..fee65f8 100644 --- a/pmu/perf_counter.h +++ b/pmu/perf_counter.h @@ -36,7 +36,7 @@ namespace KUNPENG_PMU { ~PerfCounter() {} int Init() override; - int Read(std::vector &data, std::vector &sampleIps) override; + int Read(std::vector &data, std::vector &sampleIps, std::vector &extPool) override; int MapPerfAttr() override; }; } // namespace KUNPENG_PMU diff --git a/pmu/pmu_list.cpp b/pmu/pmu_list.cpp index 15d2319..aaa047a 100644 --- a/pmu/pmu_list.cpp +++ b/pmu/pmu_list.cpp @@ -163,7 +163,7 @@ namespace KUNPENG_PMU { auto eventList = GetEvtList(pd); for (auto item : eventList) { item->SetTimeStamp(ts); - auto err = item->Read(evtData.data, evtData.sampleIps); + auto err = item->Read(evtData.data, evtData.sampleIps, evtData.extPool); if (err != SUCCESS) { return err; } @@ -462,6 +462,9 @@ namespace KUNPENG_PMU { if (findData == userDataList.end()) { return; } + for (auto &extMem : findData->second.extPool) { + delete[] extMem; + } userDataList.erase(pmuData); } diff --git a/pmu/pmu_list.h b/pmu/pmu_list.h index 9d297a2..699261a 100644 --- a/pmu/pmu_list.h +++ b/pmu/pmu_list.h @@ -84,6 +84,7 @@ private: PmuTaskType collectType; std::vector data; std::vector sampleIps; + std::vector extPool; }; void InsertEvtList(const unsigned pd, std::shared_ptr evtList); diff --git a/pmu/sampler.cpp b/pmu/sampler.cpp index c463d6c..dc1f149 100644 --- a/pmu/sampler.cpp +++ b/pmu/sampler.cpp @@ -211,7 +211,7 @@ void KUNPENG_PMU::PerfSampler::FillComm(const size_t &start, const size_t &end, } } -int KUNPENG_PMU::PerfSampler::Read(vector &data, std::vector &sampleIps) +int KUNPENG_PMU::PerfSampler::Read(vector &data, std::vector &sampleIps, std::vector &extPool) { auto err = this->ReadInit(); if (__glibc_unlikely(err != SUCCESS)) { diff --git a/pmu/sampler.h b/pmu/sampler.h index a55405f..5509f61 100644 --- a/pmu/sampler.h +++ b/pmu/sampler.h @@ -40,7 +40,7 @@ namespace KUNPENG_PMU { {} int Init() override; - int Read(std::vector &data, std::vector &sampleIps) override; + int Read(std::vector &data, std::vector &sampleIps, std::vector &extPool) override; int MapPerfAttr() override; diff --git a/pmu/spe_sampler.cpp b/pmu/spe_sampler.cpp index d9ce7f0..62c6d7f 100644 --- a/pmu/spe_sampler.cpp +++ b/pmu/spe_sampler.cpp @@ -65,7 +65,7 @@ namespace KUNPENG_PMU { return SUCCESS; } - int PerfSpe::Read(vector &data, std::vector &sampleIps) + int PerfSpe::Read(vector &data, std::vector &sampleIps, std::vector &extPool) { auto findSpe = speSet.find(this->cpu); if (findSpe == speSet.end()) { @@ -90,14 +90,14 @@ namespace KUNPENG_PMU { continue; } // Insert each spe record for each tid. - InsertSpeRecords(records.first, records.second, data, sampleIps); + InsertSpeRecords(records.first, records.second, data, sampleIps, extPool); } } else { // Loop over all tids. for (auto &proc : procMap) { // Get all spe records for tid. const auto &records = findSpe->second.GetPidRecords(proc.first); - InsertSpeRecords(proc.second->tid, records, data, sampleIps); + InsertSpeRecords(proc.second->tid, records, data, sampleIps, extPool); } } @@ -105,7 +105,7 @@ namespace KUNPENG_PMU { } void PerfSpe::InsertSpeRecords( - const int &tid, const std::vector &speRecords, vector &data, vector &sampleIps) + const int &tid, const std::vector &speRecords, vector &data, vector &sampleIps, std::vector &extPool) { ProcTopology *procTopo = nullptr; auto findProc = procMap.find(tid); @@ -187,10 +187,6 @@ namespace KUNPENG_PMU { findSpe->second.Close(); speSet.erase(this->cpu); - for (auto extPtr : extPool) { - delete[] extPtr; - } - extPool.clear(); return SUCCESS; } diff --git a/pmu/spe_sampler.h b/pmu/spe_sampler.h index fa45ca8..910fdec 100644 --- a/pmu/spe_sampler.h +++ b/pmu/spe_sampler.h @@ -37,7 +37,7 @@ namespace KUNPENG_PMU { {} int Init() override; - int Read(std::vector &data, std::vector &sampleIps) override; + int Read(std::vector &data, std::vector &sampleIps, std::vector &extPool) override; int MapPerfAttr() override; bool Mmap(); @@ -50,10 +50,8 @@ namespace KUNPENG_PMU { private: bool SpeExist(int cpu) const; void InsertSpeRecords(const int &tid, const std::vector &speRecords, std::vector &data, - std::vector &sampleIps); + std::vector &sampleIps, std::vector &extPool); void UpdatePidList(const Spe &spe); - - std::vector extPool; }; } // namespace KUNPENG_PMU #endif -- Gitee From d944a00c221c5d2dbfe6477557308d49805a5124 Mon Sep 17 00:00:00 2001 From: Galaxy Date: Mon, 20 May 2024 04:55:41 -0700 Subject: [PATCH 2/2] Check sample rate in PmuOpen. If perf_event_max_sample_rate cannot be read, do not check frequency and perf_event_open will check later. --- include/pcerrc.h | 1 + pmu/pmu.cpp | 27 +++++++++++++++++++++++++++ pmu/pmu_list.cpp | 1 + test/test_perf/test_api.cpp | 9 +++++++++ util/pcerr.cpp | 1 + 5 files changed, 39 insertions(+) diff --git a/include/pcerrc.h b/include/pcerrc.h index eabeb6e..aaed5c4 100644 --- a/include/pcerrc.h +++ b/include/pcerrc.h @@ -73,6 +73,7 @@ extern "C" { #define LIBPERF_ERR_QUERY_EVENT_TYPE_INVALID 1029 #define LIBPERF_ERR_QUERY_EVENT_LIST_FAILED 1030 #define LIBPERF_ERR_FOLDER_PATH_INACCESSIBLE 1031 +#define LIBPERF_ERR_INVALID_SAMPLE_RATE 1032 #define UNKNOWN_ERROR 9999 diff --git a/pmu/pmu.cpp b/pmu/pmu.cpp index ce65657..5f50b26 100644 --- a/pmu/pmu.cpp +++ b/pmu/pmu.cpp @@ -14,6 +14,7 @@ * and handling performance counters in the KUNPENG_PMU namespace ******************************************************************************/ #include +#include #include #include #include @@ -105,6 +106,28 @@ static int CheckEvtList(unsigned numEvt, char** evtList) return SUCCESS; } +static bool InvalidSampleRate(enum PmuTaskType collectType, struct PmuAttr *attr) +{ + // When sampling, sample frequency must be less than or equal to perf_event_max_sample_rate. + if (collectType != SAMPLING) { + return false; + } + if (!attr->useFreq) { + return false; + } + const string sysSampleRate = "/proc/sys/kernel/perf_event_max_sample_rate"; + ifstream inSys(sysSampleRate); + if (!inSys.is_open()) { + // If perf_event_max_sample_rate cannot be read, do not check frequency + // and perf_event_open will check later. + return false; + } + unsigned long maxRate = 0; + inSys >> maxRate; + + return attr->freq > maxRate; +} + static int CheckAttr(enum PmuTaskType collectType, struct PmuAttr *attr) { auto err = CheckCpuList(attr->numCpu, attr->cpuList); @@ -127,6 +150,10 @@ static int CheckAttr(enum PmuTaskType collectType, struct PmuAttr *attr) New(LIBPERF_ERR_INVALID_EVTLIST); return LIBPERF_ERR_INVALID_EVTLIST; } + if (InvalidSampleRate(collectType, attr)) { + New(LIBPERF_ERR_INVALID_SAMPLE_RATE); + return LIBPERF_ERR_INVALID_SAMPLE_RATE; + } return SUCCESS; } diff --git a/pmu/pmu_list.cpp b/pmu/pmu_list.cpp index aaa047a..003d1b5 100644 --- a/pmu/pmu_list.cpp +++ b/pmu/pmu_list.cpp @@ -462,6 +462,7 @@ namespace KUNPENG_PMU { if (findData == userDataList.end()) { return; } + // Delete ext pointer malloced in SpeSampler. for (auto &extMem : findData->second.extPool) { delete[] extMem; } diff --git a/test/test_perf/test_api.cpp b/test/test_perf/test_api.cpp index 0056c52..b2943f4 100644 --- a/test/test_perf/test_api.cpp +++ b/test/test_perf/test_api.cpp @@ -388,6 +388,15 @@ TEST_F(TestAPI, CollectInvalidTime) ASSERT_EQ(Perrorno(), LIBPERF_ERR_INVALID_TIME); } +TEST_F(TestAPI, InvalidSampleRate) +{ + auto attr = GetPmuAttribute(); + attr.useFreq = 1; + attr.freq = 999999; + pd = PmuOpen(SAMPLING, &attr); + ASSERT_EQ(Perrorno(), LIBPERF_ERR_INVALID_SAMPLE_RATE); +} + TEST_F(TestAPI, TestRaiseNumFd) { // Given (setup) diff --git a/util/pcerr.cpp b/util/pcerr.cpp index 862aa84..b26f188 100644 --- a/util/pcerr.cpp +++ b/util/pcerr.cpp @@ -43,6 +43,7 @@ namespace pcerr { {LIBPERF_ERR_KERNEL_NOT_SUPPORT, "current pmu task is not supported by kernel"}, {LIBPERF_ERR_TOO_MANY_FD, "too many open files"}, {LIBPERF_ERR_RAISE_FD, "failed to setrlimit or getrlimit"}, + {LIBPERF_ERR_INVALID_SAMPLE_RATE, "invalid sample rate, please check /proc/sys/kernel/perf_event_max_sample_rate"}, }; ProfErrorObj ProfErrorObj::profErrorObj; -- Gitee