From fda80732b5953d2b554864d34149966297ebe84d Mon Sep 17 00:00:00 2001 From: Arjen Hiemstra Date: Thu, 29 Sep 2022 14:45:27 +0200 Subject: [PATCH] cpu: Account for offline processors when adding CPU objects Processors can be marked as offline. In that case, they will not be reported by /proc/cpuinfo . However, the processor ID in /proc/stat will account for the missing processor. In that case, we end up trying to perform an out of bounds read on the vector of CPU objects. To account for missing processors, store the CPU objects in a hash rather than a vector so that we can store them based on the actual processor ID. In addition, rework the naming so we report a more proper core name if cores are offline. BUG: 459130 --- plugins/cpu/linuxcpu.cpp | 8 ++--- plugins/cpu/linuxcpu.h | 7 +++-- plugins/cpu/linuxcpuplugin.cpp | 56 ++++++++++++++++++++++++++-------- plugins/cpu/linuxcpuplugin.h | 4 +-- 4 files changed, 54 insertions(+), 21 deletions(-) diff --git a/plugins/cpu/linuxcpu.cpp b/plugins/cpu/linuxcpu.cpp index bb85d9c..b68d2c8 100644 --- a/plugins/cpu/linuxcpu.cpp +++ b/plugins/cpu/linuxcpu.cpp @@ -23,9 +23,9 @@ static double readCpuFreq(const QString &cpuId, const QString &attribute, bool & return 0; } -LinuxCpuObject::LinuxCpuObject(const QString &id, const QString &name, KSysGuard::SensorContainer *parent) +LinuxCpuObject::LinuxCpuObject(const QString &id, const QString &name, double initialFrequency, KSysGuard::SensorContainer *parent) : CpuObject(id, name, parent) - + , m_initialFrequency(initialFrequency) { } @@ -34,10 +34,10 @@ void LinuxCpuObject::makeTemperatureSensor(const sensors_chip_name * const chipN m_temperature = KSysGuard::makeSensorsFeatureSensor(QStringLiteral("temperature"), chipName, feature, this); } -void LinuxCpuObject::initialize(double initialFrequency) +void LinuxCpuObject::initialize() { CpuObject::initialize(); - m_frequency->setValue(initialFrequency); + m_frequency->setValue(m_initialFrequency); bool ok; const double max = readCpuFreq(id(), QStringLiteral("cpuinfo_max_freq"), ok); if (ok) { diff --git a/plugins/cpu/linuxcpu.h b/plugins/cpu/linuxcpu.h index b1144d6..b3b0176 100644 --- a/plugins/cpu/linuxcpu.h +++ b/plugins/cpu/linuxcpu.h @@ -18,14 +18,15 @@ class SensorsFeatureSensor; class LinuxCpuObject : public CpuObject { public: - LinuxCpuObject(const QString &id, const QString &name, KSysGuard::SensorContainer *parent); + LinuxCpuObject(const QString &id, const QString &name, double initialFrequency, KSysGuard::SensorContainer *parent); void update(unsigned long long system, unsigned long long user, unsigned long long wait, unsigned long long idle); - void initialize(double initialFrequency); + void initialize() override; void makeTemperatureSensor(const sensors_chip_name * constchipName, const sensors_feature * const feature); private: - void initialize() override {}; void makeSensors() override; + + double m_initialFrequency; UsageComputer m_usageComputer; }; diff --git a/plugins/cpu/linuxcpuplugin.cpp b/plugins/cpu/linuxcpuplugin.cpp index c220906..58f25e8 100644 --- a/plugins/cpu/linuxcpuplugin.cpp +++ b/plugins/cpu/linuxcpuplugin.cpp @@ -22,9 +22,41 @@ struct CpuInfo int id = -1; int cpu = -1; int core = -1; + int siblings = -1; qreal frequency = 0.0; }; +// Determine sensor names for all the found processors. Because processors can +// be offline, we need to account for processor IDs skipping and report the +// proper names. +static QHash makeCpuNames(const QVector &cpus, int cpuCount) +{ + QHash result; + + if (cpuCount == 1) { + // Simple case: Only one CPU, just report CPU number + 1 as core number. + for (const auto &info : cpus) { + result.insert(info.id, i18nc("@title", "Core %1", info.id + 1)); + } + return result; + } + + int currentCpu = 0; + int previousCpuSiblings = 0; + + for (const auto &info : cpus) { + if (info.cpu != currentCpu) { + previousCpuSiblings = previousCpuSiblings + info.siblings; + currentCpu = info.cpu; + } + + int coreNumber = info.id - previousCpuSiblings; + result.insert(info.id, i18nc("@title", "CPU %1 Core %2", currentCpu + 1, coreNumber)); + } + + return result; +} + LinuxCpuPluginPrivate::LinuxCpuPluginPrivate(CpuPlugin *q) : CpuPluginPrivate(q) { @@ -56,6 +88,8 @@ LinuxCpuPluginPrivate::LinuxCpuPluginPrivate(CpuPlugin *q) info.core = value.toInt(); } else if (field == "cpu MHz") { info.frequency = value.toDouble(); + } else if (field == "siblings") { + info.siblings = value.toInt(); } } @@ -67,20 +101,18 @@ LinuxCpuPluginPrivate::LinuxCpuPluginPrivate(CpuPlugin *q) // one compared to the actual number of CPUs. Correct that here. cpuCount += 1; + auto names = makeCpuNames(cpus, cpuCount); + QHash numCores; for (const auto &entry : qAsConst(cpus)) { - const QString name = cpuCount > 1 - ? i18nc("@title", "CPU %1 Core %2", entry.cpu + 1, ++numCores[entry.cpu]) - : i18nc("@title", "Core %1", ++numCores[entry.cpu]); - - auto cpu = new LinuxCpuObject(QStringLiteral("cpu%1").arg(entry.id), name, m_container); - m_cpus.push_back(cpu); + auto cpu = new LinuxCpuObject(QStringLiteral("cpu%1").arg(entry.id), names.value(entry.id), entry.frequency, m_container); + m_cpus.insert(entry.id, cpu); m_cpusBySystemIds.insert({entry.cpu, entry.core}, cpu); } addSensors(); - for (int i = 0; i < m_cpus.size(); ++i) { - m_cpus.at(i)->initialize(cpus.at(i).frequency); + for (const auto cpu : std::as_const(m_cpus)) { + cpu->initialize(); } m_allCpus = new LinuxAllCpusObject(m_container); m_allCpus->initialize(); @@ -93,7 +125,7 @@ void LinuxCpuPluginPrivate::update() m_loadAverages->update(); auto isSubscribed = [] (const KSysGuard::SensorObject *o) {return o->isSubscribed();}; - if (std::none_of(m_cpusBySystemIds.cbegin(), m_cpusBySystemIds.cend(), isSubscribed) && !m_allCpus->isSubscribed()) { + if (std::none_of(m_cpus.cbegin(), m_cpus.cend(), isSubscribed) && !m_allCpus->isSubscribed()) { return; } @@ -120,7 +152,7 @@ void LinuxCpuPluginPrivate::update() if (line.startsWith("cpu ")) { m_allCpus->update(system + irq + softirq, user + nice , iowait + steal, idle); } else if (line.startsWith("cpu")) { - auto cpu = m_cpus[std::atoi(line.mid(strlen("cpu")))]; + auto cpu = m_cpus.value(std::atoi(line.mid(strlen("cpu")))); cpu->update(system + irq + softirq, user + nice , iowait + steal, idle); } } @@ -165,10 +197,10 @@ void LinuxCpuPluginPrivate::addSensorsIntel(const sensors_chip_name * const chip return; } for (auto feature = coreFeatures.cbegin(); feature != coreFeatures.cend(); ++feature) { - if (m_cpusBySystemIds.contains({physicalId, feature.key()})) { + if (m_cpusBySystemIds.contains({physicalId, int(feature.key())})) { // When the cpu has hyperthreading we display multiple cores for each physical core. // Naturally they share the same temperature sensor and have the same coreId. - auto cpu_range = m_cpusBySystemIds.equal_range({physicalId, feature.key()}); + auto cpu_range = m_cpusBySystemIds.equal_range({physicalId, int(feature.key())}); for (auto cpu_it = cpu_range.first; cpu_it != cpu_range.second; ++cpu_it) { (*cpu_it)->makeTemperatureSensor(chipName, feature.value()); } diff --git a/plugins/cpu/linuxcpuplugin.h b/plugins/cpu/linuxcpuplugin.h index 11ad074..c8d61ad 100644 --- a/plugins/cpu/linuxcpuplugin.h +++ b/plugins/cpu/linuxcpuplugin.h @@ -25,8 +25,8 @@ private: void addSensorsAmd(const sensors_chip_name * const chipName); LinuxAllCpusObject *m_allCpus; - QVector m_cpus; - QMultiHash, LinuxCpuObject * const> m_cpusBySystemIds; + QHash m_cpus; + QMultiHash, LinuxCpuObject * const> m_cpusBySystemIds; LoadAverages *m_loadAverages; }; -- GitLab