Re: [PATCH 1/3] drivers: misc: add omap_hwspinlock driver

[Ohad Ben-Cohen]

On Tue, Oct 19, 2010 at 11:08 PM, Arnd Bergmann <arnd@xxxxxxxx> wrote:
> Right. There are two more things to consider though:
>
> If you know that interrupts are disabled, you may still want to avoid
> having to save the interrupt flag to the stack, to save some cycles
> saving and restoring it. I don't know how expensive that is on ARM,
> some other architectures take an microseconds to restore the interrupt
> enabled flag from a register.

To do this we need to introduce a new set of API which will not
disable interrupts, and which should only be used when the caller
knows that interrupts are already disabled.

This will save some cycles, but my concern is that this API will be
abused by drivers, and will eventually be used to take the hwspinlocks
for lengthy period of times (which might even involve sleeping).

Given that the access to the L4 interconnect is anyway slow I also
feel that the gain is negligible.

Nevertheless, it's a viable way to squeeze some cycles.

We don't have a use case in which we know the interrupts are already
disabled, but when we will, we will consider adding such API.

> Even in the case where you know that interrupts are enabled, you may
> want to avoid saving the interrupt flag to the stack, the simpler
> API (one argument instead of two) gives less room for screwing up.

This sounds like adding a set of API that resembles spin_{unlock,lock}_irq.

My gut feeling here is that while this may be useful and simple at
certain places, it is somewhat error prone; a driver which would
erroneously use this at the wrong place will end up enabling
interrupts where it really shouldn't.

Don't you feel that proving a simple spin_lock_irqsave-like API is
actually safer and less error prone ?

I guess that is one of the reasons why spin_lock_irqsave is much more
popular than spin_lock_irq - people just know it will never screw up.

Thanks,
Ohad.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/