Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework CPU counting #656

Merged
merged 12 commits into from
Aug 2, 2021
8 changes: 4 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
- name: Bootstrap
run: ./autogen.sh
- name: Configure
run: ./configure --enable-werror --enable-linux-affinity --disable-unicode --disable-sensors
run: ./configure --enable-werror --enable-affinity --disable-unicode --disable-sensors
- name: Enable compatibility modes
run: |
sed -i 's/#define HAVE_FSTATAT 1/#undef HAVE_FSTATAT/g' config.h
Expand All @@ -26,7 +26,7 @@ jobs:
- name: Build
run: make -k
- name: Distcheck
run: make distcheck DISTCHECK_CONFIGURE_FLAGS="--enable-werror --enable-linux-affinity --disable-unicode --disable-sensors"
run: make distcheck DISTCHECK_CONFIGURE_FLAGS="--enable-werror --enable-affinity --disable-unicode --disable-sensors"

build-ubuntu-latest-minimal-clang:
runs-on: ubuntu-latest
Expand All @@ -44,11 +44,11 @@ jobs:
- name: Bootstrap
run: ./autogen.sh
- name: Configure
run: ./configure --enable-werror --enable-linux-affinity --disable-unicode --disable-sensors
run: ./configure --enable-werror --enable-affinity --disable-unicode --disable-sensors
- name: Build
run: make -k
- name: Distcheck
run: make distcheck DISTCHECK_CONFIGURE_FLAGS="--enable-werror --enable-linux-affinity --disable-unicode --disable-sensors"
run: make distcheck DISTCHECK_CONFIGURE_FLAGS="--enable-werror --enable-affinity --disable-unicode --disable-sensors"

build-ubuntu-latest-full-featured-gcc:
runs-on: ubuntu-latest
Expand Down
13 changes: 8 additions & 5 deletions Action.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ in the source distribution for its full text.
#include "Vector.h"
#include "XUtils.h"

#if (defined(HAVE_LIBHWLOC) || defined(HAVE_LINUX_AFFINITY))
#if (defined(HAVE_LIBHWLOC) || defined(HAVE_AFFINITY))
#include "Affinity.h"
#include "AffinityPanel.h"
#endif
Expand Down Expand Up @@ -302,10 +302,10 @@ static Htop_Reaction actionSetAffinity(State* st) {
if (Settings_isReadonly())
return HTOP_OK;

if (st->pl->cpuCount == 1)
if (st->pl->activeCPUs == 1)
return HTOP_OK;

#if (defined(HAVE_LIBHWLOC) || defined(HAVE_LINUX_AFFINITY))
#if (defined(HAVE_LIBHWLOC) || defined(HAVE_AFFINITY))
const Process* p = (const Process*) Panel_getSelected((Panel*)st->mainPanel);
if (!p)
return HTOP_OK;
Expand All @@ -328,8 +328,11 @@ static Htop_Reaction actionSetAffinity(State* st) {
Affinity_delete(affinity2);
}
Object_delete(affinityPanel);
#endif
return HTOP_REFRESH | HTOP_REDRAW_BAR | HTOP_UPDATE_PANELHDR;
#else
return HTOP_OK;
#endif

}

