Re: [PATCH] x86: SGU UV Add volatile to macros that access chipsetregisters

From: Chris Friesen
Date: Wed Sep 09 2009 - 14:55:29 EST


On 09/09/2009 12:11 PM, Daniel Walker wrote:
> On Wed, 2009-09-09 at 13:01 -0500, Jack Steiner wrote:
>> Volatile is being added to the accessor functions that are used to
>> read/write memory-mapped I/O registers located within the UV chipset.
>> The use of volatile is hidden within the functions and is not exposed
>> to the users of the functions.
>>
>> Note that the use is limited to the accessor functions in the header
>> file. No .c files are changed or need to know about volatile.
>>
>>
>> This seems to be consistent with other uses of volatile within the kernel.
>
> The document that I cited specifically addresses memory accessors as not
> needing the volatile keyword .. So your still not addressing exactly why
> your code needs it .. Are your accessors special in some way? Is there
> some defect your seeing without the volatile keyword?


>From that document:

"There are still a few rare situations where volatile makes sense in the
kernel:

- The above-mentioned accessor functions might use volatile on
architectures where direct I/O memory access does work.
Essentially, each accessor call becomes a little critical section
on its own and ensures that the access happens as expected by the
programmer."


However, the fact that these functions are returning volatile pointers
is a bit sketchy. Normally accesses to such memory would be wrapped
entirely in a read/write function which would handle the volatility
internally to the function.

In this case there's no point in the function returning a "volatile
unsigned long *" if it's going to be cast to a volatile pointer before
being dereferenced.

Chris
--
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/