[PATCH] Remove volatile qualifiers from mt_allocator

[Richard Guenther]

On Wed, 30 Aug 2006, Paolo Carlini wrote:

> Paolo Carlini wrote:
> 
> > Richard Guenther wrote:
> >
> > > This patch removes volatile qualifiers from pointer members in
> > > mt_allocator.h.  It doesn't change function signatures and so
> > > should be ABI safe.
> > >
> > > Bootstrapped and tested on x86_64-unknown-linux-gnu some time ago.
> > >
> >> _M_free and _M_used pointers need not be volatile, they are written
> > > in __pool::_M_initialize() only.
> >
> > Can you please study a bit libstdc++/24469, before we go ahead with this
> > kind of clean-up? You can start from line # 295 in mt_allocator.cc
> 
> Now I see that, contrary to my recollections, for some (crazy ;) reason the
> _M_used pointer is typed as volatile, *not* the pointed-to memory. Therefore,
> I think removing those volatiles cannot hurt, in any case.

We can "fix" that particular code by doing this adjustment

        // Return this block to our list and update counters and
        // owner id as needed.
        --__bin._M_used[__block->_M_thread_id];

only if __block->_M_thread_id == __thread_id as _M_used is only used to
optimize reclaiming of thread-local free lists here:

  __pool<true>::_M_reclaim_block(char* __p, size_t __bytes)
...
        unsigned long __remove = __bin._M_free[__thread_id];
        __remove *= __options._M_freelist_headroom;
        if (__remove >= __bin._M_used[__thread_id])
          __remove -= __bin._M_used[__thread_id];
        else 
          __remove = 0;

so it doesn't really matter if we get slightly wrong.  I think the
reporter of #24469 got the logic slightly wrong, as 
__bin._M_free[__thread_id] is used to calculate __remove size, and
remove size is only _decremented_ by __bin._M_used[__thread_id], so
making that larger doesn't cause us to walk the list too far.

So, my fix would be to do

	if (__block->_M_tread_id == __tread_id)
	   --__bin._M_used[__thread_id];

in which case we over-estimate _M_used[__thread_id] and make sure
_M_used[__tread_id] is only accessed from a single thread.

Richard.

--
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs