Messages in this thread | | | Date | Fri, 27 Sep 2013 13:16:16 -0700 | Subject | Re: [PATCH v6 5/6] MCS Lock: Restructure the MCS lock defines and locking code into its own file | From | Jason Low <> |
| |
On Fri, Sep 27, 2013 at 12:38 PM, Tim Chen <tim.c.chen@linux.intel.com> wrote:
> BTW, is the above memory barrier necessary? It seems like the xchg > instruction already provided a memory barrier. > > Now if we made the changes that Jason suggested: > > > /* Init node */ > - node->locked = 0; > node->next = NULL; > > prev = xchg(lock, node); > if (likely(prev == NULL)) { > /* Lock acquired */ > - node->locked = 1; > return; > } > + node->locked = 0; > ACCESS_ONCE(prev->next) = node; > smp_wmb(); > > We are probably still okay as other cpus do not read the value of > node->locked, which is a local variable.
Similarly, I was wondering if we should also move smp_wmb() so that it is before the ACCESS_ONCE(prev->next) = node and after the node->locked = 0. Would we want to guarantee that the node->locked gets set before it is added to the linked list where a previous thread calling mcs_spin_unlock() would potentially modify node->locked?
| |