Re: [libvirt] PATCH: 18/25: Dynamic thread workers pool

On Fri, Jan 16, 2009 at 12:10:35PM +0000, Richard W.M. Jones wrote: > On Tue, Jan 13, 2009 at 05:46:08PM +0000, Daniel P. Berrange wrote: > > @@ -1948,6 +2000,26 @@ static int qemudRunLoop(struct qemud_ser > > } > > } > > > > + /* If number of active workers exceeds both the min_workers > > + * threshold and the number of clients, then kill some > > + * off */ > > + for (i = 0 ; (i < server->nworkers && > > + server->nactiveworkers > server->nclients && > > + server->nactiveworkers > min_workers) ; i++) { > > + > > + if (server->workers[i].active && > > + !server->workers[i].processing) { > > + server->workers[i].quit = 1; > > + > > + virCondBroadcast(&server->job); > > + virMutexUnlock(&server->lock); > > + pthread_join(server->workers[i].thread, NULL); > > + virMutexLock(&server->lock); > > + server->workers[i].active = 0; > > + server->nactiveworkers--; > > + } > > + } > > + > > Doesn't this cause the main loop to hang -- eg. if we happen to try to > kill of a worker which is doing some lengthy operation? If the worker is currently processing an RPC call, its 'processing' flag will be set, and thus we won't consider it a candidate for killing off. > > +struct qemud_worker { > > + pthread_t thread; > > + int active :1; > > + int processing :1; > > + int quit : 1; > > I guess maybe I'm unclear about the meaning of these flags. What's > the difference between active & processing? I'll add comments to this struct before commiting. Their meanings are: active: the thread actually exists processing: the thread is currently processing an RPC call

quit: the thread has been asked to quit At startup we malloc enough 'struct qemud_worker' objects to hold the entire 'max_workers' set, but we only start threads for 'min_workers. This is what the 'active' field tracks. The 'processing' flag is there so that when we want to kill off a thread, we won't block waiting for a thread that's currently busy working. And the 'quit' flag is that condition that a thread checks upon waking up from its condition variable sleep to see if it was asked to quit. Regards, Daniel