On Tue, Jul 13, 2010 at 8:57 AM, Avi Kivity <avi@redhat.com> wrote:
> On 07/13/2010 05:19 PM, Peter Palfrader wrote:
>>
>>> So it looks like last_value was placed in a read only section.  Please
>>> post your System.map somewhere.
>>>
>>
>> weasel@intrepid:~$ publish System.map
>>
>> http://asteria.noreply.org/~weasel/volatile/2010-07-13-mbm2xEdd8Q4/System.map
>> weasel@intrepid:~$ grep -i last_value System.map
>> ffffffff81712e80 r last_value
>> ffffffff81b05240 b last_value.26163
>>
>
> "r" = "read only"
>
> How does it look in 'nm arch/x86/kernel/pvclock.o'?

I bet it is the same. And I have a suspicion: because the only write
access to that variable is in an asm that uses the "memory" clobber to
say it wrote to it (rather than say it writes to it directly), and
because the variable is marked 'static', gcc decides that nothing ever
writes to it in that compilation unit, and it can be made read-only.

Look at our definition for "xchg()" in
arch/x86/include/asm/cmpxchg_64.h. It boils down to

                asm volatile("xchgq %0,%1"                              \
                             : "=r" (__x)                               \
                             : "m" (*__xg(ptr)), "0" (__x)              \
                             : "memory");                               \

for the 8-byte case (which is obviously what atomic64_xchg() uses).
And the _reason_ we do that thing where we use a memory _input_ and
then a clobber is that older versions of gcc did not accept the thing
we _want_ to use, namely using "+m" to say that we actually change the
memory.  So the above is "wrong", but has historical reasons - and
it's apparently never been changed.

However, the "+m" was fixed, and we use it elsewhere, so I think the
"m" plus memory clobber is now purely historical. Does a patch
something like the appended fix it? I also suspect we should look at
some other uses in this area. The atomic64_64.h file uses "=m" and
"m", which looks like another legacy thing (again, "+m" historically
wasn't allowed, and then later became the 'correct' way to do things).

NOTE NOTE NOTE! This is UNTESTED and INCOMPLETE. We should do cmpxchg
too, and the 32-bit versions. I'm adding Ingo and Peter to the cc.

                     Linus