Am I using threads right?

[ResidentBiscuit]

So I never program with threads, and for some reason they just confuse the hell out of me when trying to use them. Anyway, I have an assignment that passes a bunch of threads to a function and does some matrix multiplication. Am I doing this right?

int i, j;
    pthread_t tid;
    pthread_attr_t attr;

    // create M * N worker threads
    for(i = 0; i < M; i++)
      for(j = 0; j < N; j++) {
	struct v* data = (struct v*) malloc(sizeof(struct v));
	data->i = i;
	data->j = j;
	
	//Create thread
	pthread_attr_init(&attr);
	pthread_create(&tid,&attr,runner,data);
      }
    
    //Wait for threads to finish
    pthread_t workers[NUM_THREADS];
    for(i = 0; i < NUM_THREADS; i++)
      pthread_join(workers[i], NULL);

Basically, am I creating the threads correctly and am I passing them data correctly? And am I also doing the wait correctly?

The runner function looks like void* runner(void* param)

EDIT:
Running this causes a segmentation fault -_- I'm pretty lost here.

[ne555]

`workers' in uninitialized.

man wrote:

Calling pthread_attr_init() on a thread attributes object that has already been initialized results in undefined behaviour.

[ResidentBiscuit]

I ran gdb on this, the issue is in the pthread_join(), which I kind of figured. I took that from an example in the book, and I thought it seemed wrong. Now I'm not sure how to wait for all the threads to finish .

@ne555, then I'm not sure how to create threads inside of a loop like this.

EDIT: I may have gotten it to work using what seems like a sloppy hack.

int i, j, k = 0;
    pthread_attr_t attr;
    pthread_t workers[NUM_THREADS];

    // create M * N worker threads
    for(i = 0; i < M; i++)
      for(j = 0; j < N; j++) {
	struct v* data = (struct v*) malloc(sizeof(struct v));
	data->i = i;
	data->j = j;
	
	//Create thread
	pthread_attr_init(&attr);
	pthread_create(&workers[k],&attr,runner,data);
	k++;
      }

[ne555]

Check out the example in http://www.kernel.org/doc/man-pages/online/pages/man3/pthread_create.3.html
You still are `reinitializing' attr.

[ResidentBiscuit]

Hmm after reading this it just seems like I should make a call to:
pthread_attr_destroy() right after the pthread_create(). Is that right?

[rapidcoder]

What is the purpose of this? To multiply matrices efficiently? If so, you are doing it completely wrong. You should not spawn any threads inside the loops. Multiple threads can only gain you something if you give them enough big tasks to do without interruption or locking. So it isn't wise to use them for multiplying 3x3 matrices (too small task) nor creating N * M threads.

[ne555]

attr_init() sets default values.
If you just want the defaults, there is no need to recreate it, you could simply use it in several create()

[ResidentBiscuit]

@Rapidcoder, you got it. The assignment was create a thread for each calculation. I agree for a 3x3 matrix it's kinda dumb, but it's the assignment.

What's wrong with creating them in a loop? If I understand this correctly, each iteration it's creating a new thread and then moving on to making the next. I don't see where any interruption or locking would happen there as they're all getting sent to work with different parts of the matrix.

But as I said, I am brand new to threads and I'm just following a book right now. If it's teaching me bad practices, please tell me.

[liquidfuzz]

It seems to try and teach you how to start threads, not make it in a, lets say, healthy way. You get the idea of threads, right? -Use computing power, multiple cores, to make computations snappier. Each thread you start hurts the speed. Like rapidcode stated, the work load should be big enough otherwise you only hurt the speed. Starting threads in a loop isn't wrong if you do it right. Many applications starts threads in loops, often with a max number of threads. For instance if you do a binary search you can start thread for each branch until you hit a level of your choosing. From the max level on you go serial.