On Mon, 3 Jan 2005, Pete Zaitcev wrote:
> This is not a good idea, at least not typically. This was extensively
> discussed before, and although I cannot repeat all arguments precisely,
> I remember that I found them convincing. It boils down to two things.
> Fist is SMP: volatile does nothing to help serialize accesses, although
> I can see that in perhaps in this case it's not an issue. The second
> is portability: on a few CPUs special instructions are necessary for
> serializations, which are provided by memory barriers. I would suggest
> adding barriers explicitly.
Curiously, I just finished a long discussion off-list with Dave Brownell
on this very point.
I haven't seen those earlier discussions. If you could provide a pointer,
I would appreciate it.
Regarding your two points: As you noted, SMP is not an issue here. The
code in question is fully protected by a spinlock already. Serialization
also is not an issue. You are correct that "volatile" does not provide
serialization, but access-ordering isn't important for the problem
addressed by this patch. There already are barriers in the driver to
provide correct serialization (although perhaps a few more may be needed;
I haven't gone through the code carefully with this in mind). There are
needed in any case, independent of the patch.
What _is_ at stake is preventing the compiler from performing certain
incorrect optimizations. For example, without a "volatile" declaration,
code looking like this:
status = td->status;
if (status & TD_ACTIVE) ...
is compiled by some versions of gcc into the equivalent of:
status = td->status;
if (td->status & TD_ACTIVE) ...
which is wrong because td->status can change between the two
statements.
Defeating such optimizations probably can be done using appropriate
compiler barriers. I chose not to do it that way, because:
It's nearly impossible to tell where the compiler will
perform optimizations that should not be allowed. I know
that the bad sequence above occurs in several places; there
may be others I don't know about.
Going in and inserting barriers by hand is a painstaking and
error-prone process, particularly when the code changes in the
future. By adding a single "volatile", I can make the compiler
do all that work for me, with no errors.
In David's words, it's a tradeoff. As the maintainer, I decided to go
in favor of simplicity and ease-of-maintenance.
Alan Stern
-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
