Cc: Linus Torvalds <torvalds@...>, Matthew Wilcox <matthew@...>, Trent Piepho <tpiepho@...>, Russell King <rmk+lkml@...>, Benjamin Herrenschmidt <benh@...>, David Miller <davem@...>, <linux-arch@...>, <scottwood@...>, <linuxppc-dev@...>, <alan@...>, <linux-kernel@...>
On Wednesday 11 June 2008 15:13, Paul Mackerras wrote:
But this is a case where you really should be scared, so FUD is
completely appropriate.
At least, I don't see how it is at all worse than FUD about how much
slower everything will be with stronger ordering, and how nobody will
be able to do anything about it. In reality, for most devices it will
not be measurable, and for some like network or disk might use a bit
of a tweak in one or two fastpaths.
If the driver author honestly does not know the code well enough to
say whether a particular ordering is allowed or not, then it should
just not be allowed.
Anyway, what do I win if I give you a concrete example?
I found one in the first file I picked out of my grep: drivers/net/s2io.c
if (test_and_set_bit(__S2IO_STATE_LINK_TASK, &(nic->state))) {
/* The card is being reset, no point doing anything */
goto out_unlock;
}
...
val64 = readq(&bar0->adapter_control);
val64 |= ADAPTER_LED_ON;
writeq(val64, &bar0->adapter_control);
s2io_link(nic, LINK_UP);
} else {
if (CARDS_WITH_FAULTY_LINK_INDICATORS(nic->device_type,
subid)) {
val64 = readq(&bar0->gpio_control);
val64 &= ~GPIO_CTRL_GPIO_0;
writeq(val64, &bar0->gpio_control);
val64 = readq(&bar0->gpio_control);
}
/* turn off LED */
val64 = readq(&bar0->adapter_control);
val64 = val64 &(~ADAPTER_LED_ON);
writeq(val64, &bar0->adapter_control);
s2io_link(nic, LINK_DOWN);
}
clear_bit(__S2IO_STATE_LINK_TASK, &(nic->state));
Now I can't say definitively that this is going to be wrong on
powerpc, because I don't know the code well enough. But I'd be
90% sure that the unlock is not supposed to be visible to
other CPUs before the writeqs are queued to the card. On x86 it
wouldn't be.
If the relaxed writel and explicit barriers are not well documented and
well understood, then they should not be used.
And if the memory ordering issues are not well understood, then it
should be done with locking and strongly ordered IO accessors.
*You* don't have to watch it, the maintainer does. And if they can't
understand the barriers, then it should not be converted to use them.
Unless you want to take over maintainership. And if you do, then you
should be watching all changes that go into it.
And with the strongly ordered scheme, there is much smaller set of
gotchas again, and it behaves the same as the x86 it was developed on,
and you don't get confused by the list of rules: IO access is either
strongly ordered or weakly ordered.
--
