[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]