Skip to content

Commit acde4bf

Browse files
soerengrunewaldsoerengrunewald
committed
Unify reading sysfs files
[Why] Apparently we have a lot of places in the code where we do basically the same thing: rewind, fflush, fscanf. So it makes sense to provide some common code for that. [How] Add a new helper function to file_utils.h, which bundles the usual steps. Since we wanna be able to handle types correctly we use some templating here, so the compiler will figure the correct format string based on the requested type. And since we have C++17 we can use std::optional to make the error propagation somewhat nicer, allowing for easy fallback values. Lastly move some includes from the header to the source file, where the they are actually needed. And while at it also remove the using namespace directive from the header, since it is considered bad practice [1] in unscoped headers. Signed-off-by: Soeren Grunewald <soeren.grunewald@gmx.net> -- [1] https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rs-using-directive
1 parent 7898825 commit acde4bf

4 files changed

Lines changed: 101 additions & 175 deletions

File tree

‎src/amdgpu.cpp‎

Lines changed: 27 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@
66
#include "amdgpu.h"
77
#include "gpu.h"
88
#include "cpu.h"
9+
#include "file_utils.h"
910
#include "overlay.h"
1011
#include "hud_elements.h"
1112
#include "logging.h"
1213
#include "mesa/util/macros.h"
1314

