Re: mutexes, locks and so on...
On Thu, 11 Nov 2010 17:22:03 +0100 Johnny Billquist b...@softjar.se wrote: I've been looking through the netbsd mutex, and lock implementations the last few days, with the intent to try and improve the performance on the VAX, which has suffered somewhat in performance with NetBSD (maybe other platforms as well, but they might not feel the pain that much :-) ). While doing this, I have stumbled upon a few questions, as well as some general observations. A few observations first. The mutex implementation in place now, is nice in many ways, as it is rather open to different implementations based on what the hardware can do. However, I found that only one platform (hppa) is currently using this. All others rely on the __HAVE_SIMPLE_MUTEXES implementation, which utilize a CAS function. Obviously the VAX does not have a CAS, and it is rather costly to simulate it, so I'm working on getting away from this. (Does really all other platforms have a CAS?) What would be nice here is if it had been possible to inline the mutex_lock and mutex_unlock (as well as their spin equivalents) functions, but I found out (the hard way) that this is not possible. I also found out that mutex spin locks are more or less equivalent to the old scheme of using splraise() and splx(). However, I noticed one big difference. In the old days, splraise() and splx() were always handled as a pair. You did not do: a = splraise(..) b = splraise(..) splx(a) splx(b) which would have been very broken. With mutex_spin, you instead store the original spl at the first mutex_spin_enter, and later calls to mutex_spin_enter can only possibly raise the ipl further. At mutex_spin_exit, we do not lower the spl again, until the final mutex_spin_exit, which resets the spl to the value as before any mutex was held. The cause a slightly different behaviour, as the spl will continue to possibly be very high, even though you are only holding a low ipl mutex. While it obviously don't cause a system to fail, it can introduce delays which might not be neccesary, and could in theory cause interrupts to be dropped that needn't be. Is this a conscious design? Do we not expect/enforce mutexes to be released in the reverse order they were acquired? Isnt that because of priority inversion? Moving on to locks: the simple lock defined in lock.h is easy enough, and I haven't found any problems with it. The rwlock, however, is written with the explicit assumption that there is a CAS function. It would be great if that code were a little more like the mutex code, so that alternative implementations could be done for architectures where you'd like to do it in other ways. Is the reason for this not being the case just an oversight, a lack of time and resources, or is there some underlying reason why this could not be done? Also, there are a few places in the code in the kernel, where atomic_cas is called explicitly. Wouldn't it be better if such code were abstracted out, so we didn't depend on specific cpu instructions in the MI code? Well, just a few thoughts, and a question for all of you... I'll happily listed to your wisdom as I try to get my alternative locking on the VAX running... Johnny -- NetBSD - Simplicity is prerequisite for reliability
Re: mutexes, locks and so on...
On 11/11/10 18:07, Mindaugas Rasiukevicius wrote: Hello, Similar questions were raised a few times.. let's go through this again. Sorry if I am rehashing old stuff then... Johnny Billquistb...@softjar.se wrote: With mutex_spin, you instead store the original spl at the first mutex_spin_enter, and later calls to mutex_spin_enter can only possibly raise the ipl further. At mutex_spin_exit, we do not lower the spl again, until the final mutex_spin_exit, which resets the spl to the value as before any mutex was held. This is correct. The cause a slightly different behaviour, as the spl will continue to possibly be very high, even though you are only holding a low ipl mutex. While it obviously don't cause a system to fail, it can introduce delays which might not be neccesary, and could in theory cause interrupts to be dropped that needn't be. In theory, it might. But see below. Is this a conscious design? Do we not expect/enforce mutexes to be released in the reverse order they were acquired? Very conscious, made about 4 years ago. There are very few (would say a tiny fraction, in all tree) where we have the following locking cases: mutex_spin_exit(a); - e.g. where a has IPL_HIGH some code - some code still runs at IPL_HIGH mutex_spin_exit(b); - e.g. where b has IPL_VM One such case is in tty subsystem, which needs proper locking (and perhaps generally revamped.. anyone?). Generally, such cases should be fixed. Hum? So that was introduced with the new locking code then? Because back when we used splraise/splx, the above would not have worked. Also, AFAIK effect was was actually measured during newlock2 branch development. Supporting such micro-optimisation would mean quite a bit of complexity in critical mutex(9) interface with practically no gain. My initial reaction was that each mutex should have just stored what ipl it came from, and just restored it back to that when the mutex was released. That would also allow us to skip the mutex count in cpu_info, as well as the global saved ipl in cpu_info, but then I realized that this solution would break if people actually wrote code like lock(a) lock(b) release(a) release(b) or if code actually relied on the ipl staying at a high level, even though you don't have any mutex that motivates this anymore. Otherwise, I would think that each mutex keeping the old ipl level for restoring at release is both prettier and shorter, and is safer. And since a mutex can't be held by multiple users at the same time, there is no risk that the old ipl would become clobbered either. (Although right now I'm trying to figure out why I seem to lock a spin mutex twice when the system is coming up, and it might be in the MI bits of code...) Also, there are a few places in the code in the kernel, where atomic_cas is called explicitly. Wouldn't it be better if such code were abstracted out, so we didn't depend on specific cpu instructions in the MI code? atomic_cas_*() routines are mandatory part of atomic_ops(3/9) interface. I believe almost any modern operating system should have it these days. We model algorithms using them. I believe MI code, design-wise, should look to the future, rather than the past. :) Hmm. Hard to argue about this. While I think it's nice if we try to keep the kernel agnostic, the user api is not something I'm arguing about changing. But it would be nice if we could leave it possible to do things in different ways when it's not really the effect of a CAS that is needed, but we'd still like to keep it in a way that allowed the compiled code to be as nice and efficient as possible. Johnny
