| Message ID | 2255222.Ergk1VO0fz@tjmaciei-mobl2 |
|---|---|
| State | New |
| Headers | show |
On Thu, 30 Aug 2012 12:48:34 +0200 Thiago Macieira <thiago.macieira@intel.com> wrote: > Hello > > The attached patch is a simple improvement to make a thread that > failed to set the waiting bit to exit the function earlier, if it > detects that another thread has successfully finished initialising. > It matches the CAS code from a few lines above. > > The change from RELAXED to ACQUIRE is noted in the previous patch > I've just sent. I like this, but want // make a thread that failed to set the waiting bit to exit the function // earlier, if it detects that another thread has successfully finished // initialising added as a comment in the if (expected == pending_bit) branch. I would like to put this in trunk + comment and give it 2-3 days at least before 4.7 branch. -benjamin
On Thu, Sep 06, 2012 at 01:33:11PM -0700, Benjamin De Kosnik wrote: > Here's the patch as applied to trunk in rev. 191042. I'll apply it to > 4.7 this weekend as long as nobody yelps. > 2012-09-06 Thiago Macieira <thiago.macieira@intel.com> > > PR libstdc++/54172 > * libsupc++/guard.cc (__cxa_guard_acquire): Exit the loop earlier if > we detect that another thread has had success. Don't compare_exchange > from a finished state back to a waiting state. Comment. > > diff --git a/libstdc++-v3/libsupc++/guard.cc b/libstdc++-v3/libsupc++/guard.cc > index adc9608..60165cd 100644 > --- a/libstdc++-v3/libsupc++/guard.cc > +++ b/libstdc++-v3/libsupc++/guard.cc > @@ -244,13 +244,13 @@ namespace __cxxabiv1 > if (__gthread_active_p ()) > { > int *gi = (int *) (void *) g; > - int expected(0); > const int guard_bit = _GLIBCXX_GUARD_BIT; > const int pending_bit = _GLIBCXX_GUARD_PENDING_BIT; > const int waiting_bit = _GLIBCXX_GUARD_WAITING_BIT; > > while (1) > { > + int expected(0); > if (__atomic_compare_exchange_n(gi, &expected, pending_bit, false, > __ATOMIC_ACQ_REL, > __ATOMIC_RELAXED)) Shouldn't this __ATOMIC_RELAXED be also __ATOMIC_ACQUIRE? If expected ends up being guard_bit, then the code will return 0; right away. > @@ -264,13 +264,26 @@ namespace __cxxabiv1 > // Already initialized. > return 0; > } > + > if (expected == pending_bit) > { > + // Use acquire here. > int newv = expected | waiting_bit; > if (!__atomic_compare_exchange_n(gi, &expected, newv, false, > __ATOMIC_ACQ_REL, > - __ATOMIC_RELAXED)) > - continue; > + __ATOMIC_ACQUIRE)) > + { > + if (expected == guard_bit) > + { > + // Make a thread that failed to set the > + // waiting bit exit the function earlier, > + // if it detects that another thread has > + // successfully finished initialising. > + return 0; > + } > + if (expected == 0) > + continue; > + } > > expected = newv; > } Jakub
2012-08-30 Thiago Macieira <thiago.macieira@intel.com> * libsupc++/guard.cc (__cxa_guard_acquire): exit the loop earlier if we detect that another thread has had success. --- libstdc++-v3/libsupc++/guard.cc | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/libstdc++-v3/libsupc++/guard.cc b/libstdc++-v3/libsupc++/guard.cc index bfe1a59..73d7221 100644 --- a/libstdc++-v3/libsupc++/guard.cc +++ b/libstdc++-v3/libsupc++/guard.cc @@ -269,8 +269,16 @@ namespace __cxxabiv1 int newv = expected | waiting_bit; if (!__atomic_compare_exchange_n(gi, &expected, newv, false, __ATOMIC_ACQ_REL, - __ATOMIC_RELAXED)) - continue; + __ATOMIC_ACQUIRE)) + { + if (expected == guard_bit) + { + // Already initialized. + return 0; + } + if (expected == 0) + continue; + } expected = newv; } -- 1.7.11.4