On Fri, Jul 11, 2008 at 05:18:38PM +0100, Mindaugas Rasiukevicius wrote: > Andrew Doran <andrew%hairylemon.org@localhost> wrote: > > It has to be said, I have not seen a convincing explanation as to why this > > is desirable or in the best interests of NetBSD as a product. > > > > SA threading in NetBSD has serious problems and drawbacks. For example: > > > > - it works only on a handful of architectures, eg x86. > > - in most tests its performance is demonstrably inferior to 1:1. > > - it's completely unreliable, even opening the machine to DOS attacks. > > - it has architectural, code quality and code maintenance issues. > > - it completely lacks any kind of real-time support. > > > > In its current form SA threading is a regressive proposition. Even if all > > the remaining issues are addressed, what benefits would it bring over and > > above 1:1 threading? > > With all respect to Bill's work and his good ideas - he really improved SA > lot, and put a lot of effort on this - I fully agree with Andrew. Besides: > > - SA increases code complexity a lot (in our threading subsystem); I'm not fully sure of this. Yes, kern_sa.c is a lot of code and it's complicated. But I'm not seeing where this complexity leaks "a lot" into the _rest_ of the threading subsystem. As of the present time, all lwps in a process are in standard states. For the moment, I've abandoned the use of LSSUSPEND and instead inactive threads are on one of two per-vp condition variables. This means they are always visible to ps (before they wouldn't be), but it also means we're using more standard primitives. The main hooks into the threading system that I can see are: 1) sleepq_block() may call sa_switch() instead of mi_switch(). There's not much way around this as this is one of the places we need a hook into code. 2) (kinda 1a) we will grab a lock in sa_switch(). If we have to drop l's lock while doing this, we do do the correct "did we just get woken" dance. I don't think this is really a problem. 3) sa_setwoken(). I think this can probably be done away with, but I need to look at it. All this does is help smack the idle lwp on the head. 4) The fact that kern_sa.c knows how sleep queues work. We don't have in-kernel functionality for using lwps as a pool the way SA does, so it's a bit hand-rolled. In the places where it's done, I've tried to comment, "This is a hand-rolled version of routine X() with these arguments." I don't think it'd take much change to extend the sleep queue system to do this, but I felt that was too invasive for this project. If your thoughts differ, I'm more than willing to adapt. 5) Code parts of the kernel "know" they shouldn't generate an upcall. I want to add the adaptive mutex code to this list. This consists of setting LP_SA_NOBLOCK in l_pflag on entry and remembering if it was already set. Then on exit, clearing it if it wasn't already set. I personally don't consider this a hardship. > - does not support thread affinity features; I don't see how revivesa can. To support thread affility means changing the libpthread scheduler to only schedule certain threads on the vp bound to the given CPU. However a new libpthread isn't really a deliverable of revivesa, so by definition revivesa can't fix the problem. This can be done, it's just a separeate step. > - have not seen that performance of SA is better than 1:1 even on very > CPU-bound workload; I think a better question is if you've seen 1:1 threading to be noticably slower with SA compiled in. :-) > Also, I believe SA might be potential security problem (over-complicated code > tends to be breakable - consider a lot of threads with continual blocking). Continual blocking isn't a problem (I think). Massive unblocking might be a problem. I have thoughts in this area. > That is why I suggested to make SA at least a kernel option. My suggestion to > make SA a kernel option was exactly because of that. This also would reduce > the modifications to the existing threading subsystem. I agree. A kernel option is on my to-do list. That way if you don't want it, you don't get it. Likewise if we find a must-fix-before-using problem, we just turn off the option. Take care, Bill
Attachment:
pgpqSexeETVg5.pgp
Description: PGP signature