Re: [patch 2.6.24-rc2 1/3] generic gpio -- gpio_chip support

[Haavard Skinnemoen]

On Wed, 14 Nov 2007 00:37:57 -0800
David Brownell <david-b@xxxxxxxxxxx> wrote:

> Although another point is related to "trivial":  the data
> is being protected through an operation too trivial to be
> worth paying for any of that priority logic.

But isn't there any way we can remove the lock from the fast path
altogether? What is it really protecting?

Since this is the code that runs under the lock (excluding the "extra
checks" case):

+static inline struct gpio_chip *gpio_to_chip(unsigned gpio)
+{
+	return chips[gpio / ARCH_GPIOS_PER_CHIP];
+}

I'd say it protects against chips being removed in the middle of the
operation. However, this comment says that chips cannot be removed
while any gpio on it is requested:

+/* gpio_lock protects the table of chips and to gpio_chip->requested.
+ * While any gpio is requested, its gpio_chip is not removable.  It's
+ * a raw spinlock to ensure safe access from hardirq contexts, and to
+ * shrink bitbang overhead:  per-bit preemption would be very wrong.
+ */

And since we drop the lock before calling the actual get/set bit
operation, we would be screwed anyway if the chip was removed during
the call to __gpio_set_value(). So what does the lock really buy us?

HÃvard
-
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/