This is the mail archive of the
libstdc++@gcc.gnu.org
mailing list for the libstdc++ project.
Re: [v3] libstdc++/24469
- From: "Richard Guenther" <richard dot guenther at gmail dot com>
- To: "Benjamin Kosnik" <bkoz at redhat dot com>
- Cc: libstdc++ at gcc dot gnu dot org, gcc-patches at gcc dot gnu dot org
- Date: Thu, 16 Mar 2006 10:57:29 +0100
- Subject: Re: [v3] libstdc++/24469
- References: <20060316014442.e21a6fff.bkoz@redhat.com>
On 3/16/06, Benjamin Kosnik <bkoz@redhat.com> wrote:
>
> Here is a patch for this bug, for which we still do not have a
> testcase. This patch seems pretty simple, given the previous patches:
> that was the idea.
>
> As expected, performance on x86/linux takes a hit with this approach. I
> haven't yet measured powerpc64 or others yet.
>
> I'd like to check this in to libstdcxx_so_7-branch, but will wait a bit
> for comments.
I see the v7 sources have diverged somewhat from the v6 ones, but
the patch doesn't look correct. Either __thread_bin is per thread, in
which case you don't need any special locking against other threads,
or you are not closing the race window, as f.i.
- --__thread_bin->_M_free;
- ++__thread_bin->_M_used;
+ __exchange_and_add(&__thread_bin->_M_free, -1);
+ __atomic_add(&__thread_bin->_M_used, 1);
will have a window of inconsistent _M_free and _M_used, likewise
__thread_bin._M_first = __tmp->_M_next;
- __thread_bin._M_free -= __removed;
+ __exchange_and_add(&__thread_bin._M_free, -__removed);
does not atomically set _M_first and decrement _M_free (I see in v6 sources
_M_first is a volatile ** -- all uses of volatile are very suspicious).
I'll have a look at the v7 branch.
Richard.