14-
1515
#define IS_VALID_METRIC(FIELD) (FIELD != 0xffff)
1616
void AMDGPU::get_instant_metrics(struct amdgpu_common_metrics *metrics) {
1717
FILE *f;
@@ -373,24 +373,11 @@ void AMDGPU::metrics_polling_thread() {
373373
}
374374

375375
void AMDGPU::get_sysfs_metrics() {
376-
int64_t value = 0;
377-
if (sysfs_nodes.busy) {
378-
rewind(sysfs_nodes.busy);
379-
fflush(sysfs_nodes.busy);
380-
int value = 0;
381-
if (fscanf(sysfs_nodes.busy, "%d", &value) != 1)
382-
value = 0;
383-
metrics.load = value;
384-
}
376+
if (sysfs_nodes.busy)
377+
metrics.load = read_as<int>(sysfs_nodes.busy).value_or(0);
385378

386-
if (sysfs_nodes.memory_clock) {
387-
rewind(sysfs_nodes.memory_clock);
388-
fflush(sysfs_nodes.memory_clock);
389-
if (fscanf(sysfs_nodes.memory_clock, "%" PRId64, &value) != 1)
390-
value = 0;
391-
392-
metrics.MemClock = value / 1000000;
393-
}
379+
if (sysfs_nodes.memory_clock)
380+
metrics.MemClock = read_as<int64_t>(sysfs_nodes.memory_clock).value_or(0) / 1000000;
394381

395382
// TODO: on some gpus this will use the power1_input instead
396383
// this value is instantaneous and should be averaged over time
@@ -402,14 +389,8 @@ void AMDGPU::get_sysfs_metrics() {
402389
metrics.powerUsage = 0;
403390
} else
404391
#endif
405-
if (sysfs_nodes.power_usage) {
406-
rewind(sysfs_nodes.power_usage);
407-
fflush(sysfs_nodes.power_usage);
408-
if (fscanf(sysfs_nodes.power_usage, "%" PRId64, &value) != 1)
409-
value = 0;
410-
411-
metrics.powerUsage = value / 1000000;
412-
}
392+
if (sysfs_nodes.power_usage)
393+
metrics.powerUsage = read_as<int64_t>(sysfs_nodes.power_usage).value_or(0) / 1000000;
413394

414395
#ifndef TEST_ONLY
415396
if (!get_params()->enabled[OVERLAY_PARAM_ENABLED_gpu_power_limit]) {
@@ -418,92 +399,39 @@ void AMDGPU::get_sysfs_metrics() {
418399
metrics.powerLimit = 0;
419400
} else
420401
#endif
421-
if (sysfs_nodes.power_limit) {
422-
rewind(sysfs_nodes.power_limit);
423-
fflush(sysfs_nodes.power_limit);
424-
if (fscanf(sysfs_nodes.power_limit, "%" PRId64, &value) != 1)
425-
value = 0;
426-
427-
metrics.powerLimit = value / 1000000;
428-
}
402+
if (sysfs_nodes.power_limit)
403+
metrics.powerLimit = read_as<int64_t>(sysfs_nodes.power_limit).value_or(0) / 1000000;
429404

430405
if (sysfs_nodes.fan) {
431-
rewind(sysfs_nodes.fan);
432-
fflush(sysfs_nodes.fan);
433-
if (fscanf(sysfs_nodes.fan, "%" PRId64, &value) != 1)
434-
value = 0;
435-
metrics.fan_speed = value;
406+
metrics.fan_speed = read_as<int64_t>(sysfs_nodes.fan).value_or(0);
436407
metrics.fan_rpm = true;
437408
}
438409

439-
if (sysfs_nodes.vram_total) {
440-
rewind(sysfs_nodes.vram_total);
441-
fflush(sysfs_nodes.vram_total);
442-
if (fscanf(sysfs_nodes.vram_total, "%" PRId64, &value) != 1)
443-
value = 0;
444-
metrics.memoryTotal = float(value) / (1024 * 1024 * 1024);
445-
}
410+
if (sysfs_nodes.vram_total)
411+
metrics.memoryTotal = float(read_as<int64_t>(sysfs_nodes.vram_total).value_or(0)) / (1024 * 1024 * 1024);
412+
413+
if (sysfs_nodes.vram_used)
414+
metrics.sys_vram_used = float(read_as<int64_t>(sysfs_nodes.vram_used).value_or(0)) / (1024 * 1024 * 1024);
446415

447-
if (sysfs_nodes.vram_used) {
448-
rewind(sysfs_nodes.vram_used);
449-
fflush(sysfs_nodes.vram_used);
450-
if (fscanf(sysfs_nodes.vram_used, "%" PRId64, &value) != 1)
451-
value = 0;
452-
metrics.sys_vram_used = float(value) / (1024 * 1024 * 1024);
453-
}
454416
// On some GPUs SMU can sometimes return the wrong temperature.
455417
// As HWMON is way more visible than the SMU metrics, let's always trust it as it is the most likely to work
456-
if (sysfs_nodes.core_clock) {
457-
rewind(sysfs_nodes.core_clock);
458-
fflush(sysfs_nodes.core_clock);
459-
if (fscanf(sysfs_nodes.core_clock, "%" PRId64, &value) != 1)
460-
value = 0;
418+
if (sysfs_nodes.core_clock)
419+
metrics.CoreClock = read_as<int64_t>(sysfs_nodes.core_clock).value_or(0) / 1000000;
461420

462-
metrics.CoreClock = value / 1000000;
463-
}
421+
if (sysfs_nodes.temp)
422+
metrics.temp = read_as<int>(sysfs_nodes.temp).value_or(0) / 1000;
464423

465-
if (sysfs_nodes.temp){
466-
rewind(sysfs_nodes.temp);
467-
fflush(sysfs_nodes.temp);
468-
int value = 0;
469-
if (fscanf(sysfs_nodes.temp, "%d", &value) != 1)
470-
value = 0;
471-
metrics.temp = value / 1000;
472-
}
424+
if (sysfs_nodes.junction_temp)
425+
metrics.junction_temp = read_as<int>(sysfs_nodes.junction_temp).value_or(0) / 1000;
473426

474-
if (sysfs_nodes.junction_temp){
475-
rewind(sysfs_nodes.junction_temp);
476-
fflush(sysfs_nodes.junction_temp);
477-
int value = 0;
478-
if (fscanf(sysfs_nodes.junction_temp, "%d", &value) != 1)
479-
value = 0;
480-
metrics.junction_temp = value / 1000;
481-
}
427+
if (sysfs_nodes.memory_temp)
428+
metrics.memory_temp = read_as<int>(sysfs_nodes.memory_temp).value_or(0) / 1000;
482429

483-
if (sysfs_nodes.memory_temp){
484-
rewind(sysfs_nodes.memory_temp);
485-
fflush(sysfs_nodes.memory_temp);
486-
int value = 0;
487-
if (fscanf(sysfs_nodes.memory_temp, "%d", &value) != 1)
488-
value = 0;
489-
metrics.memory_temp = value / 1000;
490-
}
491-
492-
if (sysfs_nodes.gtt_used) {
493-
rewind(sysfs_nodes.gtt_used);
494-
fflush(sysfs_nodes.gtt_used);
495-
if (fscanf(sysfs_nodes.gtt_used, "%" PRId64, &value) != 1)
496-
value = 0;
497-
metrics.gtt_used = float(value) / (1024 * 1024 * 1024);
498-
}
430+
if (sysfs_nodes.gtt_used)
431+
metrics.gtt_used = float(read_as<int64_t>(sysfs_nodes.gtt_used).value_or(0)) / (1024 * 1024 * 1024);
499432

500-
if (sysfs_nodes.gpu_voltage_soc) {
501-
rewind(sysfs_nodes.gpu_voltage_soc);
502-
fflush(sysfs_nodes.gpu_voltage_soc);
503-
if (fscanf(sysfs_nodes.gpu_voltage_soc, "%" PRId64, &value) != 1)
504-
value = 0;
505-
metrics.voltage = value;
506-
}
433+
if (sysfs_nodes.gpu_voltage_soc)
434+
metrics.voltage = read_as<int64_t>(sysfs_nodes.gpu_voltage_soc).value_or(0);
507435
}
508436

509437
AMDGPU::AMDGPU(std::string pci_dev, uint32_t device_id, uint32_t vendor_id) {

‎src/cpu.cpp‎

Lines changed: 35 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -284,12 +284,8 @@ bool CPUStats::ReadcpuTempFile(int& temp) {
284284
if (!m_cpuTempFile)
285285
return false;
286286

287-
rewind(m_cpuTempFile);
288-
fflush(m_cpuTempFile);
289-
bool ret = (fscanf(m_cpuTempFile, "%d", &temp) == 1);
290-
temp = temp / 1000;
291-
292-
return ret;
287+
temp = read_as<int>(m_cpuTempFile).value_or(0) / 1000;
288+
return temp == 0;
293289
}
294290

295291
bool CPUStats::UpdateCpuTemp() {
@@ -314,44 +310,32 @@ static bool get_cpu_power_k10temp(CPUPowerData* cpuPowerData, float& power) {
314310

315311
if(powerData_k10temp->corePowerFile || powerData_k10temp->socPowerFile)
316312
{
317-
rewind(powerData_k10temp->corePowerFile);
318-
rewind(powerData_k10temp->socPowerFile);
319-
fflush(powerData_k10temp->corePowerFile);
320-
fflush(powerData_k10temp->socPowerFile);
321-
int corePower, socPower;
322-
if (fscanf(powerData_k10temp->corePowerFile, "%d", &corePower) != 1)
323-
goto voltagebased;
324-
if (fscanf(powerData_k10temp->socPowerFile, "%d", &socPower) != 1)
325-
goto voltagebased;
326-
power = (corePower + socPower) / 1000000;
327-
return true;
313+
auto const corePower = read_as<int>(powerData_k10temp->corePowerFile);
314+
auto const socPower = read_as<int>(powerData_k10temp->socPowerFile);
315+
if (corePower and socPower) {
316+
power = (corePower.value() + socPower.value()) / 1000000;
317+
return true;
318+
}
328319
}
329-
voltagebased:
320+
330321
if (!powerData_k10temp->coreVoltageFile || !powerData_k10temp->coreCurrentFile || !powerData_k10temp->socVoltageFile || !powerData_k10temp->socCurrentFile)
331322
return false;
332-
rewind(powerData_k10temp->coreVoltageFile);
333-
rewind(powerData_k10temp->coreCurrentFile);
334-
rewind(powerData_k10temp->socVoltageFile);
335-
rewind(powerData_k10temp->socCurrentFile);
336-
337-
fflush(powerData_k10temp->coreVoltageFile);
338-
fflush(powerData_k10temp->coreCurrentFile);
339-
fflush(powerData_k10temp->socVoltageFile);
340-
fflush(powerData_k10temp->socCurrentFile);
341-
342-
int coreVoltage, coreCurrent;
343-
int socVoltage, socCurrent;
344323

345-
if (fscanf(powerData_k10temp->coreVoltageFile, "%d", &coreVoltage) != 1)
324+
auto const coreVoltage = read_as<int>(powerData_k10temp->coreVoltageFile);
325+
if (not coreVoltage)
346326
return false;
347-
if (fscanf(powerData_k10temp->coreCurrentFile, "%d", &coreCurrent) != 1)
327+
auto const coreCurrent = read_as<int>(powerData_k10temp->coreCurrentFile);
328+
if (not coreCurrent)
348329
return false;
349-
if (fscanf(powerData_k10temp->socVoltageFile, "%d", &socVoltage) != 1)
330+
auto const socVoltage = read_as<int>(powerData_k10temp->socVoltageFile);
331+
if (not socVoltage)
350332
return false;
351-
if (fscanf(powerData_k10temp->socCurrentFile, "%d", &socCurrent) != 1)
333+
auto const socCurrent = read_as<int>(powerData_k10temp->socCurrentFile);
334+
if (not socCurrent)
352335
return false;
353336

354-
power = (coreVoltage * coreCurrent + socVoltage * socCurrent) / 1000000;
337+
power = (coreVoltage.value() * coreCurrent.value() +
338+
socVoltage.value() * socCurrent.value()) / 1000000;
355339

356340
return true;
357341
}
@@ -362,20 +346,14 @@ static bool get_cpu_power_zenpower(CPUPowerData* cpuPowerData, float& power) {
362346
if (!powerData_zenpower->corePowerFile || !powerData_zenpower->socPowerFile)
363347
return false;
364348

365-
rewind(powerData_zenpower->corePowerFile);
366-
rewind(powerData_zenpower->socPowerFile);
367-
368-
fflush(powerData_zenpower->corePowerFile);
369-
fflush(powerData_zenpower->socPowerFile);
370-
371-
int corePower, socPower;
372-
373-
if (fscanf(powerData_zenpower->corePowerFile, "%d", &corePower) != 1)
349+
auto const corePower = read_as<int>(powerData_zenpower->corePowerFile);
350+
if (not corePower)
374351
return false;
375-
if (fscanf(powerData_zenpower->socPowerFile, "%d", &socPower) != 1)
352+
auto const socPower = read_as<int>(powerData_zenpower->socPowerFile);
353+
if (not socPower)
376354
return false;
377355

378-
power = (corePower + socPower) / 1000000;
356+
power = (corePower.value() + socPower.value()) / 1000000;
379357

380358
return true;
381359
}
@@ -385,23 +363,20 @@ static bool get_cpu_power_zenergy(CPUPowerData* cpuPowerData, float& power) {
385363
if (!powerData_zenergy->energyCounterFile)
386364
return false;
387365

388-
rewind(powerData_zenergy->energyCounterFile);
389-
fflush(powerData_zenergy->energyCounterFile);
390-
391-
uint64_t energyCounterValue = 0;
392-
if (fscanf(powerData_zenergy->energyCounterFile, "%" SCNu64, &energyCounterValue) != 1)
366+
auto const energyCounterValue = read_as<uint64_t>(powerData_zenergy->energyCounterFile);
367+
if (not energyCounterValue)
393368
return false;
394369

395370
Clock::time_point now = Clock::now();
396371
Clock::duration timeDiff = now - powerData_zenergy->lastCounterValueTime;
397372
int64_t timeDiffMicro = std::chrono::duration_cast<std::chrono::microseconds>(timeDiff).count();
398-
uint64_t energyCounterDiff = energyCounterValue - powerData_zenergy->lastCounterValue;
373+
uint64_t energyCounterDiff = energyCounterValue.value() - powerData_zenergy->lastCounterValue;
399374

400375

401376
if (powerData_zenergy->lastCounterValue > 0 && energyCounterValue > powerData_zenergy->lastCounterValue)
402377
power = (float) energyCounterDiff / (float) timeDiffMicro;
403378

404-
powerData_zenergy->lastCounterValue = energyCounterValue;
379+
powerData_zenergy->lastCounterValue = energyCounterValue.value();
405380
powerData_zenergy->lastCounterValueTime = now;
406381

407382
return true;
@@ -413,22 +388,19 @@ static bool get_cpu_power_rapl(CPUPowerData* cpuPowerData, float& power) {
413388
if (!powerData_rapl->energyCounterFile)
414389
return false;
415390

416-
rewind(powerData_rapl->energyCounterFile);
417-
fflush(powerData_rapl->energyCounterFile);
418-
419-
uint64_t energyCounterValue = 0;
420-
if (fscanf(powerData_rapl->energyCounterFile, "%" SCNu64, &energyCounterValue) != 1)
391+
auto const energyCounterValue = read_as<uint64_t>(powerData_rapl->energyCounterFile);
392+
if (not energyCounterValue)
421393
return false;
422394

423395
Clock::time_point now = Clock::now();
424396
Clock::duration timeDiff = now - powerData_rapl->lastCounterValueTime;
425397
int64_t timeDiffMicro = std::chrono::duration_cast<std::chrono::microseconds>(timeDiff).count();
426-
uint64_t energyCounterDiff = energyCounterValue - powerData_rapl->lastCounterValue;
398+
uint64_t energyCounterDiff = energyCounterValue.value() - powerData_rapl->lastCounterValue;
427399

428400
if (powerData_rapl->lastCounterValue > 0 && energyCounterValue > powerData_rapl->lastCounterValue)
429401
power = energyCounterDiff / timeDiffMicro;
430402

431-
powerData_rapl->lastCounterValue = energyCounterValue;
403+
powerData_rapl->lastCounterValue = energyCounterValue.value();
432404
powerData_rapl->lastCounterValueTime = now;
433405

434406
return true;
@@ -450,14 +422,11 @@ static bool get_cpu_power_xgene(CPUPowerData* cpuPowerData, float& power) {
450422
if (!powerData_xgene->powerFile)
451423
return false;
452424

453-
rewind(powerData_xgene->powerFile);
454-
fflush(powerData_xgene->powerFile);
455-
456-
uint64_t powerValue = 0;
457-
if (fscanf(powerData_xgene->powerFile, "%" SCNu64, &powerValue) != 1)
425+
auto const powerValue = read_as<uint64_t>(powerData_xgene->powerFile);
426+
if (not powerValue)
458427
return false;
459428

460-
power = (float) powerValue / 1000000.0f;
429+
power = (float) powerValue.value() / 1000000.0f;
461430

462431
return true;
463432
}

‎src/file_utils.cpp‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,25 @@
11
#include "file_utils.h"
22
#include "string_utils.h"
3-
#include <sys/types.h>
4-
#include <sys/stat.h>
5-
#include <unistd.h>
6-
#include <dirent.h>
7-
#include <limits.h>
8-
#include <fstream>
3+
94
#include <cstring>
5+
#include <fstream>
6+
#include <regex>
107
#include <string>
8+
9+
#include <dirent.h>
10+
#include <limits.h>
11+
#include <sys/stat.h>
12+
#include <sys/types.h>
13+
#include <unistd.h>
14+
1115
#include <spdlog/spdlog.h>
1216

1317
#ifndef PROCDIR
1418
#define PROCDIR "/proc"
1519
#endif
1620

21+
namespace fs = ghc::filesystem;
22+
1723
std::string read_line(const std::string& filename)
1824
{
1925
std::string line;

0 commit comments

Comments
 (0)