patterncMinor
Multithreaded C program to calculate CPU usage of cgroups
Viewed 0 times
programusagecalculatecpucgroupsmultithreaded
Problem
I am writing a program in an environment that makes use of cgroups to identify and group processes together. I want to parse the CPU utilization of each cgroup by sampling
I want to do this in a multithreaded way because if I did it serially, the samples would happen serially, and the program would take a while to run.
I also use PCRE to distinguish between two different process types.
```
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#define ITERATIONS 3
#if !defined(CLK_TIME_PER_SECOND) && defined(_SC_CLK_TCK)
# define CLK_TIME_PER_SECOND ((int) sysconf (_SC_CLK_TCK))
#endif
#if !defined(CLK_TIME_PER_SECOND) && defined(CLK_TCK)
# define CLK_TIME_PER_SECOND ((int) CLK_TCK)
#endif
#if !defined(CLK_TIME_PER_SECOND) && defined(CLOCKS_PER_SEC)
# define CLK_TIME_PER_SECOND ((int) CLOCKS_PER_SEC)
#endif
#if !defined(CLK_TIME_PER_SECOND)
# define CLK_TIME_PER_SECOND 100
#endif
struct job_struct {
char procid[50]; //store job information as procid,procType tuple
char procType[50];
};
typedef struct {
char *procid; //for passing to the threads
char *procType;
} sample_struct;
int sumFlag, clk; //TODO: localize?
long double sum1, sum2, systemUtilization, otherCPU;
char *aStrRegex;
const char *pcreErrorStr;
int pcreErrorOffset, pcreExecRet;
int subStrVec[30];
pcre *reCompiled;
pcre_extra *pcreExtra;
pthread_mutex_t total_lock = PTHREAD_MUTEX_INITIALIZER;
pthread_cond_t ok_to_add = PTHREAD_COND_INITIALIZER;
int sampleCGroup(char procid[], char procType[]);
void *addSampletoTotal();
void *overallUtilization();
long double getSystemUtilization(void);
int getOSVersion(void);
int getCPUCount(void);
int main(void) {
int cpus, i, jobCount, osVersion;
FILE *fp;
char buff[1024];
/sys/fs/cgroup/cpuacct/.../cpuacct.stat over a 1 second interval a few times and then averaging them. It also calculates the overall CPU utilization of the system by sampling /proc/stat in the same manner.I want to do this in a multithreaded way because if I did it serially, the samples would happen serially, and the program would take a while to run.
I also use PCRE to distinguish between two different process types.
```
#include
#include
#include
#include
#include
#include
#include
#include
#include
#include
#define ITERATIONS 3
#if !defined(CLK_TIME_PER_SECOND) && defined(_SC_CLK_TCK)
# define CLK_TIME_PER_SECOND ((int) sysconf (_SC_CLK_TCK))
#endif
#if !defined(CLK_TIME_PER_SECOND) && defined(CLK_TCK)
# define CLK_TIME_PER_SECOND ((int) CLK_TCK)
#endif
#if !defined(CLK_TIME_PER_SECOND) && defined(CLOCKS_PER_SEC)
# define CLK_TIME_PER_SECOND ((int) CLOCKS_PER_SEC)
#endif
#if !defined(CLK_TIME_PER_SECOND)
# define CLK_TIME_PER_SECOND 100
#endif
struct job_struct {
char procid[50]; //store job information as procid,procType tuple
char procType[50];
};
typedef struct {
char *procid; //for passing to the threads
char *procType;
} sample_struct;
int sumFlag, clk; //TODO: localize?
long double sum1, sum2, systemUtilization, otherCPU;
char *aStrRegex;
const char *pcreErrorStr;
int pcreErrorOffset, pcreExecRet;
int subStrVec[30];
pcre *reCompiled;
pcre_extra *pcreExtra;
pthread_mutex_t total_lock = PTHREAD_MUTEX_INITIALIZER;
pthread_cond_t ok_to_add = PTHREAD_COND_INITIALIZER;
int sampleCGroup(char procid[], char procType[]);
void *addSampletoTotal();
void *overallUtilization();
long double getSystemUtilization(void);
int getOSVersion(void);
int getCPUCount(void);
int main(void) {
int cpus, i, jobCount, osVersion;
FILE *fp;
char buff[1024];
Solution
Global Variables
I see this comment just prior to the declarations for the global variables:
My answer would be yes. It is very difficult to read, write, debug and maintain programs that use global variables. Global variables can be modified by any function within the program and therefore require each function to be examined before making changes in the code. In C and C++ global variables impact the namespace and they can cause linking errors if they are defined in multiple files. The answers in this stackoverflow question provide a fuller explanation.
Apply Best Practices Consistently
In the function
Return From
Rather than hard code values in return from main, it would be better to use the system defined symbolic constants EXIT_SUCCESS and EXIT_FAILURE. There are 2 benefits to this, it makes the code more self documenting and it makes the code more portable (may not be necessary in this case). These symbolic constants are always defined in
DRY Code
There is a programming principle called the Don't Repeat Yourself Principle sometimes referred to as DRY code. If you find yourself repeating the same code mutiple times it is better to encapsulate it in a function. If it is possible to loop through the code that can reduce repetition as well.
The following code is repeated many times in
Complexity
There are two functions in the code,
In both
In the case of
As programs grow in size the use of
Example of a simplified main()
```
void report_utilization_and_clean_up_mutex(int cpus)
{
pthread_join(systr, NULL); //join the system utilization thread
if (systemUtilization >
I see this comment just prior to the declarations for the global variables:
//TODO: localize?My answer would be yes. It is very difficult to read, write, debug and maintain programs that use global variables. Global variables can be modified by any function within the program and therefore require each function to be examined before making changes in the code. In C and C++ global variables impact the namespace and they can cause linking errors if they are defined in multiple files. The answers in this stackoverflow question provide a fuller explanation.
Apply Best Practices Consistently
In the function
sampleCGroup() the result from malloc() is tested before it is used, however, in main() the result from malloc() is not tested before it is used.Return From
main()Rather than hard code values in return from main, it would be better to use the system defined symbolic constants EXIT_SUCCESS and EXIT_FAILURE. There are 2 benefits to this, it makes the code more self documenting and it makes the code more portable (may not be necessary in this case). These symbolic constants are always defined in
stdlib.h.DRY Code
There is a programming principle called the Don't Repeat Yourself Principle sometimes referred to as DRY code. If you find yourself repeating the same code mutiple times it is better to encapsulate it in a function. If it is possible to loop through the code that can reduce repetition as well.
The following code is repeated many times in
main(), it would be better to encapsulate it in a function:pthread_join(systr, NULL); //join the system utilization thread
if (systemUtilization > (long double)cpus) { //sanity check for HT
systemUtilization = (long double)cpus;
}
printf("sys=%Lf\n", systemUtilization); //print info
printf("cgrType2=%f\n", 0.0);
printf("cgrType1=%f\n", 0.0);
printf("cgrOther=%Lf\n", systemUtilization);
pthread_mutex_destroy(&total_lock); //clean up mutex lock
pthread_cond_destroy(&ok_to_add);Complexity
There are two functions in the code,
main() and sampleCGroup() that are too complex (do too much). The code in these 2 functions should be broken into smaller functions. Functions that are too large or too complex are very difficult to understand, this makes it much harder to write, read, debug or maintain them. A general rule of thumb is that any function that is larger than a single screen in an editor or IDE is too large, a second indication of complexity is the level of indentation in the function.In both
main() and sampleCGroup() it is important to note that part of what is making these functions larger is code repetition as discussed in DRY CODE.In the case of
sampleGroup() some obvious functions are:char* make_pid_path(char procid[], char procType[])
{
const char* pathHead = "/sys/fs/cgroup/cpuacct/";
const char* pathTail = "/cpuacct.stat";
char* pidPath = (char*) malloc(
strlen(pathHead) + strlen(pathTail) + strlen(procid) + 1);//+1 for the zero-terminator
if (pidPath == NULL) {
perror("Could not allocate memory for str\n");
return NULL;
}
else {
strcpy(pidPath, "/sys/fs/cgroup/cpuacct/"); //create the path to the cgroup info
strcat(pidPath, procid);
strcat(pidPath, "/cpuacct.stat");
}
return pidPath;
}
bool parse_cGroup_CPU_sample(char* pidPath, const char* pass, long double result[2])
{
FILE* fp = fopen(pidPath, "r");
if (fp == NULL) {
perror(pidPath);
return (-1);
}
else {
if (fscanf(fp, "%*4s %Lf %*6s %Lf", &result[0], &result[1]) != 2) {
char error_buffer[256];
sprintf(error_buffer, "Could not parse %s cgroup CPU sample!\n", pass);
perror(error_buffer);
return false;
}
fclose(fp);
}
return true;
}
long double getLoadAverage(char* pidPath)
{
long double a[2], b[2];
long double loadavg = 0.0;
for (int i = 0; i 0) {
sum1 += loadavg;
}
else if (pcreExecRet == PCRE_ERROR_NOMATCH) {
sum2 += loadavg;
}
else {
sum2 += loadavg; //assume type2 if this fails
perror("Could not determine type!\n");
}
sumFlag = 1;
pthread_cond_signal(&ok_to_add); //signal that it is ok to add
pthread_mutex_unlock(&total_lock); //unlock mutex
//CRITICAL SECTION END
return 0; //success
}As programs grow in size the use of
main() should be limited to calling functions that parse the command line, calling functions that set up for processing, calling functions that execute the desired function of the program, and calling functions to clean up after the main portion of the program.Example of a simplified main()
```
void report_utilization_and_clean_up_mutex(int cpus)
{
pthread_join(systr, NULL); //join the system utilization thread
if (systemUtilization >
Code Snippets
//TODO: localize?pthread_join(systr, NULL); //join the system utilization thread
if (systemUtilization > (long double)cpus) { //sanity check for HT
systemUtilization = (long double)cpus;
}
printf("sys=%Lf\n", systemUtilization); //print info
printf("cgrType2=%f\n", 0.0);
printf("cgrType1=%f\n", 0.0);
printf("cgrOther=%Lf\n", systemUtilization);
pthread_mutex_destroy(&total_lock); //clean up mutex lock
pthread_cond_destroy(&ok_to_add);char* make_pid_path(char procid[], char procType[])
{
const char* pathHead = "/sys/fs/cgroup/cpuacct/";
const char* pathTail = "/cpuacct.stat";
char* pidPath = (char*) malloc(
strlen(pathHead) + strlen(pathTail) + strlen(procid) + 1);//+1 for the zero-terminator
if (pidPath == NULL) {
perror("Could not allocate memory for str\n");
return NULL;
}
else {
strcpy(pidPath, "/sys/fs/cgroup/cpuacct/"); //create the path to the cgroup info
strcat(pidPath, procid);
strcat(pidPath, "/cpuacct.stat");
}
return pidPath;
}
bool parse_cGroup_CPU_sample(char* pidPath, const char* pass, long double result[2])
{
FILE* fp = fopen(pidPath, "r");
if (fp == NULL) {
perror(pidPath);
return (-1);
}
else {
if (fscanf(fp, "%*4s %Lf %*6s %Lf", &result[0], &result[1]) != 2) {
char error_buffer[256];
sprintf(error_buffer, "Could not parse %s cgroup CPU sample!\n", pass);
perror(error_buffer);
return false;
}
fclose(fp);
}
return true;
}
long double getLoadAverage(char* pidPath)
{
long double a[2], b[2];
long double loadavg = 0.0;
for (int i = 0; i < ITERATIONS; i++) {
if (!parse_cGroup_CPU_sample(pidPath, "first", a))
{
return -1;
}
else
{
sleep(1);
}
if (!parse_cGroup_CPU_sample(pidPath, "second", b))
{
return -1;
}
loadavg += ((b[0] + b[1]) - (a[0] + a[1])); //delta
}
loadavg = (loadavg / (clk * ITERATIONS)); //calculate the CPU use by delta divided by clock ticks per one second times iterations
return loadavg;
}
int sampleCGroup(char procid[], char procType[]) {
//open the cgroup cpustat file for the job based on the procid arg
char* pidPath = make_pid_path(procid, procType);
if (pidPath == NULL)
{
return -1;
}
long double loadavg = getLoadAverage(pidPath);
free(pidPath); //free the memory allocated to the path variable
if (loadavg < 0)
{
return -1;
}
//CRITICAL SECTION BEGIN
pthread_mutex_lock(&total_lock); //lock mutex
while (sumFlag == 0) {
pthread_cond_wait(&ok_to_add, &total_lock); //wait on ok to add signal
}
pcreExecRet = pcre_exec(reCompiled,
pcreExtra,
procType,
strlen(procType), // length of string
0, // Start looking at this point
0, // OPTIONS
subStrVec,
30); // Length of subStrVec
if (pcreExecRet > 0) {
sum1 += loadavg;
}
else if (pcreExecRet == PCRE_ERROR_NOMATCH) {
sum2 += loadavg;
}
else {
sum2 += loadavg; //assume type2 if this fails
perror("Could not determine type!\n");
}
sumFlag = 1;
pthread_cond_signal(&ok_to_add); //signal that it is ovoid report_utilization_and_clean_up_mutex(int cpus)
{
pthread_join(systr, NULL); //join the system utilization thread
if (systemUtilization > (long double)cpus) { //sanity check for HT
systemUtilization = (long double)cpus;
}
printf("sys=%Lf\n", systemUtilization); //print info
printf("cgrType2=%f\n", 0.0);
printf("cgrType1=%f\n", 0.0);
printf("cgrOther=%Lf\n", systemUtilization);
pthread_mutex_destroy(&total_lock); //clean up mutex lock
pthread_cond_destroy(&ok_to_add);
}
void testOperationNotSupported()
{
int osVersion = getOSVersion();
if (osVersion <= 10) {
systemUtilization = getSystemUtilization();
printf("sys=%Lf\n", systemUtilization); //cgroups are not supported on certain OS versions, so print the overall utilization and exit, otherwise, try to run
pthread_mutex_destroy(&total_lock); //clean up mutex lock
pthread_cond_destroy(&ok_to_add);
exit(EXIT_SUCCESS);
}
}
void performPcreOperations(int cpus)
{
aStrRegex = "proc_type1"; //type1
reCompiled = pcre_compile(aStrRegex, 0, &pcreErrorStr, &pcreErrorOffset, NULL); //compile the regex
if (reCompiled == NULL) {
printf("ERROR: Could not compile '%s': %s\n", aStrRegex, pcreErrorStr);
report_utilization_and_clean_up_mutex(cpus);
exit(EXIT_FAILURE);
}
// Optimize the regex
pcreExtra = pcre_study(reCompiled, 0, &pcreErrorStr);
/* pcre_study() returns NULL for both errors and when it can not optimize the regex. The last argument is how one checks for
errors (it is NULL if everything works, and points to an error string otherwise. */
if (pcreErrorStr != NULL) {
printf("ERROR: Could not study '%s': %s\n", aStrRegex, pcreErrorStr);
report_utilization_and_clean_up_mutex(cpus);
exit(EXIT_FAILURE);
} /* end if */
}
struct job_struct* getJobs(int cpus, int* jobCount)
{
char buff[1024];
struct job_struct* jobs = (job_struct*)calloc(cpus, sizeof(*jobs)); //initially size the array to be the number of CPUs
if (jobs == NULL)
{
fprintf(stderr, "Failed to allocate %d job structures in main()\n", cpus);
exit(EXIT_FAILURE);
}
*jobCount = 0;
FILE* fp = popen("/var/bin/getProcs", "r"); //open getProcs to list the procs, this returns the cgroup ID and the type
if (fp == NULL) {
perror("/var/bin/getProcs");
report_utilization_and_clean_up_mutex(cpus);
return (EXIT_FAILURE);
}
int ljobCount = 0;
while (fgets(buff, sizeof(buff), fp) != NULL) {
sscanf(buff, "%[^,],%[^,]", jobs[ljobCount].procid, jobs[ljobCount].procType);
ljobCount++;
}
*jobCount = ljobCount;
return jobs;
}
int main(void) {
int jobCount = 0;
testOperationNotSupported();
int cpus = getCPUCount();
clk = CLK_TIME_PER_SECOND;
struct job_struct* jobs = getJobs(cpus, &jobCount);
sumFlag = 1; //condition variable for mutex
Context
StackExchange Code Review Q#139383, answer score: 3
Revisions (0)
No revisions yet.