Bug 17720 - console-kit-deamon spawns too many threads
console-kit-deamon spawns too many threads
Status: REOPENED
Product: ConsoleKit
Classification: Unclassified
Component: Daemon
unspecified
Other All
: medium normal
Assigned To: william.jon.mccann
https://bugs.launchpad.net/bugs/148454
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-09-22 11:42 UTC by Alex Riesen
Modified: 2011-08-19 08:52 UTC (History)
14 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Patch to console kit to use alternative wait mechanism (2.62 KB, patch)
2009-03-24 13:51 UTC, Jason Hegler
Details | Splinter Review
Kernel patch adds a VT_WAITSWITCH ioctl command (2.01 KB, patch)
2009-03-24 13:53 UTC, Jason Hegler
Details | Splinter Review
Patch to make maximal number of watched VTs configurable (2.62 KB, patch)
2010-06-05 04:29 UTC, Steffen Bauch
Details | Splinter Review
New patch that uses new VT_WAITEVENT ioctl (4.98 KB, patch)
2010-08-19 02:38 UTC, Kan-Ru Chen
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Riesen 2008-09-22 11:42:28 UTC
From Ubuntu: https://bugs.launchpad.net/bugs/148454

since upgrading to gutsy beta htop shows about fifty processes like the following

pid user pri ni virt res shr s cpu% mem% time+ command
7582 root 23 0 7456 2016 1336 s 0.0 0.2 0:00.00 /usr/sbin/console-kit-deamon

It maybe the case that console-kit-daemon starts a thread for each console that can theoretically exist. On Linux (/usr/include/linux/vt.h) MAX_NR_CONSOLE is defined to be 63.

Note that those processes do not show up in DEFAULT top or ps output --BUT-- if you run either of those tools with the -H parameter, so as to display all the threads of a process, then they do show up.
Comment 1 Martin Pitt 2008-10-02 04:50:11 UTC
This might also explain https://launchpad.net/bugs/184519, which has 26 duplicates already, with this stack trace:

  http://launchpadlibrarian.net/12890099/Stacktrace.txt

(crash in vt_thread_start). Or was that actually fixed by dfcab49480565a7bcf71752c5b39eb367df81a19 "cleanly shutdown event logging threads"?
Comment 2 Martin Pitt 2008-10-02 04:56:47 UTC
Ah, to answer myself, this is probably the case: https://launchpad.net/bugs/196724 is another set of 50 duplicates about the same "crash in thread start" problem, which was fixed with the 0.3 patch I mentioned.
Comment 3 Linus Walleij 2008-11-14 14:32:09 UTC
I experience these 63 spawned processes as well, if for nothing else
(all CoW memory etc accounted for) it hogs the scheduler by making 
it slower to sort the CFS RB-tree which is O(log n).

An Ubuntu user proposed the following patch to
autoconfigure the maximum number of consoles from sysfs:

http://launchpadlibrarian.net/15940846/0001-Autoconfigure-maximum-number-of-kernel-consoles.patch

which looks sane to me.

