# 1 Producer, N Consumers working in parallel, each doing ~1/nth of a task

I am not wondering about error checking (I will add that soon). I would instead like to know about correctness, efficiency and simplicity.

#include <pthread.h>
#include <stdio.h>
#include <unistd.h>

#define  RUN_N_TIMES 37

volatile int producerCond           = 0;

void do_work(unsigned int* current_level){
/* Some work done */
++*current_level;
}

unsigned int level = 0;
printf("Thread %u has completed %u units of work.\nIt was #%d to complete it's task this round..\n", \
producerCond = 1;
}
}
return NULL;
} else {
do_work(&level);
}
}
return NULL;
}

void reset_cond(int val){
int i;
consumerCond[i] = val;
}

int main(int argc, char **argv)
{
int                   i;

for(i=0; i < NTHREADS; ++i) {
}

/* Do nothing */
}

printf("Waking up all waiting threads " "#RUN_N_TIMES" " times...\n");
for(i = 0; i < RUN_N_TIMES; i++){
reset_cond(1);
/* Wake them up */
producerCond = 0;
while (!producerCond) {
printf("Main waiting\n");
/* Go to sleep */
}
}
reset_cond(1);
printf("Go home everybody!\n");

}

printf("Main done\n");
return 0;
}


Firstly, the code runs nicely. I'm running OSX and it compiles cleanly and executes correctly (as far as I can tell). I was a little surprised that threads wake up in the same order each time - in other words if thread 123456 is the first to complete the first job, then it seems to be the first to complete each subsequent job. This says more about the thread implementation than your code I guess.

The problem I have with the code is that although it seems to work, it is coded very specifically for the problem in hand. The barriers ensuring that all threads come together at intervals are not separately identifiable but are entwined in the control code. The variable mini_tasks_complete, used in creating the barrier is initialized to -NTHREADS, allowed to run up to zero during the initial barrier and then cycles from 0 to NTHREADS. This clearly works but is inelegant (I think you will find that the initial barrier can be omitted and worked into the loop with a bit of thought).

I would prefer to see the thread function have the simple form:

while (more_tasks) {
barrier();
work();
}


and the main thread have the general form:

for (...) {
prepare_work();
barrier();
}


Written like this one can clearly identify the synchronisation. The barrier function here would also preferably take a parameter identifying the barrier and not use globals.

I assume your platform, like mine, lacks the pthread barrier. You might write your own barrier function. From what I have read, barriers are normally based upon a pair of condition variables and a pair of mutexes. The Little Book of Semaphores has a discussion of this and there is a useful Solaris implementation that could be used as a template.

Some trivia:

• I would find while (more_tasks) more logical than while (!no_more_tasks)

• inconsistent use of camelCase