master: Add volatile to the pointers in the list_item structures.

[bosilca]

In addition to making the code correct, it also makes it faster.
This commit fixes #3450.

Signed-off-by: George Bosilca [email protected]

[jsquyres]

@bosilca This test is failing at Travis because it is generating MB worth of compile logs containing these warnings:

In file included from ../../../../../opal/class/opal_free_list.h:28:0,
                 from ../../../../../ompi/request/request.h:35,
                 from ../../../../../ompi/mca/coll/coll.h:73,
                 from ../../../../../ompi/mca/coll/self/coll_self.h:29,
                 from ../../../../../ompi/mca/coll/self/coll_self_alltoallw.c:26:
../../../../../opal/class/opal_lifo.h: In function ‘opal_lifo_pop_atomic’:
../../../../../opal/class/opal_lifo.h:148:14: warning: assignment discards ‘volatile’ qualifier from pointer target type [enabled by default]
         item = old_head.data.item = lifo->opal_lifo_head.data.item;
              ^

(Travis ultimately fails because the compile exceeds the max log size!)

Are we sure that the commit on this PR is correct?

[jsquyres]

Looks like I exaggerated, sorry -- there's not MB of logs with these kinds of warning messages. But there are at least a few dozen of these "argument discards volatile qualifier..." warning messages.

[bosilca]

@jsquyres my compiler was more tolerant when casting a "volatile T*" into a "T*". I have fixed the warning (and travis seems clean again).

[rhc54]

forgive my ignorance: could you please educate me as to why the need for double volatile declarations? I'm not objecting - just truly asking to be educated.

[bosilca]

My understanding is that there are 3 combinations possible:

• a pointer to a volatile data (2 syntax supported)

    volatile T* blah;
    T volatile * blah;

• A volatile pointer to a non-volatile data (not sure when this one might be necessary but it does exists)

  T * volatile blah;

• volatile pointer to volatile data (with 2 syntax as above)

  volatile T* volatile blah;
  T volatile * volatile blah;

What we had before was just telling the compiler "this is a pointer to a volatile struct". We made no statements about the pointer itself, and as a result a very conservative compiler assumed that the pointer itself doesn't change. What we really need is the last form, because the pointer itself can also be changed (by another thread so the value should never be locally cached). Thus, the second volatile is an indicator that the pointer in addition to the struct should also be considered volatile, and prevents a compiler from any caching related to the pointer or the pointed struct.

[rhc54]

Thanks for the explanation. I guess I'll have to read up more on that 3rd option as I'd never heard of a "volatile struct" before and I'm not sure what that means. I'll also go back to some places where I've used the volatile flag and ensure there aren't more of these to correct.