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

["Chris Friesen" <cfriesen@nortel.com>]

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