> I thihnk this would make the code work, but not the locking. You are using
> here:
>
> quit_comp_thread: global, and not completely clear what protects it
> comp_done_lock: global
> comp_done_cond: global
>
> param[i].busy: I would suggest renaming to pending work
> param[i].mutex:
> param[i].cond:
> thread is waiting for work
>
>
> Issues:
>
> param->busy is protected on do_data_compress() and start_compression()
> with param->busy, but in flush_compressed_data() and
> comress_page_with_multithread() it is protected by comp_done_lock.
>
> At this point, I would suggest to just drop param[i].mutex and use
> everywhere comp_done_lock. We can make locking granularly later if
> needed, but 1st get it correct?
> Code basically does (forget termination and locking)
>
> each compression_thread()
>
> while(1) {
> while(!work_to_do)
> wait_for_work
> do_work
> }
>
> And the main thread does:
>
>
> while(1) {
> foreacth compression_thread {
> if thread free {
> put it to work
> break;
> }
> wait_for_thread_to_finish
> }
> }
>
> Notice how we are walking all threads each time that we need to do anything
>
> Perhaps code should be more simple if we put the data that needs to be
> done on a global variable and change this to:
>
> compression_thread
>
> while(1) {
> while(!work_to_do)
> wait_for_work
> pick work from global variable
> wakeup main thread
> do_work
> }
>
> main thread:
>
> put work on global variable
> while(nobody_pick_thework) {
> signal all threads
> wait for a compression thread to take the work }
>
> Why? because then we only have a global mutex and two condition variables,
> with a clear semantics:
> - lock protects two conditions and global variable with work
> - one condition where threads wait for work
> - one condition where main thread wait for a worker to be ready
>
> As we would need to lock every single tame to put the work in the global
> variable, to wait or to pick up the work, we can stop all the:
>
> if (!foo) {
> mutex_lock
> if(!foo) /* this time with lock */
> ....
> }
>
>
> Sorry for the very long mail, if it makes you feel better, this is the second
> time that I wrote it, because the 1st version, my locking proposal didn't
> worked correctly.
>
> What do you think?
I have tried to use comp_done_lock everywhere instead of param[i].mutex and
found
that the performance is poor, the total migration time increase about 5 times.
So I add another variable to make the code correct and remain the
param[i].mutex and the
logic of the compression thread and main thread.
Add another lock will drop the performance and increase total migration time
...
Liang
> Later, Juan.
