Greg Ames wrote:
> Bill Stoddard wrote:
>
> (moved from dev@httpd to dev@apr)
>
>
>>I hesitate to commit the because I am not sure if apr_atomic_dec will be portable and
>>usable on enough OS/hardware combinations.
>
>
> as we discussed offline, you're right. The return value for apr_atomic_dec
> isn't defined, but as your code demonstrates, it's very useful for making the
> cache more scaleable.
>
>
>>If any apr_atomic_* implementations have hardware dependencies (ie, the
>implementation
>>explicitly exploits hardware features via assembly language calls for instance),
>>supporting the atomic operations in APR could become a real nightmare. APR compiled
>on
>>one machine may not run on another machine even if the OS level of the two machines
>is
>>identical. Most gnarley...
>
>
> yeah, that's bad for binaries. Seems like we would want to build them on the
> most ancient version of the CPU architecture supported, or have #defines which
> "dumb down" your build machine.
>
>
>>- if (sconf->lock) {
>>- apr_thread_mutex_lock(sconf->lock);
>>- }
>>- obj->refcount--;
>>- /* If the object is marked for cleanup and the refcount
>>- * has dropped to zero, cleanup the object
>>- */
>>- if ((obj->cleanup) && (!obj->refcount)) {
>>- cleanup_cache_object(obj);
>>- }
>>- if (sconf->lock) {
>>- apr_thread_mutex_unlock(sconf->lock);
>>+ if (!apr_atomic_dec(&obj->refcount)) {
>>+ if (obj->cleanup) {
>>+ cleanup_cache_object(obj);
>>+ }
>
>
> nice...a concise lock free example of the "last guy out the door turns out the
> lights" pattern.
>
> However, this code won't work on Linux at the moment. apr_atomic_dec invokes
> the atomic_dec function which doesn't define a return value. Linux does have an
> atomic_dec_and_test which we could use. But it returns true when the counter
> becomes zero; Win32 returns the new value of the counter, so it is false when
> the counter becomes zero. It looks to me like Solaris behaves like Win32; dunno
> about Netware.
>
> FreeBSD doesn't seem to have userland atomic functions that return anything. It
> shouldn't be too hard to write an i386 asm function, but FreeBSD runs on other
> chip families where I wouldn't feel comfortable with asm. Perhaps we could
> emulate a atomic_dec that tells you when the value reaches zero for non-i386
> FreeBSD using a lock, like Ian's existing default functions.
>
> Would it make sense to create another new API, apr_atomic_dec_and_test (or
> whatever) which is defined to return something consistant? I think Bill's code
> demonstrates how useful it is to have such a function for reference counting.
> The Linux kernel uses both atomic_dec and atomic_dec_and_test.
I think we should just modifiy the _dec function to return a value.
I'll patch this tonight if you don't beat me to it.
as for bsd.. I'm working on getting a freeBSD-current system which has a
atomic_cmpset_int
which will allow us to implement the rest of them..
sorry for not fixing this earlier..
ian
>
> We already have a return value from apr_atomic_cas; I don't know that we need a
> return value for the other atomics.
>
> Greg
>
> ----------------------------------------------------------------------------
>
> a quick hack to allow apr_atomic_dec to work on Linux for refcounts:
>
> Index: srclib/apr/include/apr_atomic.h
> ===================================================================
> RCS file: /cvs/apache/apr/include/apr_atomic.h,v
> retrieving revision 1.15
> diff -u -d -b -r1.15 apr_atomic.h
> --- apr_atomic.h 2002/03/13 20:39:13 1.15
> +++ apr_atomic.h 2002/03/15 17:36:13
> @@ -117,6 +117,7 @@
> /**
> * decrement the atomic variable by 1
> * @param mem pointer to the atomic value
> + * @return false (0) if the value became 0; otherwise true
> */
> void apr_atomic_dec(volatile apr_atomic_t *mem);
>
> @@ -158,7 +159,7 @@
> #define apr_atomic_t atomic_t
>
> #define apr_atomic_add(mem, val) atomic_add(val,mem)
> -#define apr_atomic_dec(mem) atomic_dec(mem)
> +#define apr_atomic_dec(mem) !atomic_dec_and_test(mem)
> #define apr_atomic_inc(mem) atomic_inc(mem)
> #define apr_atomic_set(mem, val) atomic_set(mem, val)
> #define apr_atomic_read(mem) atomic_read(mem)
>