I made a graphical tool to display process migration etc in the system
and the plethora of console-kit-daemons really cloud the view.
Comment 4 Alex Riesen 2008-11-14 15:59:59 UTC
(In reply to comment #3)
> I experience these 63 spawned processes as well, if for nothing else
> (all CoW memory etc accounted for) it hogs the scheduler by making 
> it slower to sort the CFS RB-tree which is O(log n).
> 
> An Ubuntu user proposed the following patch to
> autoconfigure the maximum number of consoles from sysfs:
> 
> http://launchpadlibrarian.net/15940846/0001-Autoconfigure-maximum-number-of-kernel-consoles.patch
> 
> which looks sane to me.
> 
> I made a graphical tool to display process migration etc in the system
> and the plethora of console-kit-daemons really cloud the view.
> 

I am that user. The patch broke the logout process of Ubuntu: the
currently logged in user lost the ability to power off the system,
only logout worked.
Comment 5 william.jon.mccann 2009-02-11 15:27:42 UTC
Sorry for the long delay.  However, it seems like this bug has a few different issues in it.  The issue that is reported in the initial report is actually not a bug but a design decision.  Once we improve or replace the VT subsystem we can do better than this.  Until then we don't have another way.
Comment 6 Linus Walleij 2009-02-11 16:17:12 UTC
Aha it's not a bug it's a feature.
Comment 7 Jason Hegler 2009-03-24 13:51:25 UTC
Created attachment 24210 [details] [review]
Patch to console kit to use alternative wait mechanism

Here is a patch for console kit that removes the numerous threads by utilizing a new kernel ioctl.  (This requires a kernel patch as well).
Comment 8 Jason Hegler 2009-03-24 13:53:40 UTC
Created attachment 24211 [details] [review]
Kernel patch adds a VT_WAITSWITCH ioctl command

This patch adds an ioctl to the kernel which console-kit can use to wait on a terminal switch without creating 60 threads.  It's not incredibly pretty but it seems to work and demonstrates a straightforward solution.
Comment 9 Jason Hegler 2009-04-06 06:52:24 UTC
Does anybody know who the official maintainer of this package is?  I am willing to work on getting the patch included in the kernel if this approach is acceptable to the ConsoleKit team, but I don't want to bother with the kernel side of things unless ConsoleKit will incorporate the changes also.  I tried emailing Jon McCann but did not get any response.
Comment 10 Michael Biebl 2009-04-24 14:53:24 UTC
(In reply to comment #9)
> Does anybody know who the official maintainer of this package is?  I am willing

William Jon McCann is the official maintainer.

> to work on getting the patch included in the kernel if this approach is
> acceptable to the ConsoleKit team, but I don't want to bother with the kernel
> side of things unless ConsoleKit will incorporate the changes also.  I tried
> emailing Jon McCann but did not get any response.
> 

That's very unfortunate. I can only assume he is very busy. Maybe it helps to ping him again.
Comment 11 Matthias Clasen 2009-06-08 07:25:53 UTC
> Does anybody know who the official maintainer of this package is?  I am willing
> to work on getting the patch included in the kernel if this approach is
> acceptable to the ConsoleKit team, but I don't want to bother with the kernel
> side of things unless ConsoleKit will incorporate the changes also.  I tried
> emailing Jon McCann but did not get any response.

This kind of hedging typically doesn't work were well in free software projects.
Don't let the lack of preapproval here keep you from getting the kernel side of things fixed.
Comment 12 Lennart Poettering 2009-07-27 14:46:59 UTC
BTW:

http://lkml.indiana.edu/hypermail/linux/kernel/0906.3/02726.html

Alan wanted cook up his own patch, but uh, he didn't respond in a week or so.
Comment 13 william.jon.mccann 2009-09-23 19:29:25 UTC
Lennart, was there any followup from Alan on this?
Comment 14 Lennart Poettering 2009-09-24 08:19:58 UTC
(In reply to comment #13)
> Lennart, was there any followup from Alan on this?
> 

Alan gave up maintainership of the console stuff a few weeks back. gregkh then took this up and one of the first things he did is merge Alan's patch into his tree. Two days ago this was then merged into mainline. So I guess that means the kernel infrstructure should now be able to cut down the threads to two (the ioctl is still blocking)
Comment 15 william.jon.mccann 2010-01-28 15:46:55 UTC
Lennart, any chance you can make a patch to use this new functionality?
Comment 16 Rez 2010-02-16 02:08:08 UTC
This 3+ years old bug (or feature...) is still giving many people headaches: https://bugs.launchpad.net/ubuntu/+source/consolekit/+bug/148454

Any chance to see those useless processes go away in our lifetime?

If Alan gave up maintainership can someone please work on a fixed version of that ck autoconfigure patch, or something different, an hack, anything?
Comment 17 Lennart Poettering 2010-02-16 06:25:30 UTC
(In reply to comment #16) 
> Any chance to see those useless processes go away in our lifetime?

They aren't processes, they are threads. And they serve a purpose. 

The current behaviour might be suboptimal, but it is correct, as such it would be nice if this is fixed but we certainly have more burning issues to fix.

Also note that whatever the LP bug says about the 63 threads wasting oh so much resources, that is simply nonsense, the folks complaining have no clue what they are speaking of.

> If Alan gave up maintainership can someone please work on a fixed version of
> that ck autoconfigure patch, or something different, an hack, anything?

The stuff is in the kernel. It's just a matter of someone writing a patch for CK. Would be wonderful if you and your Ubuntu friends might contribute good code once in a while instead of just complaining for 3 years straight... ;-)

Anyway, I might come up with a patch eventually, but it is not a priority item on my todo list, sorry.
Comment 18 Rez 2010-02-16 08:01:38 UTC
(In reply to comment #17)

Maybe we're complaining because we're users, not developers. I didn't *choose* to install consolekit in the first place... but I have to, or stay away from X/Gnome/nm and who knows what else.

Anyway, several console-kit/kernel patches *have* been proposed on LP by those "ubuntu friends" you were talking about; unfortunately, limiting consolekit seems to cause other problems. That's why most people are just waiting it to be fixed in upstream.
Comment 19 Lennart Poettering 2010-02-16 10:47:32 UTC
(In reply to comment #18)
> Anyway, several console-kit/kernel patches *have* been proposed on LP by those
> "ubuntu friends" you were talking about; unfortunately, limiting consolekit
> seems to cause other problems. That's why most people are just waiting it to be
> fixed in upstream.

if the patches are good, why aren't they submitted upstream?
Comment 20 rifter 2010-04-06 00:25:40 UTC
(In reply to comment #17)
> (In reply to comment #16) 
> > Any chance to see those useless processes go away in our lifetime?
> 
> They aren't processes, they are threads. And they serve a purpose. 
> 
> The current behaviour might be suboptimal, but it is correct, as such it would
> be nice if this is fixed but we certainly have more burning issues to fix.
> 
> Also note that whatever the LP bug says about the 63 threads wasting oh so much
> resources, that is simply nonsense, the folks complaining have no clue what
> they are speaking of.
> 
> > If Alan gave up maintainership can someone please work on a fixed version of
> > that ck autoconfigure patch, or something different, an hack, anything?
> 
> The stuff is in the kernel. It's just a matter of someone writing a patch for
> CK. Would be wonderful if you and your Ubuntu friends might contribute good
> code once in a while instead of just complaining for 3 years straight... ;-)
> 
> Anyway, I might come up with a patch eventually, but it is not a priority item
> on my todo list, sorry.

I have to disagree with the claim that resources are not being eaten up by console-kit-daemon.  According to a top I did on a system that was recently having memory issues, the 64 console kit processes were using over 7.6GB of memory (122+MB each).  as someone else has pointed out, patches have been proposed for this.  Maybe they were not "good code,"  I haven't read it.  It would be interesting to find out why there need to be so many processes at outset rather that spawning a more conservative number of processes (or threads as the case may be; although each one seemed to have a distinct pid) to me even using 122MB total would be silly; what is this program doing that requires so much RAM?
Comment 21 Steffen Bauch 2010-06-05 04:29:41 UTC
Created attachment 36072 [details] [review]
Patch to make maximal number of watched VTs configurable

I once wrote a small patch for an older ubuntu version that makes it possible to limit the number of watched VTs. The patch is old, for consolekit-0.2.3 and I don't know if it still applies. The main idea was to make the maximal number of threads configurable to the user.

https://bugs.launchpad.net/consolekit/+bug/148454/comments/35

I would like to know if it makes sense to watch Virtual Terminals that have no gettys attached?
Comment 22 Lennart Poettering 2010-06-05 05:41:05 UTC
A few kernel releases back a new kernel API was added that makes the many threads unnecessary. Instead of coming up with ugly work-arounds for the current "problem", please fix things properly and just prepare a patch that ports CK to the new API entirely.
Comment 23 Kan-Ru Chen 2010-08-19 02:38:15 UTC
Created attachment 37970 [details] [review]
New patch that uses new VT_WAITEVENT ioctl

New patch that uses new VT_WAITEVENT ioctl.
Comment 24 Lennart Poettering 2010-09-06 07:40:31 UTC
I commited a slightly modified version of Kan-Ru's patch now to git. Thanks!
Comment 25 Linus Walleij 2010-09-10 03:30:28 UTC
Thanks for fixing this Lennart!
Comment 26 Lennart Poettering 2010-09-10 06:41:33 UTC
(In reply to comment #25)
> Thanks for fixing this Lennart!

Say thanks to Kan-Ru, he did the patch!
Comment 27 Lennart Poettering 2010-11-16 16:54:31 UTC
I have now reverted the patch and reopened this bug. VT_WAITEVENT is inherently racy, since events that happen between the time we return from a VT_WAITEVENT and go back into VT_WAITEVENT are completely lost. That has the effect that when coming back from a suspend sometimes VT switches are not noticed and ACLs incorrectly set up.

Kay has now proposed a new kernel interface that helps us to fix this properly. This is currently in discussion on LKML.
Comment 28 Lennart Poettering 2010-11-16 17:03:05 UTC
The rhbz bug about the race you find here:

https://bugzilla.redhat.com/show_bug.cgi?id=643367
Comment 29 Kan-Ru Chen 2010-11-16 19:24:20 UTC
(In reply to comment #27)
> Kay has now proposed a new kernel interface that helps us to fix this properly.
> This is currently in discussion on LKML.

Where could I find the relevant discussions?  A quick search using "VT_WAITEVENT" or "ConsoleKit" didn't find anything.
Comment 30 Lennart Poettering 2010-11-16 19:33:45 UTC
See the thread starting with this patch of Kay's:

http://markmail.org/message/dhgzoht63uhfieqc
Comment 31 Lennart Poettering 2011-02-18 05:13:19 UTC
BTW, if somebody cares enough: this bug could be fixed properly now as Kay's patch got merged into .38.

There's now a file "/sys/class/tty/tty0/active" which you can read to get the current VT. And you can poll() for it and will get POLLPRI every time the VT changes.

That said, given that CK is being retired anyway sooner or later (merged into systemd) I do wonder if it is really worth working on this for now.
Comment 32 Ionut Biru 2011-03-08 02:20:38 UTC
(In reply to comment #31)
> BTW, if somebody cares enough: this bug could be fixed properly now as Kay's
> patch got merged into .38.
> 
> There's now a file "/sys/class/tty/tty0/active" which you can read to get the
> current VT. And you can poll() for it and will get POLLPRI every time the VT
> changes.
> 
> That said, given that CK is being retired anyway sooner or later (merged into
> systemd) I do wonder if it is really worth working on this for now.


yes it does. there are other distros out there that doesn't use systemd at all and doesn't have the intention to use it.
Comment 33 Lennart Poettering 2011-03-08 06:49:36 UTC
(In reply to comment #32)
> yes it does. there are other distros out there that doesn't use systemd at all
> and doesn't have the intention to use it.

Are there? If so those folks probably have to become active... We will phase out CK one day, and those folks who find value in sticking with obsolete code will then have come forward and do the work.