On Tue, Mar 06, 2007 at 10:44:53AM +0100, Bert Wesarg wrote:
>
>
> Gleb Natapov wrote:
> > On Tue, Mar 06, 2007 at 10:10:44AM +0100, Bert Wesarg wrote:
> >> Fix the double-check locking[1] by defining the cls_initialized member to
> >> volatile.
> >>
> >> Greetings
> >>
> >> Bert Wesarg
> >>
> >> [1]: http://en.wikipedia.org/wiki/Double-checked_locking
> > Can you explain how the Java example from this page applies to the code
> > in Open MPI? cls_initialized is set only after class is fully
> > initialized and usable.
> Sure:
>
> in opal_object.c:opal_class_initialize():
>
>
> if (1 == cls->cls_initialized) {
> return;
> }
> opal_atomic_lock(&class_lock);
>
> /* If another thread initializing this same class came in at
> roughly the same time, it may have gotten the lock and
> initialized. So check again. */
>
> if (1 == cls->cls_initialized) {
> opal_atomic_unlock(&class_lock);
> return;
> }
>
> the compiler can optimize the second if check away, without being declared
> as volatile.
If it does this after opal_atomic_lock() (which is explicit memory
barrier) then it is broken.
>
> Bert
>
>
> >
> >>
> >
> >> ---
> >>
> >> opal/class/opal_object.h | 2 +-
> >> 1 files changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --quilt old/opal/class/opal_object.h new/opal/class/opal_object.h
> >> --- old/opal/class/opal_object.h
> >> +++ new/opal/class/opal_object.h
> >> @@ -148,11 +148,11 @@ typedef void (*opal_destruct_t) (opal_ob
> >> struct opal_class_t {
> >> const char *cls_name; /**< symbolic name for class */
> >> opal_class_t *cls_parent; /**< parent class descriptor */
> >> opal_construct_t cls_construct; /**< class constructor */
> >> opal_destruct_t cls_destruct; /**< class destructor */
> >> - int cls_initialized; /**< is class initialized */
> >> + volatile int cls_initialized; /**< is class initialized */
> >> int cls_depth; /**< depth of class hierarchy tree */
> >> opal_construct_t *cls_construct_array;
> >> /**< array of parent class
> >> constructors */
> >> opal_destruct_t *cls_destruct_array;
> >> /**< array of parent class
> >> destructors */
> >
> >> _______________________________________________
> >> devel mailing list
> >> de...@open-mpi.org
> >> http://www.open-mpi.org/mailman/listinfo.cgi/devel
> >
> > --
> > Gleb.
> > _______________________________________________
> > devel mailing list
> > de...@open-mpi.org
> > http://www.open-mpi.org/mailman/listinfo.cgi/devel
> _______________________________________________
> devel mailing list
> de...@open-mpi.org
> http://www.open-mpi.org/mailman/listinfo.cgi/devel
--
Gleb.
