Readers-Writers problem in C

Question

I would love some suggestions on this code of mine, pointers on overall design, code quality, optimization in terms of memory and speed.

#include <stdlib.h>
#include <stdio.h>
#include <semaphore.h>
#include <pthread.h>
#include <time.h>

#define NUM_OF_READERS 10
#define NUM_OF_WRITERS 100


static int glob = 0;

static long max_writers = 0;
static long min_writers = 1000;
static long max_readers = 0;
static long min_readers = 1000;
static long avg_writer = 0;
static long avg_reader = 0;

static sem_t rw_mutex;
static sem_t mutex;
static sem_t reader_mux;
static sem_t writer_mux;

static int read_count = 0;


static void *writerFunc(void *arg){
  int loop = *((int*)arg);
  struct timeval tv;
  long before_time;
  long after_time;
  long difference;

  if(gettimeofday(&tv,NULL)==-1){
    printf("Failure getting time");
    exit(1);
  }

  before_time = tv.tv_usec;

  do {
        wait(rw_mutex);
        glob+=10;
        signal(rw_mutex);
  }while(loop--);

  if(gettimeofday(&tv,NULL)==-1){
    printf("Failure getting time");
    exit(1);
  }

  after_time = tv.tv_usec;

  wait(writer_mux);

    difference = (after_time-before_time);
    avg_writer += difference;

    if(difference>max_writers){
      max_writers = difference;
    }

    if(difference<min_writers){
      min_writers = difference;
    }

  signal(writer_mux);

}


static void *readerFunc(void *arg){
  int loop = *((int*)arg);
  struct timeval tv;
  long before_time;
  long after_time;
  long difference;

  if(gettimeofday(&tv,NULL)==-1){
    printf("Failure getting time");
    exit(1);
  }

  before_time = tv.tv_usec;

  do {
        wait(mutex);
        read_count++;

        if (read_count == 1){
          wait(rw_mutex);
        }

        signal(mutex);
        //read is performed
        wait(mutex);
        read_count--;

        if (read_count == 0){
          signal(rw_mutex);
        }
        signal(mutex);

      }while(loop--);

  if(gettimeofday(&tv,NULL)==-1){
    printf("Failure getting time");
    exit(1);
  }

  after_time = tv.tv_usec;

  wait(reader_mux);

    difference = (after_time-before_time);
    avg_reader += difference;

    if(difference>max_readers){
      max_readers = difference;
    }

    if(difference<min_readers){
      min_readers = difference;
    }

  signal(reader_mux);

}

int main(int argc, char *argv[]) {

  pthread_t readers[NUM_OF_READERS];
  pthread_t writers[NUM_OF_WRITERS];

  int s;
  int loops;

  if(sscanf(argv[1],"%i",&loops)==-1){
    printf("Couldn't read loop value");
    exit(1);
  } 


  if (sem_init(&rw_mutex, 0, 1) == -1) {
    printf("Error, init semaphore\n");
    exit(1);
  }

  if(sem_init(&mutex,0,NUM_OF_READERS) == -1){
    printf("Error, init semaphore\n");
    exit(1);
  }

  if(sem_init(&reader_mux,0,1) == -1){
    printf("Error, init semaphore\n");
    exit(1); 
  }

  if(sem_init(&writer_mux,0,1) == -1){
    printf("Error, init semaphore\n");
    exit(1); 
  }

  int i =0;

  for(i=0;i<NUM_OF_READERS;i++){
    s = pthread_create(readers+i, NULL, readerFunc, &loops);
    if (s != 0) {
      printf("Error, creating threads\n");
      exit(1);
    }
  }

  for(i=0;i<NUM_OF_WRITERS;i++){
    s = pthread_create(writers+i, NULL, writerFunc, &loops);
    if (s != 0) {
      printf("Error, creating threads\n");
      exit(1);
    }
  }  

  for(i=0;i<NUM_OF_READERS;i++){
    s = pthread_join(*(readers+i), NULL);
    if (s != 0) {
      printf("Error, creating threads\n");
      exit(1);
    }
  }

  for(i=0;i<NUM_OF_WRITERS;i++){
    s = pthread_join(*(writers+i), NULL);
    if (s != 0) {
      printf("Error, creating threads\n");
      exit(1);
    }
  }

  printf("glob value %d \n", glob);

  printf("The maximum waiting time for writers is: %ld microseconds \n",max_writers);
  printf("The minimum waiting time for writers is: %ld microseconds \n",min_writers);
  printf("The average waiting time for writers is: %ld microseconds \n",avg_writer/NUM_OF_WRITERS);

  printf("The maximum waiting time for readers is: %ld microseconds \n",max_readers);
  printf("The minimum waiting time for readers is: %ld microseconds \n",min_readers);
  printf("The average waiting time for readers is: %ld microseconds \n",avg_reader/NUM_OF_READERS);

  exit(0);
}

Answer 1

The name implies that this is an implementation of the Readers–writers problem, but I suspect that it does not actually do that.

In this problem I would expect, that there is a piece of common memory that the reader functions read and the writers write. I could not find that in your solution.

There is also problem with the locking. In this problem either an unlimited number of readers are allowed in the critical section or (exclusively) maximum 1 writer. I extended your program with a write counter:

static int read_count = 0;
static int write_count = 0;

and printed out the number of readers and writers in the critical section whenever a thread enters it:

wait(writer_mux);
write_count++;
printf("read: %ld, write %ld\n", read_count, write_count);

...

wait(mutex);
read_count++;
printf("read: %ld, write %ld\n", read_count, write_count);

And I have got the following result:

...
read: 4, write 30
read: 4, write 30
read: 1, write 1
read: 1, write 2
read: 1, write 3
read: 1, write 4
read: 1, write 5
...

Other remarks

You have forgotten to check the number of command line parameters:

if(argc < 2 || sscanf(argv[1],"%i",&loops)==-1){
  printf("Couldn't read loop value");
  exit(1);
}

Style: I have noticed that you declare all your local variables at the beginning of the functions. This is not bad, older compilers enforced this but with newer compilers it is not necessary anymore. You may make your code more readable if you declare your variables at the first usage instead.