Re: [PATCH] threadpool: Switch to detached threads

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

 



On Thu, May 21, 2015 at 01:09:37PM +0200, Jiri Denemark wrote:
> On Thu, May 21, 2015 at 11:19:05 +0100, Daniel P. Berrange wrote:
> > On Thu, May 21, 2015 at 11:57:52AM +0200, Jiri Denemark wrote:
> > > Using joinable threads does not help anything, but it can lead to memory
> > > leaks.
> > > 
> > > When a worker thread exits, it decreases nWorkers or nPrioWorkers and
> > > once both nWorkers and nPrioWorkers are zero (i.e., the last worker is
> > > gone), quit_cond is signaled. When freeing the pool we first tell all
> > > threads to die and then we are waiting for both nWorkers and
> > > nPrioWorkers to become zero. At this point we already know all threads
> > > are gone. So the only reason for calling virThreadJoin of all workers is
> > > to free the memory allocated for joinable threads. If we avoid
> > > allocating this memory, we don't need to take care of freeing it.
> > 
> > The idea behind thread join was to make debugging with valgrind better.
> > By waiting for the threads to shutdown we ensure that all memory they
> > are using has been free before libvirtd exits, so valgrind doesn't
> > report bogus leaks.
> 
> Detached threads don't consume any memory by themselves (while joinable
> threads do), so there's no memory that needs to be freed. And if we are
> concerned about the memory allocated by the code the thread is
> executing, we are safe too, since we already wait for all threads to
> die. A few lines above the virThreadJoin, the following loop handles it:
> 
>     while (pool->nWorkers > 0 || pool->nPrioWorkers > 0)
>         ignore_value(virCondWait(&pool->quit_cond, &pool->mutex));
> 
> > Another reason for using Join is that it allows the threads to finish
> > processing whatever RPC/API call they were currently handling. If we
> > daemonize all threads, then I believe that would allow libvirtd to
> > exit while we were in the middle of performing some change on QEMU.
> > This seems like it could well lead to inconsistent state where we had
> > updated QEMU, but not our state XML, or vica-verca.
> 
> We wait for all thread to finish their processing anyway, so when the
> code gets to virThreadJoin, we already know all threads are gone.

Ok, yes, i see that now looking more closely at the code.

ACK

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]