Re: [PATCH 1/12]: MUTEX: Implement mutexes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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]
  Powered by Linux