On Sun, 18 Dec 2005, Russell King wrote:
>
> On Sat, Dec 17, 2005 at 10:30:41PM -0800, Linus Torvalds wrote:
> > An interrupt can never change the value without changing it back, except
> > for the old-fashioned use of "up()" as a completion (which I don't think
> > we do any more - we used to do it for IO completion a looong time ago).
>
> I doubt you can guarantee that statement, or has the kernel source
> been audited for this recently?
Well, _if_ it's a noticeable performance win, we should just do it. We
already know that people don't call "down()" in interrupts (it just
wouldn't work), we can instrument "up()" too.
It's easy enough to add a "might_sleep()" to the up(). Not strictly true,
but conceptually it would make sense to make up/down match in that sense.
We'd have to mark the (few) places that do down_trylock() + up() in
interrupt context with a special "up_in_interrupt()", but that would be ok
even from a documentation standpoint.
> However, the real consideration is stability - if a semaphore was
> used for a completion and it was merged, would it be found and
> fixed? Probably not, because it won't cause any problems on
> architectures where semaphores have atomic properties.
Actually, the reason we have completions is that using semaphores as
completions caused some really subtle problems that had nothing to do with
atomicity of the operations themselves, so if you find somebody who uses a
semaphore from an interrupt, I think we want to know about it.
Completions actually have another - and more important - property than the
fact that they have a more logical name for a particular usage.
The completion has "don't touch me" guarantees. A thread or interrupt that
does an "up()" on a semaphore may still touch the memory that was
allocated for the semaphore after the "down()" part has been released.
And THAT was the reason for the completions: we allocate them on the stack
or in temporary allocations, and the thing that does the "down()" to wait
for something to finish will also do the _release_ of the memory.
With semaphores, that caused problems, because the side doing the "up()"
would thus possibly touch memory that got released from under it.
This problem happens only on SMP (since you need to have the downer and
the upper running at the same time), but it's totally independent of the
other atomicity issues. And almost any semaphore that is used as a
completion from an interrupt will have this problem, so yes, if you find
somebody doing an "up()" in interrupt context, we'll fix it.
It would be good to make the rules clear, that you can never touch a
semaphore from irq context without changing it back before you return.
Of course, that still leaves the following sequence
if (!down_trylock(..)) {
... do something ..
up(..);
}
which is actually used from interrupts too. At least the console layer
does that (printk() can happen from interrupts, and we do a down_trylock
on the console semaphore. But that one shouldn't mess with the _count_,
although it does mean that the wait-queue preparation etc (for when the
fast case fails) does still need to be protected against interrupts.
But that would be the slow case, so from a performance standpoint, it
would still allow the case that really _matters_ to be done with
interrupts enabled.
> Unless of course sparse can be extended to detect the use of unbalanced
> semaphores in interrupt contexts.
In theory, yes, but in practice I'd much rather just do the stupid brute
force things.
> > (Of course, maybe it's not worth it. It might not be a big performance
> > issue).
>
> Balancing the elimination of 4 instructions per semaphore operation,
> totalling about 4 to 6 cycles, vs stability I'd go for stability
> unless we can prove the above assertion via (eg) sparse.
I agree, if arm interrupt disables are fast. For example, on x86 (where
this isn't needed, because you can have an "interrupt-safe" decrement by
just having it as a single instruction, even if it isn't SMP-safe),
disabling and re-enabling interrupts is just one instruction each, but the
combination is usually something like 50+ cycles. So if this was an issue
on x86, we'd definitely care.
But if you don't think it's a big issue on ARM, it just doesn't matter.
Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]