static Htop_Reaction actionKill(State* st) {
Expand Down Expand Up @@ -484,7 +487,7 @@ static const struct {
{ .key = " F9 k: ", .roInactive = true, .info = "kill process/tagged processes" },
{ .key = " F7 ]: ", .roInactive = true, .info = "higher priority (root only)" },
{ .key = " F8 [: ", .roInactive = false, .info = "lower priority (+ nice)" },
#if (defined(HAVE_LIBHWLOC) || defined(HAVE_LINUX_AFFINITY))
#if (defined(HAVE_LIBHWLOC) || defined(HAVE_AFFINITY))
{ .key = " a: ", .roInactive = true, .info = "set CPU affinity" },
#endif
{ .key = " e: ", .roInactive = false, .info = "show process environment" },
Expand Down
8 changes: 4 additions & 4 deletions Affinity.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ in the source distribution for its full text.
#else
#define HTOP_HWLOC_CPUBIND_FLAG HWLOC_CPUBIND_PROCESS
#endif
#elif defined(HAVE_LINUX_AFFINITY)
#elif defined(HAVE_AFFINITY)
#include <sched.h>
#endif

Expand Down Expand Up @@ -59,7 +59,7 @@ Affinity* Affinity_get(const Process* proc, ProcessList* pl) {
if (ok) {
affinity = Affinity_new(pl);
if (hwloc_bitmap_last(cpuset) == -1) {
for (unsigned int i = 0; i < pl->cpuCount; i++) {
for (unsigned int i = 0; i < pl->existingCPUs; i++) {
Affinity_add(affinity, i);
}
} else {
Expand All @@ -84,7 +84,7 @@ bool Affinity_set(Process* proc, Arg arg) {
return ok;
}

#elif defined(HAVE_LINUX_AFFINITY)
#elif defined(HAVE_AFFINITY)

Affinity* Affinity_get(const Process* proc, ProcessList* pl) {
cpu_set_t cpuset;
Expand All @@ -93,7 +93,7 @@ Affinity* Affinity_get(const Process* proc, ProcessList* pl) {
return NULL;

Affinity* affinity = Affinity_new(pl);
for (unsigned int i = 0; i < pl->cpuCount; i++) {
for (unsigned int i = 0; i < pl->existingCPUs; i++) {
if (CPU_ISSET(i, &cpuset)) {
Affinity_add(affinity, i);
}
Expand Down
10 changes: 5 additions & 5 deletions Affinity.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,16 @@ in the source distribution for its full text.

#include "ProcessList.h"

#if defined(HAVE_LIBHWLOC) || defined(HAVE_LINUX_AFFINITY)
#if defined(HAVE_LIBHWLOC) || defined(HAVE_AFFINITY)
#include <stdbool.h>

#include "Object.h"
#include "Process.h"
#endif


#if defined(HAVE_LIBHWLOC) && defined(HAVE_LINUX_AFFINITY)
#error hwloc and linux affinity are mutual exclusive.
#if defined(HAVE_LIBHWLOC) && defined(HAVE_AFFINITY)
#error hwloc and affinity support are mutual exclusive.
#endif


Expand All @@ -38,12 +38,12 @@ void Affinity_delete(Affinity* this);

void Affinity_add(Affinity* this, unsigned int id);

#if defined(HAVE_LIBHWLOC) || defined(HAVE_LINUX_AFFINITY)
#if defined(HAVE_LIBHWLOC) || defined(HAVE_AFFINITY)

Affinity* Affinity_get(const Process* proc, ProcessList* pl);

bool Affinity_set(Process* proc, Arg arg);

#endif /* HAVE_LIBHWLOC || HAVE_LINUX_AFFINITY */
#endif /* HAVE_LIBHWLOC || HAVE_AFFINITY */

#endif
7 changes: 5 additions & 2 deletions AffinityPanel.c
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,10 @@ Panel* AffinityPanel_new(ProcessList* pl, const Affinity* affinity, int* width)
Panel_setHeader(super, "Use CPUs:");

unsigned int curCpu = 0;
for (unsigned int i = 0; i < pl->cpuCount; i++) {
for (unsigned int i = 0; i < pl->existingCPUs; i++) {
if (!ProcessList_isCPUonline(this->pl, i))
continue;

char number[16];
xSnprintf(number, 9, "CPU %d", Settings_cpuId(pl->settings, i));
unsigned cpu_width = 4 + strlen(number);
Expand Down Expand Up @@ -427,7 +430,7 @@ Affinity* AffinityPanel_getAffinity(Panel* super, ProcessList* pl) {
Affinity_add(affinity, i);
hwloc_bitmap_foreach_end();
#else
for (unsigned int i = 0; i < this->pl->cpuCount; i++) {
for (int i = 0; i < Vector_size(this->cpuids); i++) {
const MaskItem* item = (const MaskItem*)Vector_get(this->cpuids, i);
if (item->value) {
Affinity_add(affinity, item->cpu);
Expand Down
4 changes: 2 additions & 2 deletions AvailableMetersPanel.c
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,9 @@ const PanelClass AvailableMetersPanel_class = {

// Handle (&CPUMeter_class) entries in the AvailableMetersPanel
static void AvailableMetersPanel_addCPUMeters(Panel* super, const MeterClass* type, const ProcessList* pl) {
if (pl->cpuCount > 1) {
if (pl->existingCPUs > 1) {
Panel_add(super, (Object*) ListItem_new("CPU average", 0));
for (unsigned int i = 1; i <= pl->cpuCount; i++) {
for (unsigned int i = 1; i <= pl->existingCPUs; i++) {
char buffer[50];
xSnprintf(buffer, sizeof(buffer), "%s %d", type->uiName, Settings_cpuId(pl->settings, i - 1));
Panel_add(super, (Object*) ListItem_new(buffer, i));
Expand Down
21 changes: 12 additions & 9 deletions CPUMeter.c
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ static void CPUMeter_init(Meter* this) {
unsigned int cpu = this->param;
if (cpu == 0) {
Meter_setCaption(this, "Avg");
} else if (this->pl->cpuCount > 1) {
} else if (this->pl->activeCPUs > 1) {
char caption[10];
xSnprintf(caption, sizeof(caption), "%3u", Settings_cpuId(this->pl->settings, cpu - 1));
Meter_setCaption(this, caption);
Expand All @@ -59,21 +59,24 @@ static void CPUMeter_getUiName(const Meter* this, char* buffer, size_t length) {
}

static void CPUMeter_updateValues(Meter* this) {
memset(this->values, 0, sizeof(double) * CPU_METER_ITEMCOUNT);

unsigned int cpu = this->param;
if (cpu > this->pl->cpuCount) {
if (cpu > this->pl->existingCPUs) {
xSnprintf(this->txtBuffer, sizeof(this->txtBuffer), "absent");
for (uint8_t i = 0; i < this->curItems; i++)
this->values[i] = 0;
return;
}
memset(this->values, 0, sizeof(double) * CPU_METER_ITEMCOUNT);

double percent = Platform_setCPUValues(this, cpu);
if (isnan(percent)) {
xSnprintf(this->txtBuffer, sizeof(this->txtBuffer), "offline");
return;
}

char cpuUsageBuffer[8] = { 0 };
char cpuFrequencyBuffer[16] = { 0 };
char cpuTemperatureBuffer[16] = { 0 };

double percent = Platform_setCPUValues(this, cpu);

if (this->pl->settings->showCPUUsage) {
xSnprintf(cpuUsageBuffer, sizeof(cpuUsageBuffer), "%.1f%%", percent);
}
Expand Down Expand Up @@ -112,7 +115,7 @@ static void CPUMeter_display(const Object* cast, RichString* out) {
int len;
const Meter* this = (const Meter*)cast;

if (this->param > this->pl->cpuCount) {
if (this->param > this->pl->existingCPUs) {
RichString_appendAscii(out, CRT_colors[METER_TEXT], "absent");
return;
}
Expand Down Expand Up @@ -206,7 +209,7 @@ static void AllCPUsMeter_updateValues(Meter* this) {
}

static void CPUMeterCommonInit(Meter* this, int ncol) {
unsigned int cpus = this->pl->cpuCount;
unsigned int cpus = this->pl->existingCPUs;
CPUMeterData* data = this->meterData;
if (!data) {
data = this->meterData = xMalloc(sizeof(CPUMeterData));
Expand Down
2 changes: 1 addition & 1 deletion CommandLine.c
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ int CommandLine_run(const char* name, int argc, char** argv) {
Hashtable* dm = DynamicMeters_new();
ProcessList* pl = ProcessList_new(ut, dm, flags.pidMatchList, flags.userId);

Settings* settings = Settings_new(pl->cpuCount);
Settings* settings = Settings_new(pl->activeCPUs);
pl->settings = settings;

Header* header = Header_new(pl, settings, 2);
Expand Down
12 changes: 6 additions & 6 deletions LoadAverageMeter.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,12 @@ static void LoadAverageMeter_updateValues(Meter* this) {
if (this->values[0] < 1.0) {
this->curAttributes = OK_attributes;
this->total = 1.0;
} else if (this->values[0] < this->pl->cpuCount) {
} else if (this->values[0] < this->pl->activeCPUs) {
this->curAttributes = Medium_attributes;
this->total = this->pl->cpuCount;
this->total = this->pl->activeCPUs;
} else {
this->curAttributes = High_attributes;
this->total = 2 * this->pl->cpuCount;
this->total = 2 * this->pl->activeCPUs;
}

xSnprintf(this->txtBuffer, sizeof(this->txtBuffer), "%.2f/%.2f/%.2f", this->values[0], this->values[1], this->values[2]);
Expand All @@ -79,12 +79,12 @@ static void LoadMeter_updateValues(Meter* this) {
if (this->values[0] < 1.0) {
this->curAttributes = OK_attributes;
this->total = 1.0;
} else if (this->values[0] < this->pl->cpuCount) {
} else if (this->values[0] < this->pl->activeCPUs) {
this->curAttributes = Medium_attributes;
this->total = this->pl->cpuCount;
this->total = this->pl->activeCPUs;
} else {
this->curAttributes = High_attributes;
this->total = 2 * this->pl->cpuCount;
this->total = 2 * this->pl->activeCPUs;
}

xSnprintf(this->txtBuffer, sizeof(this->txtBuffer), "%.2f", this->values[0]);
Expand Down
2 changes: 1 addition & 1 deletion Process.c
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,7 @@ void Process_writeField(const Process* this, RichString* str, ProcessField field
case PERCENT_NORM_CPU: {
float cpuPercentage = this->percent_cpu;
if (field == PERCENT_NORM_CPU) {
cpuPercentage /= this->processList->cpuCount;
cpuPercentage /= this->processList->activeCPUs;
}
if (cpuPercentage > 999.9F) {
xSnprintf(buffer, n, "%4u ", (unsigned int)cpuPercentage);
Expand Down
3 changes: 2 additions & 1 deletion ProcessList.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ ProcessList* ProcessList_init(ProcessList* this, const ObjectClass* klass, Users
this->userId = userId;

// set later by platform-specific code
this->cpuCount = 0;
this->activeCPUs = 0;
this->existingCPUs = 0;
this->monotonicMs = 0;

// always maintain valid realtime timestamps
Expand Down
5 changes: 4 additions & 1 deletion ProcessList.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,15 @@ typedef struct ProcessList_ {
memory_t usedSwap;
memory_t cachedSwap;

unsigned int cpuCount;
unsigned int activeCPUs;
unsigned int existingCPUs;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No big deal but I find the naming slightly unclear - AIUI 'active' means 'online' and 'existing' means the CPU exists but is either online or offline? An alternative might be to use 'onlineCPUs' and something like 'actualCPUs' or 'realCPUs' here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer actualCPUs over realCPUs to not clash with virtualization stuff …

} ProcessList;

/* Implemented by platforms */
ProcessList* ProcessList_new(UsersTable* usersTable, Hashtable* dynamicMeters, Hashtable* pidMatchList, uid_t userId);
void ProcessList_delete(ProcessList* pl);
void ProcessList_goThroughEntries(ProcessList* super, bool pauseProcessUpdate);
bool ProcessList_isCPUonline(const ProcessList* super, unsigned int id);


ProcessList* ProcessList_init(ProcessList* this, const ObjectClass* klass, UsersTable* usersTable, Hashtable* dynamicMeters, Hashtable* pidMatchList, uid_t userId);
Expand Down
8 changes: 4 additions & 4 deletions README
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,11 @@ To install on the local system run `make install`. By default `make install` ins
enable Performance Co-Pilot support via a new pcp-htop utility
dependency: *libpcp*
default: *no*
* `--enable-affinity`:
enable `sched_setaffinity(2)` and `sched_getaffinity(2)` for affinity support; conflicts with hwloc
default: *check*
* `--enable-hwloc`:
enable hwloc support for CPU affinity; disables Linux affinity
enable hwloc support for CPU affinity; disables affinity support
dependency: *libhwloc*
default: *no*
* `--enable-static`:
Expand Down Expand Up @@ -113,9 +116,6 @@ To install on the local system run `make install`. By default `make install` ins
* `--enable-ancient-vserver`:
enable ancient VServer support (implies `--enable-vserver`)
default: *no*
* `--enable-linux-affinity`:
enable Linux `sched_setaffinity(2)` and `sched_getaffinity(2)` for affinity support; conflicts with hwloc
default: *check*
* `--enable-delayacct`:
enable Linux delay accounting support
dependencies: *pkg-config*(build-time), *libnl-3* and *libnl-genl-3*
Expand Down
2 changes: 1 addition & 1 deletion TasksMeter.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ static void TasksMeter_updateValues(Meter* this) {
this->values[0] = pl->kernelThreads;
this->values[1] = pl->userlandThreads;
this->values[2] = pl->totalTasks - pl->kernelThreads - pl->userlandThreads;
this->values[3] = MINIMUM(pl->runningTasks, pl->cpuCount);
this->values[3] = MINIMUM(pl->runningTasks, pl->activeCPUs);
this->total = pl->totalTasks;

xSnprintf(this->txtBuffer, sizeof(this->txtBuffer), "%d/%d", (int) this->values[3], (int) this->total);
Expand Down
2 changes: 1 addition & 1 deletion UptimeMeter.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ static const int UptimeMeter_attributes[] = {

static void UptimeMeter_updateValues(Meter* this) {
int totalseconds = Platform_getUptime();
if (totalseconds == -1) {
if (totalseconds <= 0) {
xSnprintf(this->txtBuffer, sizeof(this->txtBuffer), "(unknown)");
return;
}
Expand Down
Loading