Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 25 Aug 2007 15:23:10 -0700 (PDT)
From:      Jeff Roberson <jroberson@chesapeake.net>
To:        Kris Kennaway <kris@freebsd.org>
Cc:        Attilio Rao <attilio@freebsd.org>, Alfred Perlstein <alfred@freebsd.org>, Pawel Jakub Dawidek <pjd@freebsd.org>, freebsd-arch@freebsd.org
Subject:   Re: Lockless uidinfo.
Message-ID:  <20070825151913.S568@10.0.0.1>
In-Reply-To: <20070824142952.GA24469@hub.freebsd.org>
References:  <20070818120056.GA6498@garage.freebsd.pl> <20070818155041.GY90381@elvis.mu.org> <20070818161449.GE6498@garage.freebsd.pl> <200708211403.29293.jhb@freebsd.org> <20070821191902.GA4187@garage.freebsd.pl> <20070821202136.GB4187@garage.freebsd.pl> <3bbf2fe10708221202h44b3258cyf5ca5e9b867ac0e7@mail.gmail.com> <20070824140927.GC14536@garage.freebsd.pl> <20070824142952.GA24469@hub.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

On Fri, 24 Aug 2007, Kris Kennaway wrote:

> On Fri, Aug 24, 2007 at 04:09:27PM +0200, Pawel Jakub Dawidek wrote:
>> On Wed, Aug 22, 2007 at 09:02:53PM +0200, Attilio Rao wrote:
>>> 2007/8/21, Pawel Jakub Dawidek <pjd@freebsd.org>:
>>>>
>>>> New patch is here:
>>>>
>>>>         http://people.freebsd.org/~pjd/patches/uidinfo_waitfree.patch
>>>
>>> ---  sys/ia64/include/atomic.h.orig
>>> +++  sys/ia64/include/atomic.h
>>> @@ -370,4 +370,15 @@
>>>
>>>  #define	atomic_fetchadd_int		atomic_fetchadd_32
>>>
>>> +static __inline u_long
>>> +atomic_fetchadd_long(volatile u_long *p, u_long v)
>>> +{
>>> +	u_long value;
>>> +
>>> +	do {
>>> +		value = *p;
>>> +	} while (!atomic_cmpset_64(p, value, value + v));
>>> +	return (value);
>>> +}
>>> +
>>>
>>> In cycles like those, as you get spinning, I would arrange things in
>>> order to do a cpu_spinwait(). Like this:
>>>
>>> for (;;) {
>>>         value = *p;
>>>         if (atomic_cmpset_64(p, value, value + v))
>>>                 break;
>>>         cpu_spinwait();
>>> }
>>
>> In this case there is no difference as this is MI ia64 code and
>> cpu_spinwait() is defined as /* nothing */ there. As a general rule,
>> this might be a good idea.


I don't know that these kinds of loops really need cpu_spinwait().  If you 
consider that the loop condition is only triggered if a single instruction 
overlaps with another thread and one thread always wins, the number of 
cases where we restart more than once is negligible.  I believe pjd's test 
that was artificially constructed to cause as much contention as possible 
still only saw 30% loop once.

This is contrasted with spin-wait code where no-one is making any progress 
while a lock is held.  I think this just adds complexity to the code 
without any advantage.

The cpu_spinwait() function is meant to stop one HTT core from starving 
another that is holding a lock.  In this case there is no lock and so 
there is no starvation possible.  Actually by spinwaiting you may increase 
the chance for a second loop.

Jeff

>
> Better to still do it in case that changes.
>
> Kris
> _______________________________________________
> freebsd-arch@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-arch
> To unsubscribe, send any mail to "freebsd-arch-unsubscribe@freebsd.org"
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070825151913.S568>