Reputation: 183
When I was discussing the behavior of spinlocks in uni- and SMP kernels with some colleagues, we dived into the code and found a line that really surprised us, and we can’t figure out why it’s done this way.
short calltrace to show where we’re coming from:
spin_lock calls raw_spin_lock,
raw_spin_lock calls _raw_spin_lock, and
on a uni-processor system, _raw_spin_lock is #defined as __LOCK
__LOCK is a define:
#define __LOCK(lock) \
do { preempt_disable(); ___LOCK(lock); } while (0)
So far, so good. We disable preemption by increasing the kernel task’s lock counter. I assume this is done to improve performance: since you should not hold a spinlock for more than a very short time, you should just finish your critical section instead of being interrupted and potentially have another task spin its scheduling slice away while waiting for you to finish.
However, now we finally come to my question. The corresponding unlock code looks like this:
#define __UNLOCK(lock) \
do { preempt_enable(); ___UNLOCK(lock); } while (0)
Why would you call preempt_enable() before ___UNLOCK? This seems very unintuitive to us, because you might get preempted immediately after calling preempt_enable, without ever having the chance to release your spinlock. It feels like this renders the whole preempt_disable/preempt_enable logic somewhat ineffective, especially since preempt_disable specifically checks during its call whether the lock counter is 0 again, and then calls the scheduler. It seems to us like it would make so much more sense to first release the lock, then decrease the lock counter and thus potentially enable scheduling again.
What are we missing? What is the idea behind calling preempt_enable before ___UNLOCK instead of the other way round?
Upvotes: 4
Views: 423
Reputation: 5271
You're looking at the uni-processor defines. As the comment in spinlock_api_up.h
says (http://lxr.free-electrons.com/source/include/linux/spinlock_api_up.h#L21):
/*
* In the UP-nondebug case there's no real locking going on, so the
* only thing we have to do is to keep the preempt counts and irq
* flags straight, to suppress compiler warnings of unused lock
* variables, and to add the proper checker annotations:
*/
The ___LOCK
and ___UNLOCK
macros are there for annotation purposes, and unless __CHECKER__
is defined (It is defined by sparse
), it ends up to be compiled out.
In other words, preempt_enable()
and preempt_disable()
are the ones doing the locking in a single processor case.
Upvotes: 5