In kernel 2.4.18 or so, if two Token Ring
boards are used in
a SMP computer, a lock-up happens. IBM asked me to look at
a patch by Denis J. Barlow which was said to fix the lockup.
The patch was too big and touched too much code, so I figured
what it did to locking and wrote a small patchlet which
fixed it. Today I received a mail from Arnaldo (the maintainer of
weird networking stacks) which said
that someone else did exactly the same thing for 2.5
already, so he is taking the patch and sending it
to Marcelo. In a year or so we'll know if it actually
worked...
I am a little sad to see all the rest of Denis' patch
to fall by wayside, but he is stubbornly refusing to
split it into manageable chunks.
Another moral of the story is: Avoid exotic locks,
such as spin_lock_bh(). The "my" patch simply
replaces spin_lock_bh with spin_lock_irqsave. Why did
the author use spin_lock_bh in the first place?
I place the blame squarely on premature optimization.
Somewhere in August, Robert Love
wrote an article in Linux Journal about Linux
locks. It was an honest article, which listed
all lock varieties and even tried to explain how to use
them. When I read it, I thought it was a wonderful article; the only omission I noticed was that he did not mention
that semaphore down() cannot be used in an interrupt.
But guess what, about a month ago, Grant Edwards, a
comp.os.linux.development.system regular, made a posting
where he told he was going to use exotic locking. He
wrote that he went through RML's article, and so, everything
was going to be ok, because he'll use those correctly.
I had to step in to direct him to sane locking.
Of course, his code would be correct - in that particular version
of the kernel. One step left, one step right, and it
bitrots. Just like Token Ring did.
I extend the KISS phylosophy way beyond spin_lock_bh,
into the realm of reader-writer locks. They can improve
performance of contented locks a lot, but they are rife
with pitfalls. In this regard
Bill Irwin grieves me
especially, because he advocates using reader-writer
locks by default while being sharp and respected.
He probably corrupted minds of hundreds of newbie
driver writers. He does not understand that they
are not ready for the sophisticated stuff.
Newbies need something simple instead, which may not
be the most scalable, but working robustly, like a set of
these "Zaitcev's Commandments":
- If you are in a process context (any syscall)
and want to lock other process out, use a semaphore.
You can take a semaphore and sleep (copy_from_user or
kmalloc(x,GFP_KERNEL)).
- Otherwise (== data can be touched in an interrupt),
use spin_lock_irqsave and spin_unlock_irqrestore.
- Avoid holding spinlock for more than 5 lines of code
and across any function call (except accessors like readb).
That would be it. Just keep it simple. Any of these
rules can be broken under some circomstances, but the
problem is that newbies cannot judge adequately.
This is why they are "Commandments", to prevent them
from confusing themselves.