multithreading question

I wrote a multithreaded program two multiply to matrices for an assignment, but when I run it it doesn't give me the results I expect. I set it up so that the threads would do the rows in order, using a mutex to keep them seperate:

Code:

---

void *matmult(void *ptr) {
  int i, j;
  static int count = -1;
  ++count;
  while (rowcount < m - 1) {
    pthread_mutex_lock(&mutex);
    if (rowcount < m - 1)
      ++rowcount;
    else {
      pthread_mutex_unlock(&mutex);
      return NULL;
    }
    pthread_mutex_unlock(&mutex);
    printf("thread %d running\n",count);
    for (i = 0; i < k; ++i) {
      C[rowcount][i] = 0;
      for (j = 0; j < n; ++j)
        C[rowcount][i] += B[rowcount][j] * A[j][i];
    }
  }
}

---

And the thread creation code looks like:

Code:

---

  pthread_t *tid = new pthread_t[numthreads];
  for (i = 0; i < numthreads; ++i)
    pthread_create(&tid[i],NULL,matmult,NULL);
  for (i = 0; i < numthreads; ++i)
    pthread_join(tid[i],NULL);

---

This seems like it should split the work evenly between all the threads, but when I run it I get output like:

Quote:

---

thread 0 running
thread 0 running
thread 0 running
thread 0 running
thread 0 running
thread 0 running
thread 0 running
thread 0 running
thread 0 running
thread 0 running
thread 0 running
thread 0 running
thread 0 running
thread 0 running
thread 0 running
thread 1 running
thread 2 running
thread 3 running
thread 4 running
thread 5 running
thread 6 running
thread 7 running
thread 8 running
thread 9 running
thread 10 running

---

I don't really understand why this is happening. Is it because each row takes very little time to process, or because I'm not creating them with the right attributes, or what?

The thing is, the first thread is going to just keep going untill its done, or has been interupted. Thier is the system call for some *nix's, called sched_yield in the shced.h header, that you can use so the thread in a way says "hey, go ahead and give some one else some CPU time." You could try putting that in the loop some where.

This kind of goes to show, that even if you can write something multithreaded, doesn't mean its actually going to get spread out among them, unless the schedualer actualy interupts it, and starts one of the other threads, or the thread interupts itself, using the call above, so another program or thread can run (possibly one of your own).

Hopefully some one with more experience in multithreading can chime in and add to this, as I'm sure I missed something.

Is the problem specifically with multi-threading or multiplying matrices do you think?

Really, their isn't a problem at all, just that the work your doing, isn't enough that the os ends up swapping threads. try adding that sched_yeild() after the "printf("thread %d running\n",count);" and see what kind of results you get then.

>> Really, their isn't a problem at all...
Synchronized access to shared variables is incorrect.

Globals being used by matmult(): count, rowcount, m, mutex, k, n, A, B, C.

If all those variables are initialized before the first thread starts, and the values do not change, then each thread can safely read from those vars.

rowcount can change as each thread is run, and access is properly synchronized. However, it should be qualified as volatile as well.

count can change as each thread is run, and access is not properly synchronized. It also should be qualified as volatile.

Each thread should be writting to its own row within C, so no write syncrhonization with C is required.

So, in your example output, what was "numthreads" supposed to be? And when is the assignment due?

gg

The example output was multiplying two 25x25 arrays with 25 threads available. Since then I've changed the code like this:

Code:

---

void *matmult(void *ptr) {
  int i, j, pos = (int)ptr;
  while (rowcount < m - 1) {
    pthread_mutex_lock(&mutex);
    if (rowcount < m - 1)
      ++rowcount;
    else {
      pthread_mutex_unlock(&mutex);
      return NULL;
    }
    pthread_mutex_unlock(&mutex);
    sleep(1); //lets other threads into queue before running and going back
    printf("thread %d is processing row %d\n",pos,rowcount);
    for (i = 0; i < k; ++i) {
      C[rowcount][i] = 0;
      for (j = 0; j < n; ++j)
        C[rowcount][i] += B[rowcount][j] * A[j][i];
    }
  }
}

---

and the thread creation code like:

Code:

---

  pthread_t *tid = new pthread_t[numthreads];
  for (i = 0; i < numthreads; ++i)
    pthread_create(&tid[i],NULL,matmult,(void*)i);
  for (i = 0; i < numthreads; ++i)
    pthread_join(tid[i],NULL);

---

These changes make it so that each thread is actually identified, and they step through the rowslike I had in mind. (0 1 2 3 0 1 2 3 0 1 2 3) etc. Still, I agree that replacing sleep with sched_yield would be a better solution, but that'll have to wait until I get back to school to try out.

There's no need to call sleep() or sched_yield(). Let the scheduler within your OS give out equal time. If you want all your threads to start at the same time, then create a barrier at the beginning of matmult(). Just use a simple condition variable, and once all the threads are created call pthread_cond_broadcast() to release them all at once.
But I wouldn't even bother with that. So what if the first thread knocks out more rows than the others.

gg

Also, your while loop should be changed to just "while (1)".
Your exit condition relies on rowcount - which should only be read while mutex is locked. So reading from rowcount within the while expression isn't really valid or needed.

gg