On Mon, Feb 4, 2013 at 4:07 AM, James Peach <jpe...@apache.org> wrote:
> On 03/02/2013, at 9:00 AM, Yunkai Zhang <yunkai...@gmail.com> wrote: > > > On Sun, Feb 3, 2013 at 8:15 PM, Igor Galić <i.ga...@brainsware.org> > wrote: > > > >> On Sunday, February 03, 2013 10:38:37 AM z...@apache.org wrote: > >>> TS-1006: Add a new wrapper: ink_atomic_decrement() > >>> > >>> This is a wrapper of __sync_fetch_and_sub() in GNU env. > >> > >> And what about non-GNU environments? > >> > > > > I have writen a TODO list in the code: > > We should investigate using C++ atomics where possible. It looks like GCC > has changed the memory barrier semantics of the intrinsics over time; > there's probably cases where this is hurting us ... > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51798 > > > > > static inline void > > ink_chunk_delete(InkFreeList *f, InkThreadCache *pCache, InkChunkInfo > > *pChunk) > > { > > void *chunk_addr = pChunk->head; > > > > ... > > > > /* > > * TODO: I had used ink_atomic_increment() here, but it would > > * lead to incorrect value in linux OS, I don't know why: > > Comparing the generated code might be illuminating ... > Ok, I'll give a patch later. > > > * ink_atomic_decrement((int64_t *)&total_mem_in_byte, > > -f->chunk_byte_size); > > * > > * So I create a new wrap, ink_atomic_decrement(), in ink_atomic.h, > > * it works well. But we should create the same wrap for other OS. > > */ > > ink_atomic_decrement(&total_mem_in_byte, f->chunk_byte_size); > > } > > > > I'll give new patch once I have other platforms to test and improve it. > > > > > >> > >>> I need this wrapper in reclaimable freelist, as ink_atomic_increment() > >>> can't work correctly in some situation by unknown reason. > >>> > >>> Signed-off-by: Yunkai Zhang <qiushu....@taobao.com> > >>> Signed-off-by: Zhao Yongming <ming....@gmail.com> > >>> > >>> > >>> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo > >>> Commit: > >> http://git-wip-us.apache.org/repos/asf/trafficserver/commit/4397abf7 > >>> Tree: > http://git-wip-us.apache.org/repos/asf/trafficserver/tree/4397abf7 > >>> Diff: > http://git-wip-us.apache.org/repos/asf/trafficserver/diff/4397abf7 > >>> > >>> Branch: refs/heads/master > >>> Commit: 4397abf76c4fbb7babccc9cc11b0456f2a7a1516 > >>> Parents: 5d8cc8a > >>> Author: Yunkai Zhang <qiushu....@taobao.com> > >>> Authored: Sat Feb 2 17:08:18 2013 +0800 > >>> Committer: Zhao Yongming <ming....@gmail.com> > >>> Committed: Sun Feb 3 10:52:58 2013 +0800 > >>> > >>> ---------------------------------------------------------------------- > >>> lib/ts/ink_atomic.h | 9 ++++++++- > >>> 1 files changed, 8 insertions(+), 1 deletions(-) > >>> ---------------------------------------------------------------------- > >>> > >>> > >>> > >> > http://git-wip-us.apache.org/repos/asf/trafficserver/blob/4397abf7/lib/ts/in > >>> k_atomic.h > >>> ---------------------------------------------------------------------- > >> diff > >>> --git a/lib/ts/ink_atomic.h b/lib/ts/ink_atomic.h > >>> index b22cf24..9425eec 100644 > >>> --- a/lib/ts/ink_atomic.h > >>> +++ b/lib/ts/ink_atomic.h > >>> @@ -160,6 +160,13 @@ ink_atomic_increment(volatile Type * mem, Amount > >> count) > >>> { return __sync_fetch_and_add(mem, (Type)count); > >>> } > >>> > >>> +// ink_atomic_decrement(ptr, count) > >>> +// Decrement @ptr by @count, returning the previous value. > >>> +template <typename Type, typename Amount> static inline Type > >>> +ink_atomic_decrement(volatile Type * mem, Amount count) { > >>> + return __sync_fetch_and_sub(mem, (Type)count); > >>> +} > >>> + > >>> // Special hacks for ARM 32-bit > >>> #if defined(__arm__) && (SIZEOF_VOIDP == 4) > >>> extern ProcessMutex __global_death; > >>> @@ -213,6 +220,6 @@ ink_atomic_increment<int64_t>(pvint64 mem, int > >> value) { > >>> > >>> #else /* not gcc > v4.1.2 */ > >>> #error Need a compiler / libc that supports atomic operations, e.g. gcc > >>> v4.1.2 or later -#endif > >>> +#endif > >>> > >>> #endif /* _ink_atomic_h_ */ > >> > > > > > > > > -- > > Yunkai Zhang > > Work at Taobao > > -- Yunkai Zhang Work at Taobao