[Asterisk-Dev] Race issue in channel.c involving uniqueint on Asterisk 1.2.1

[Luigi Rizzo]

On Fri, Jan 06, 2006 at 01:27:15PM -0600, Kevin P. Fleming wrote:
> Luigi Rizzo wrote:
> > in freebsd, and i believe other systems as well, there is a set of
> > functions that do atomic manipulation of variables e.g.
> > (from i386/include/atomic.h):
> 
> I have had this discussion with Mark previously... There were some 
> issues the last time I looked (when I was working on using atomic ops 
> for object reference counts in astobj2):
> 
> 1) FreeBSD (at the time) did not have atomic_dec_and_test, which is 
> vital for reference count implementation.
> 
> 2) The Linux headers don't necessarily provide atomic behavior (as dwmw2 
> has already posted); it depends on how they are configured. Forcibly 
> defining CONFIG_SMP before including asm/atomic.h helps to work around 
> that problem. There is also the issue of these files being GPL licensed 
> and producing object code that ends up in non-GPL Asterisk binaries, but 
> that's a bit of a gray area since the code also appears in other places 
> (other projects) and code from header files is sometimes treated 
> differently.
> 
> 3) Using these operations will make an enormous performance difference 
> between the platforms where they are easy to provide (Linux and FreeBSD) 
> and other platforms. Granted this is not as large an issue, but it's 
> still something to be aware of.

I posted this before, but it might have been missed.

(2) We do not need to rely on the system's headers, we can provide our own
implementation as shown at the end. It's 1 assembly instruction derived
from a bsd implementation, but i would bet that it appears on tons
of documentation all over the place and i doubt one could argue licensing
issues in this, or we would have trouble with any individual C statement.

(1) In terms of this you can implement the atomic_dec_and_test() as

	int atomic_dec_and_test(volatile int *p)
	{
		int a = atomic_dec_and_test(&p, 1);
		return a == 0;
	}

(3) the difference will be between platforms implementing the i386
    instructions, and others. Of course there is a simple fix - take
    (from FreeBSD, NetBSD, wherever) the relevant 1-3 lines of
    assembly. That should cover all the systems one could care about.
    For those platforms where you have to rely on the pthread mutexes,
    will not be doing worse than now, and if you really care about
    excessive lock contention in my simplistic implementation below,
    you can always define an array of mutexes (say 16) and then use one or
    another depending on bits 7..4 of the address of the variable p.

cheers
luigi


Sample code:

        /*
         * Atomically add the value of v to the integer pointed to by p and return
         * the previous value of *p.
         */
        static __inline u_int
        ast_atomic_fetchadd_int(volatile int *p, int v)
        {

	#ifdef __i386__
                __asm __volatile (
                "       lock                    "
                "       xaddl   %0, %1 ;        "
                "# atomic_fetchadd_int"
                : "+r" (v),                     /* 0 (result) */   
                  "=m" (*p)                     /* 1 */
                : "m" (*p));                    /* 2 */
                return (v);
	#else	/* low performance version */
		static <a mutex to be used for all fetc&add ops> m;
		int ret;
		ast_mutex_lock(&m);
		ret = *p;
		*p += v;
		ast_mutex_unlock(&m);
		return ret;
        }
	#else /* low performance */


> --Bandwidth and Colocation provided by Easynews.com --
> 
> Asterisk-Dev mailing list
> To UNSUBSCRIBE or update options visit:
>    http://lists.digium.com/mailman/listinfo/asterisk-dev