From owner-freebsd-arch@FreeBSD.ORG Sun Aug 19 00:39:17 2007 Return-Path: Delivered-To: freebsd-arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1B55F16A473; Sun, 19 Aug 2007 00:39:17 +0000 (UTC) (envelope-from bright@elvis.mu.org) Received: from elvis.mu.org (elvis.mu.org [192.203.228.196]) by mx1.freebsd.org (Postfix) with ESMTP id B438013C46E; Sun, 19 Aug 2007 00:39:14 +0000 (UTC) (envelope-from bright@elvis.mu.org) Received: by elvis.mu.org (Postfix, from userid 1192) id 5EBF71A4D7C; Sat, 18 Aug 2007 17:37:32 -0700 (PDT) Date: Sat, 18 Aug 2007 17:37:32 -0700 From: Alfred Perlstein To: Maciej Sobczak Message-ID: <20070819003732.GA87451@elvis.mu.org> References: <20070818120056.GA6498@garage.freebsd.pl> <20070818142337.GW90381@elvis.mu.org> <20070818150028.GD6498@garage.freebsd.pl> <20070818155041.GY90381@elvis.mu.org> <20070818161449.GE6498@garage.freebsd.pl> <46C75045.8000503@msobczak.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <46C75045.8000503@msobczak.com> User-Agent: Mutt/1.4.2.3i Cc: Pawel Jakub Dawidek , freebsd-arch@FreeBSD.org Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 19 Aug 2007 00:39:17 -0000 * Maciej Sobczak [070818 13:27] wrote: > Pawel Jakub Dawidek wrote: > > >thread1 (uifind) thread2 (uifree) > >---------------- ---------------- > > refcount_release(&uip->ui_ref)) > > /* ui_ref == 0 */ > >mtx_lock(&uihashtbl_mtx); > >refcount_acquire(&uip->ui_ref); > >/* ui_ref == 1 */ > >mtx_unlock(&uihashtbl_mtx); > > mtx_lock(&uihashtbl_mtx); > > if (uip->ui_ref > 0) { > > mtx_unlock(&uihashtbl_mtx); > > return; > > } > > > >Now, you suggest that ui_ref in 'if (uip->ui_ref > 0)' may still have > >cached 0? I don't think it is possible, first refcount_acquire() uses > >read memory bariers (but we may still need ui_ref to volatile for this > >to make any difference) and second, think of ui_ref as a field protected > >by uihashtbl_mtx mutex in this very case. > > > >Is my thinking correct? > > Yes, but I believe you are too conservative even with the above explanation. > > Unlocking (thread1) and subsequent locking (thread2) of the same mutex > guarantees memory visibility between threads, at least if the mutex > provides the fundamental release-acquire consistency. In this case, the > memory barrier is part of this process itself and you don't need to do > anything else to guarantee the visibility of ui_ref == 1 in thread2. > > The only thing to worry about is caching of values in CPU registers > (note that this issue is separate from visibility), but these should be > prevented by the compiler at the point of mtx_lock. There are basically > two ways to guarantee it: either the compiler is too stupid/conservative > to cache the value across mtx_lock if it's a function call, or it is > smart enough to know (or just see) that there is a membar inside. In any > case no register-level caching will take place. > > There should be no need to make anything volatile. That sounds right, otherwise the reading of the refcount itself in the previous code would have required special handling. -Alfred From owner-freebsd-arch@FreeBSD.ORG Sun Aug 19 04:53:57 2007 Return-Path: Delivered-To: freebsd-arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8F84C16A419; Sun, 19 Aug 2007 04:53:57 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail13.syd.optusnet.com.au (mail13.syd.optusnet.com.au [211.29.132.194]) by mx1.freebsd.org (Postfix) with ESMTP id 2D46C13C428; Sun, 19 Aug 2007 04:53:56 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c220-239-235-248.carlnfd3.nsw.optusnet.com.au (c220-239-235-248.carlnfd3.nsw.optusnet.com.au [220.239.235.248]) by mail13.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id l7J4rrwo029490 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 19 Aug 2007 14:53:55 +1000 Date: Sun, 19 Aug 2007 14:53:53 +1000 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Pawel Jakub Dawidek In-Reply-To: <20070818220756.GH6498@garage.freebsd.pl> Message-ID: <20070819142214.O34036@delplex.bde.org> References: <20070818120056.GA6498@garage.freebsd.pl> <20070818220756.GH6498@garage.freebsd.pl> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-arch@FreeBSD.org Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 19 Aug 2007 04:53:57 -0000 On Sun, 19 Aug 2007, Pawel Jakub Dawidek wrote: > Two more things... > >> The patch below remove per-uidinfo locks: >> >> http://people.freebsd.org/~pjd/patches/uidinfo_lockless.patch > > We could upgrade from lock-free algorithm I used here to wait-free > algorithm, but we don't have atomic_fetchadd_long(). How hard will it be > to implement it? > > We could then change: > > do { > old = uip->ui_proccnt; > if (old + diff > max) > return (0); > } while (atomic_cmpset_long(&uip->ui_proccnt, old, old + diff) == 0); > > to something like this: > > if (atomic_fetchadd_long(&uip->ui_proccnt, diff) + diff > max) { > atomic_subtract_long(&uip->ui_proccnt, diff); > return (0); > } atomic_*long() shouldn't exist (and doesn't exist in my version) since longs should actually be long (twice as long as registers) and thus especially epensive to lock. long is just a bogus historical type for ui_proccnt (in case int is 16 bits, and the null set of old systems with 16-bit ints have non-old CPU resources for >= 65536 processes per user). There are several variables like this. This causes problems spelling atomic accesses to such variables. Sometimes on 64-bit machines, asking for a long (128 bits) is actually useful for the bogus reason that long has the wrong size (64 bits) and 64-bits is actually useful on 64-bit machines though not needed on 32-bit machines. >> I needed to change ui_sbsize from rlim_t (64bit) to long, because we >> don't have 64bit atomics on all archs, and because sbsize represents >> size in bytes, it can't go beyond 32bit on 32bit archs (PAE might be a >> bit of a problem). This problem is similar. rlim_t is 64 bits since 64 bits is needed for a few limits and 128 bits isn't needed yet, so the same type is used for all limits. This causes problems locking it even on 64-bit machines where longs have the wrong size (not 2*64), since although 64-bit machines have 64-bit atomics, there is no good way to spell the atomic accesses to an rlim_t. On 32-bit machines, there is no way at all to spell the atomic accesses to an rlim_t. Here the spelling problems prevent misuse of atomics on longs. Otherwise, the type for a size should be something like vm_size_t, and then the problem is the spelling of atomic accesses to vm_size_t's. This problem is handled for pointers by type-punning pointers to ints or longs in . (The amd64 atomic.h uses longs. It should use int64_t or register_t. It uses 8-bit DOS/Windowspeak "Operations on 32-bit double words" and "Operations on 64-bit quad words" in comments copied from the i386 version. On 64-bit machines, 32 bits is half a word, not a double word...) Bruce From owner-freebsd-arch@FreeBSD.ORG Sun Aug 19 07:50:03 2007 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id C8A3116A418; Sun, 19 Aug 2007 07:50:03 +0000 (UTC) (envelope-from jroberson@chesapeake.net) Received: from webaccess-cl.virtdom.com (webaccess-cl.virtdom.com [216.240.101.25]) by mx1.freebsd.org (Postfix) with ESMTP id A1C0813C458; Sun, 19 Aug 2007 07:50:03 +0000 (UTC) (envelope-from jroberson@chesapeake.net) Received: from [192.168.1.103] (c-67-160-44-208.hsd1.wa.comcast.net [67.160.44.208]) (authenticated bits=0) by webaccess-cl.virtdom.com (8.13.6/8.13.6) with ESMTP id l7J7nvbu079433 (version=TLSv1/SSLv3 cipher=DHE-DSS-AES256-SHA bits=256 verify=NO); Sun, 19 Aug 2007 03:49:58 -0400 (EDT) (envelope-from jroberson@chesapeake.net) Date: Sun, 19 Aug 2007 00:52:44 -0700 (PDT) From: Jeff Roberson X-X-Sender: jroberson@10.0.0.1 To: Bruce Evans In-Reply-To: <20070819142214.O34036@delplex.bde.org> Message-ID: <20070819004949.U568@10.0.0.1> References: <20070818120056.GA6498@garage.freebsd.pl> <20070818220756.GH6498@garage.freebsd.pl> <20070819142214.O34036@delplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: Pawel Jakub Dawidek , freebsd-arch@freebsd.org Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 19 Aug 2007 07:50:03 -0000 On Sun, 19 Aug 2007, Bruce Evans wrote: > On Sun, 19 Aug 2007, Pawel Jakub Dawidek wrote: > >> Two more things... >> >>> The patch below remove per-uidinfo locks: >>> >>> http://people.freebsd.org/~pjd/patches/uidinfo_lockless.patch >> >> We could upgrade from lock-free algorithm I used here to wait-free >> algorithm, but we don't have atomic_fetchadd_long(). How hard will it be >> to implement it? >> >> We could then change: >> >> do { >> old = uip->ui_proccnt; >> if (old + diff > max) >> return (0); >> } while (atomic_cmpset_long(&uip->ui_proccnt, old, old + diff) == 0); >> >> to something like this: >> >> if (atomic_fetchadd_long(&uip->ui_proccnt, diff) + diff > max) { >> atomic_subtract_long(&uip->ui_proccnt, diff); >> return (0); >> } > > atomic_*long() shouldn't exist (and doesn't exist in my version) since > longs should actually be long (twice as long as registers) and thus > especially epensive to lock. Well unfortunately this is not how the compiler implements them on the architectures that we support. So in this case long is 32bit on 32bit machines and 64bit on 64bit machines and as such requires each architecture to treat them specially. I don't think it's unreasonable to add an atomic_fetchadd_long() that conforms to the definition of long that is actually in use. Jeff > > long is just a bogus historical type for ui_proccnt (in case int is > 16 bits, and the null set of old systems with 16-bit ints have non-old > CPU resources for >= 65536 processes per user). There are several > variables like this. This causes problems spelling atomic accesses to > such variables. Sometimes on 64-bit machines, asking for a long > (128 bits) is actually useful for the bogus reason that long has the > wrong size (64 bits) and 64-bits is actually useful on 64-bit machines > though not needed on 32-bit machines. > >>> I needed to change ui_sbsize from rlim_t (64bit) to long, because we >>> don't have 64bit atomics on all archs, and because sbsize represents >>> size in bytes, it can't go beyond 32bit on 32bit archs (PAE might be a >>> bit of a problem). > > This problem is similar. rlim_t is 64 bits since 64 bits is needed for > a few limits and 128 bits isn't needed yet, so the same type is used for > all limits. This causes problems locking it even on 64-bit machines > where longs have the wrong size (not 2*64), since although 64-bit machines > have 64-bit atomics, there is no good way to spell the atomic accesses to > an rlim_t. On 32-bit machines, there is no way at all to spell the atomic > accesses to an rlim_t. Here the spelling problems prevent misuse of atomics > on longs. > > Otherwise, the type for a size should be something like vm_size_t, and > then the problem is the spelling of atomic accesses to vm_size_t's. This > problem is handled for pointers by type-punning pointers to ints or > longs in . (The amd64 atomic.h uses longs. It > should use int64_t or register_t. It uses 8-bit DOS/Windowspeak > "Operations on 32-bit double words" and "Operations on 64-bit quad > words" in comments copied from the i386 version. On 64-bit machines, > 32 bits is half a word, not a double word...) > > Bruce > _______________________________________________ > 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" > From owner-freebsd-arch@FreeBSD.ORG Sun Aug 19 07:58:55 2007 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 7E3A016A419 for ; Sun, 19 Aug 2007 07:58:55 +0000 (UTC) (envelope-from pjd@garage.freebsd.pl) Received: from mail.garage.freebsd.pl (arm132.internetdsl.tpnet.pl [83.17.198.132]) by mx1.freebsd.org (Postfix) with ESMTP id D420B13C465 for ; Sun, 19 Aug 2007 07:58:54 +0000 (UTC) (envelope-from pjd@garage.freebsd.pl) Received: by mail.garage.freebsd.pl (Postfix, from userid 65534) id C4371487F0; Sun, 19 Aug 2007 09:58:52 +0200 (CEST) Received: from localhost (154.81.datacomsa.pl [195.34.81.154]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.garage.freebsd.pl (Postfix) with ESMTP id 4FE6845685; Sun, 19 Aug 2007 09:58:48 +0200 (CEST) Date: Sun, 19 Aug 2007 09:57:50 +0200 From: Pawel Jakub Dawidek To: Jeff Roberson Message-ID: <20070819075750.GB11792@garage.freebsd.pl> References: <20070818120056.GA6498@garage.freebsd.pl> <20070818220756.GH6498@garage.freebsd.pl> <20070818230917.GI6498@garage.freebsd.pl> <20070818163503.T568@10.0.0.1> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="0eh6TmSyL6TZE2Uz" Content-Disposition: inline In-Reply-To: <20070818163503.T568@10.0.0.1> User-Agent: Mutt/1.4.2.3i X-PGP-Key-URL: http://people.freebsd.org/~pjd/pjd.asc X-OS: FreeBSD 7.0-CURRENT i386 X-Spam-Checker-Version: SpamAssassin 3.0.4 (2005-06-05) on mail.garage.freebsd.pl X-Spam-Level: X-Spam-Status: No, score=-1.8 required=3.0 tests=BAYES_00,WHY_WAIT autolearn=no version=3.0.4 Cc: freebsd-arch@freebsd.org Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 19 Aug 2007 07:58:55 -0000 --0eh6TmSyL6TZE2Uz Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Aug 18, 2007 at 04:35:42PM -0700, Jeff Roberson wrote: > On Sun, 19 Aug 2007, Pawel Jakub Dawidek wrote: > >Ok, after implementing atomic_fetchadd_long() on amd64, we get additional > >6% of performance improvement: > > > >x ./uidinfo_lockfree.txt (atomic_cmpset_long loop) > >+ ./uidinfo_waitfree.txt (atomic_fetchadd_long) > >+-----------------------------------------------------------------------= -------+ > >| = =20 > >+| > >| = =20 > >+| > >|x xx xx = =20 > >+ ++| > >| |__MA___| = =20 > >|AM| > >+-----------------------------------------------------------------------= -------+ > > N Min Max Median Avg Stdd= ev > >x 5 1561566 1575987 1568964 1569767 5853.1= 399 > >+ 5 1662362 1665936 1665810 1664881.8 1541.2= 693 > >Difference at 95.0% confidence > > 95114.8 +/- 6241.96 > > 6.05917% +/- 0.397636% > > (Student's t, pooled s =3D 4279.88) >=20 > How does this effect the single-threaded performance? Do you attribute= =20 > this to atomic fetchadd being cheaper than atomic cmpset? What is your= =20 > processor? CPU: Intel(R) Xeon(R) CPU E5310 @ 1.60GHz (1597.65-MHz K8-class CPU) Origin =3D "GenuineIntel" Id =3D 0x6f7 Stepping =3D 7 Features=3D0xbfebfbff Features2=3D0x4e33d AMD Features=3D0x20100800 AMD Features2=3D0x1 Cores per package: 4 Ok, I changed the code to something like this: long old; int diff, loops; atomic_add_int(&uidinfo_cnt1, 1); if (diff > 0) { loops =3D 0; do { loops++; old =3D uip->ui_sbsize; if (old + diff > max) return (0); } while (atomic_cmpset_long(&uip->ui_sbsize, old, old + diff) =3D=3D 0); if (loops > 1) atomic_add_int(&uidinfo_cnt2, loops); } else { atomic_add_long(&uip->ui_sbsize, (long)diff); } This allows me to see how many additional loops I do, because with lock-free version we still can have contention and loop, that's why wait-free version is superior. Actually I was a bit surprised with the results: debug.uidinfo.cnt1: 88746008 debug.uidinfo.cnt2: 31296304 (Running 8 processes.) Which means, because of contention, we do 31296304 additional atomic operations, which is about 30% more. --=20 Pawel Jakub Dawidek http://www.wheel.pl pjd@FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! --0eh6TmSyL6TZE2Uz Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.4 (FreeBSD) iD8DBQFGx/f9ForvXbEpPzQRAoIwAKCL/fLfk/Wow6njyNFLyOXjKky5RwCfUKoX 7ZGZAv/M+5w9Xu5RFPFoJRE= =yhzP -----END PGP SIGNATURE----- --0eh6TmSyL6TZE2Uz-- From owner-freebsd-arch@FreeBSD.ORG Sun Aug 19 08:40:30 2007 Return-Path: Delivered-To: freebsd-arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 5E66016A417 for ; Sun, 19 Aug 2007 08:40:30 +0000 (UTC) (envelope-from pjd@garage.freebsd.pl) Received: from mail.garage.freebsd.pl (arm132.internetdsl.tpnet.pl [83.17.198.132]) by mx1.freebsd.org (Postfix) with ESMTP id B164213C459 for ; Sun, 19 Aug 2007 08:40:29 +0000 (UTC) (envelope-from pjd@garage.freebsd.pl) Received: by mail.garage.freebsd.pl (Postfix, from userid 65534) id AECBA45696; Sun, 19 Aug 2007 10:40:27 +0200 (CEST) Received: from localhost (154.81.datacomsa.pl [195.34.81.154]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.garage.freebsd.pl (Postfix) with ESMTP id 1C95245681 for ; Sun, 19 Aug 2007 10:40:21 +0200 (CEST) Date: Sun, 19 Aug 2007 10:39:22 +0200 From: Pawel Jakub Dawidek To: freebsd-arch@FreeBSD.org Message-ID: <20070819083922.GC11792@garage.freebsd.pl> References: <20070818120056.GA6498@garage.freebsd.pl> <20070818220756.GH6498@garage.freebsd.pl> <20070818230917.GI6498@garage.freebsd.pl> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="s9fJI615cBHmzTOP" Content-Disposition: inline In-Reply-To: <20070818230917.GI6498@garage.freebsd.pl> User-Agent: Mutt/1.4.2.3i X-PGP-Key-URL: http://people.freebsd.org/~pjd/pjd.asc X-OS: FreeBSD 7.0-CURRENT i386 X-Spam-Checker-Version: SpamAssassin 3.0.4 (2005-06-05) on mail.garage.freebsd.pl X-Spam-Level: X-Spam-Status: No, score=-2.6 required=3.0 tests=BAYES_00 autolearn=ham version=3.0.4 Cc: Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 19 Aug 2007 08:40:30 -0000 --s9fJI615cBHmzTOP Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Aug 19, 2007 at 01:09:17AM +0200, Pawel Jakub Dawidek wrote: > Ok, after implementing atomic_fetchadd_long() on amd64, we get additional > 6% of performance improvement: >=20 > x ./uidinfo_lockfree.txt (atomic_cmpset_long loop) > + ./uidinfo_waitfree.txt (atomic_fetchadd_long) > +------------------------------------------------------------------------= ------+ > | = +| > | = +| > |x xx xx = + ++| > | |__MA___| = |AM| > +------------------------------------------------------------------------= ------+ > N Min Max Median Avg Stdd= ev > x 5 1561566 1575987 1568964 1569767 5853.13= 99 > + 5 1662362 1665936 1665810 1664881.8 1541.26= 93 > Difference at 95.0% confidence > 95114.8 +/- 6241.96 > 6.05917% +/- 0.397636% > (Student's t, pooled s =3D 4279.88) One more thing - comparsion between waitfree method and when chgsbsize() is a no-op: x ./uidinfo_waitfree.txt (atomic_fetchadd_long) + ./uidinfo_none.txt (no chgsbsize) +--------------------------------------------------------------------------= ----+ |x xx + + + + = +| | |AM| |_______M_A________| = | +--------------------------------------------------------------------------= ----+ N Min Max Median Avg Stddev x 5 1662362 1665936 1665810 1664881.8 1541.2693 + 5 1718287 1744448 1726343 1728176.6 10271.936 Difference at 95.0% confidence 63294.8 +/- 10711.8 3.80176% +/- 0.643395% (Student's t, pooled s =3D 7344.66) This small speed up is of course because of atomics in waitfree case, but it shows that we can't do much better than waitfree version, as ideal case is = only 3.8% faster, at least for this benchmark. This was for eight processes and this is for one: x ./uidinfo_up_waitfree.txt (atomic_fetchadd_long) + ./uidinfo_up_none.txt (no chgsbsize) +--------------------------------------------------------------------------= ----+ |x x x x x + += +++| | |______________M_____A_____________________| |_= AM_|| +--------------------------------------------------------------------------= ----+ N Min Max Median Avg Stddev x 5 419799 431017 422877 424180.2 4265.4168 + 5 433705 434955 434696 434509.6 519.68 Difference at 95.0% confidence 10329.4 +/- 4431.34 2.43514% +/- 1.04468% (Student's t, pooled s =3D 3038.41) This means that something else slows that eight processes case a bit, but i= t's not uidinfo. Ok, I need to stop, I need to stop right now! --=20 Pawel Jakub Dawidek http://www.wheel.pl pjd@FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! --s9fJI615cBHmzTOP Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.4 (FreeBSD) iD8DBQFGyAG6ForvXbEpPzQRApYVAJ4oNRlFeOzCQHM6N1WtncBklkaQywCggSaK Afv4wV9jNZHRESumApJeqNg= =0DHg -----END PGP SIGNATURE----- --s9fJI615cBHmzTOP-- From owner-freebsd-arch@FreeBSD.ORG Sun Aug 19 12:44:51 2007 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8685F16A419; Sun, 19 Aug 2007 12:44:51 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail08.syd.optusnet.com.au (mail08.syd.optusnet.com.au [211.29.132.189]) by mx1.freebsd.org (Postfix) with ESMTP id 0BCAB13C457; Sun, 19 Aug 2007 12:44:50 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from besplex.bde.org (c220-239-235-248.carlnfd3.nsw.optusnet.com.au [220.239.235.248]) by mail08.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id l7JCicuu023875 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 19 Aug 2007 22:44:40 +1000 Date: Sun, 19 Aug 2007 22:44:38 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Jeff Roberson In-Reply-To: <20070819004949.U568@10.0.0.1> Message-ID: <20070819223351.R1132@besplex.bde.org> References: <20070818120056.GA6498@garage.freebsd.pl> <20070818220756.GH6498@garage.freebsd.pl> <20070819142214.O34036@delplex.bde.org> <20070819004949.U568@10.0.0.1> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: Pawel Jakub Dawidek , freebsd-arch@freebsd.org Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 19 Aug 2007 12:44:51 -0000 On Sun, 19 Aug 2007, Jeff Roberson wrote: > On Sun, 19 Aug 2007, Bruce Evans wrote: >> atomic_*long() shouldn't exist (and doesn't exist in my version) since >> longs should actually be long (twice as long as registers) and thus >> especially epensive to lock. > > Well unfortunately this is not how the compiler implements them on the > architectures that we support. So in this case long is 32bit on 32bit > machines and 64bit on 64bit machines and as such requires each architecture > to treat them specially. I don't think it's unreasonable to add an > atomic_fetchadd_long() that conforms to the definition of long that is > actually in use. The compiler has nothing to do with this. The implementation is FreeBSD's and it is poor, like I said. [Context lost to top posting] Bruce From owner-freebsd-arch@FreeBSD.ORG Sun Aug 19 13:35:51 2007 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4498116A419; Sun, 19 Aug 2007 13:35:51 +0000 (UTC) (envelope-from jroberson@chesapeake.net) Received: from webaccess-cl.virtdom.com (webaccess-cl.virtdom.com [216.240.101.25]) by mx1.freebsd.org (Postfix) with ESMTP id 0C5D013C45D; Sun, 19 Aug 2007 13:35:50 +0000 (UTC) (envelope-from jroberson@chesapeake.net) Received: from [192.168.1.103] (c-67-160-44-208.hsd1.wa.comcast.net [67.160.44.208]) (authenticated bits=0) by webaccess-cl.virtdom.com (8.13.6/8.13.6) with ESMTP id l7JDZiEA009290 (version=TLSv1/SSLv3 cipher=DHE-DSS-AES256-SHA bits=256 verify=NO); Sun, 19 Aug 2007 09:35:45 -0400 (EDT) (envelope-from jroberson@chesapeake.net) Date: Sun, 19 Aug 2007 06:38:30 -0700 (PDT) From: Jeff Roberson X-X-Sender: jroberson@10.0.0.1 To: Bruce Evans In-Reply-To: <20070819223351.R1132@besplex.bde.org> Message-ID: <20070819063348.M568@10.0.0.1> References: <20070818120056.GA6498@garage.freebsd.pl> <20070818220756.GH6498@garage.freebsd.pl> <20070819142214.O34036@delplex.bde.org> <20070819004949.U568@10.0.0.1> <20070819223351.R1132@besplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: Pawel Jakub Dawidek , freebsd-arch@freebsd.org Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 19 Aug 2007 13:35:51 -0000 On Sun, 19 Aug 2007, Bruce Evans wrote: > On Sun, 19 Aug 2007, Jeff Roberson wrote: > >> On Sun, 19 Aug 2007, Bruce Evans wrote: > >>> atomic_*long() shouldn't exist (and doesn't exist in my version) since >>> longs should actually be long (twice as long as registers) and thus >>> especially epensive to lock. >> >> Well unfortunately this is not how the compiler implements them on the >> architectures that we support. So in this case long is 32bit on 32bit >> machines and 64bit on 64bit machines and as such requires each architecture >> to treat them specially. I don't think it's unreasonable to add an >> atomic_fetchadd_long() that conforms to the definition of long that is >> actually in use. > > The compiler has nothing to do with this. The implementation is FreeBSD's > and it is poor, like I said. Well this really has little to do with the problem at hand. The long decision has already been made and it's not practical to change it now. Adding apis that accept the types that we've decided on should not be crippled because you don't like the types. I agree that we can't have atomics that are wider than architectures support, but that isn't the case here. > > [Context lost to top posting] I included the context that I needed. > > Bruce > From owner-freebsd-arch@FreeBSD.ORG Sun Aug 19 17:11:05 2007 Return-Path: Delivered-To: freebsd-arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 4C4B616A420; Sun, 19 Aug 2007 17:11:05 +0000 (UTC) (envelope-from nork@FreeBSD.org) Received: from sakura.ninth-nine.com (sakura.ninth-nine.com [219.127.74.120]) by mx1.freebsd.org (Postfix) with ESMTP id BCC7013C48A; Sun, 19 Aug 2007 17:11:04 +0000 (UTC) (envelope-from nork@FreeBSD.org) Received: from nadesico.ninth-nine.com (nadesico.ninth-nine.com [219.127.74.122]) by sakura.ninth-nine.com (8.14.1/8.14.1/NinthNine) with SMTP id l7JGsOF0024019; Mon, 20 Aug 2007 01:54:24 +0900 (JST) (envelope-from nork@FreeBSD.org) Date: Mon, 20 Aug 2007 01:54:24 +0900 From: Norikatsu Shigemura To: freebsd-geom@FreeBSD.org Message-Id: <20070820015424.42677df2.nork@FreeBSD.org> X-Mailer: Sylpheed 2.4.4 (GTK+ 2.10.14; i386-portbld-freebsd6.2) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-2.0.2 (sakura.ninth-nine.com [219.127.74.121]); Mon, 20 Aug 2007 01:54:24 +0900 (JST) Cc: freebsd-proliant@FreeBSD.org, freebsd-arch@FreeBSD.org Subject: Anyone, would you try to make GEOM_DDF1(4)? X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 19 Aug 2007 17:11:05 -0000 Hi GEOM Developers. I'm using HP Proliant DL140G3 with Adaptec HostRAID (SATA). But I couldn't make FreeBSD 6.2-R install CD probe RAID1, so I researched DL140G3's RAID system(Software RAID). As the result, I understood that Adaptec HostRAID supports DDF format[*1]. [*1] http://www.snia.org/standards/DDFv1_00.pdf I was trying to make geom_ddf1(4), but I couldn't make it, it's too hard for me:-(. So, anyone, would you try to make geom_ddf1(4)? Of course, RAID1 support only OK! :D I have two real DDF1 data from configurated SATAs(/dev/ad4 and /dev/ad6 are RAID1 mirrored) like following way[*2][*3]. And I confirmed no configured SATAs filled all NUL(0x00) on DDF1 Anchor. [*2] http://people.freebsd.org/~nork/DDF1/ad4-last6k.img http://people.freebsd.org/~nork/DDF1/ad6-last6k.img *NOTE* Please restructure ad?.img from ad?-last6k.img I made a test program[*4]. I got a result of /dev/ad4. Please see also following[*5]. [*4] http://people.freebsd.org/~nork/DDF1/ddf1.h http://people.freebsd.org/~nork/DDF1/ddf1_test.c [*3] - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - In real system (RAID1 configured): - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - # geom disk list ad4 : Mediasize: 80026361856 (75G) : # echo 80026361856/512-65536 | bc 156235952 # dd if=/dev/ad4 bs=512 skip=156235952 | ssh ... 'cat > ad4-last32m.img' 65536+0 records in : # dd if=/dev/ad6 bs=512 skip=156235952 | ssh ... 'cat > ad6-last32m.img' 65536+0 records in : - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - In my system: - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - # hexdump -C ad4-last32m.img | head 00000000 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| * 01ffe800 11 de 11 de 50 a0 0d fc 64 6f c1 3a 86 80 82 26 |..P...do:...&| ~~~~~~~~ : # echo `printf %d/512 0x01ffe800` | bc 65524 # dd if=ad4-last32m.img of=ad4-last6k bs=512 skip=65524 12+0 records in 12+0 records out 6144 bytes transferred in 0.000361 secs (17021006 bytes/sec) # dd if=ad6-last32m.img of=ad6-last6k bs=512 skip=65524 12+0 records in 12+0 records out 6144 bytes transferred in 0.000361 secs (17021006 bytes/sec) # echo 156235952+65524 | bc 156301476 # dd if=ad4-last6k.img bs=512 seek=156235952 of=ad4.img # dd if=ad6-last6k.img bs=512 seek=156235952 of=ad6.img - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - [*5] ------------------------------------ sizeof struct ddf1_ddf_header = 512 ------------------------------------ Signature = 0xde11de11 CRC = 0x77802615 DDF_Header_GUID = 64 6f c1 3a 86 80 82 26 64 6f c1 3a b0 74 c1 3a 18 79 c1 3a ff ff ff ff DDF_rev = 30 32 2e 30 30 2e 30 30 Sequence_Number = -1 Timestamp = -1 Open_Flag = 0xff Foreign_Flag = 0xff Disk_Grouping = 255 Primary_Header_LBA = 0x000000000950f8a4 (156301476) Secondary_Header_LBA = 0xffffffffffffffff (18446744073709551615) Header_Type = 0x0 Workspace_Length = 32768 Workspace_LBA = 0x00000000095078a4 (156268708) Max_PD_Entries = 15 Max_VD_Entries = 4 Max_Partitions = 1 Configuration_Record_Length = 2 Max_Primary_Element_Entries = 65535 Controller_Data_Section = 1 Controller_Data_Length = 1 Physical_Disk_Records_Section = 2 Physical_Disk_Records_Length = 2 Virtual_Disk_Records_Section = 4 Virtual_Disk_Records_Length = 1 Configuration_Records_Section = 5 Configuration_Records_Length = 4 Physical_Disk_Data_Section = 9 Physical_Disk_Data_Length = 1 BBM_Log_Section = -1 BBM_Log_Length = 0 Diagnostic_Space = -1 Diagnostic_Space_Length = 0 Vendor_Specific_Logs_Section = 10 Vendor_Specific_Logs_Section_Length = 1 ------------------------------------ sizeof struct ddf1_controller_data = 512 ------------------------------------ Signature = 0xad111111 CRC = 0x8be2d4cc Controller_GUID = 64 6f c1 3a b0 74 c1 3a 18 79 c1 3a 7a 7d c1 3a 41 44 50 54 ff ff ff ff Controller_Type_Vendor_ID = 0x8086 Controller_Type_Device_ID = 0x2682 Controller_Type_Sub_Vendor_ID = 0x0000 Controller_Type_Sub_Device_ID = 0x0000 ------------------------------------ sizeof struct ddf1_phsical_disk_records = 64 ------------------------------------ Signature = 0x22222222 CRC = 0x8da47ea5 Populated_PDEs = 2 Max_PDE_Supported = 15 ------------------------------------ *sizeof struct ddf1_physical_disk_entries = 64 ------------------------------------ PD_GUID = 20 20 20 20 20 20 20 20 20 20 20 20 39 4c 52 33 4c c6 95 5a ff ff ff ff PD_Reference = 1839256256 PD_Type = 0x2 PD_State = 0x1 Configured_Size = 155987856 Path_Information = 00 00 00 00 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ------------------------------------ *sizeof struct ddf1_physical_disk_entries = 64 ------------------------------------ PD_GUID = 20 20 20 20 20 20 20 20 20 20 20 20 39 4c 52 33 cc c7 95 5a ff ff ff ff PD_Reference = 1839269174 PD_Type = 0x2 PD_State = 0x1 Configured_Size = 155987856 Path_Information = 01 00 00 00 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ------------------------------------ sizeof struct ddf1_virtual_disk_records = 64 ------------------------------------ Signature = 0xdddddddd CRC = 0xdae2a21a Populated_VDEs = 1 Max_VDE_Supported = 4 ------------------------------------ *sizeof struct ddf1_virtual_disk_entries = 64 ------------------------------------ VD_GUID = 40 35 30 5a 86 80 82 26 20 20 20 20 20 20 20 20 e6 58 a0 6d 3a 35 4a 45 VD_Number = 0 VD_Type = 0xffffffff VD_State = 0x0 Init_State = 0x2 VD_Name = DDF1-MIRROR ------------------------------------ sizeof struct ddf1_virtual_disk_configuration_record = 512 ------------------------------------ Signature = 0xeeeeeeee CRC = 0x82e01b58 VD_GUID = 40 35 30 5a 86 80 82 26 20 20 20 20 20 20 20 20 e6 58 a0 6d 3a 35 4a 45 Timestamp = 2 Sequence_Number = 2 Primary_Element_Count = 2 Strip_Size(Stripe_Size) = 7 Primary_RAID_Level = 1 RAID_Level_Qualifier = 0 Secondary_Element_Count = 1 Secondary_Element_Seq = 0 Secondary_RAID_Level = 255 Block_Count = 155987856 Size = 155987856 Cache_Policies_And_Parameters = 0x0000000000000000 BG_Rate = 16 ------------------------------------ sizeof struct ddf1_physical_disk_data = 512 ------------------------------------ Signature = 0x33333333 CRC = 0xe46196d5 PD_GUID = 20 20 20 20 20 20 20 20 20 20 20 20 39 4c 52 33 c6 49 61 5a ff ff ff ff PD_Reference = 1839256256 Forced_Ref_Flag = 0xff Forced_PD_GUID_Flag = 0x00 From owner-freebsd-arch@FreeBSD.ORG Sun Aug 19 18:34:00 2007 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8DA2A16A419; Sun, 19 Aug 2007 18:34:00 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail15.syd.optusnet.com.au (mail15.syd.optusnet.com.au [211.29.132.196]) by mx1.freebsd.org (Postfix) with ESMTP id C807A13C4CC; Sun, 19 Aug 2007 18:33:56 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from besplex.bde.org (c220-239-235-248.carlnfd3.nsw.optusnet.com.au [220.239.235.248]) by mail15.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id l7JIXg9U030359 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 20 Aug 2007 04:33:44 +1000 Date: Mon, 20 Aug 2007 04:33:42 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Jeff Roberson In-Reply-To: <20070819063348.M568@10.0.0.1> Message-ID: <20070820031158.W2115@besplex.bde.org> References: <20070818120056.GA6498@garage.freebsd.pl> <20070818220756.GH6498@garage.freebsd.pl> <20070819142214.O34036@delplex.bde.org> <20070819004949.U568@10.0.0.1> <20070819223351.R1132@besplex.bde.org> <20070819063348.M568@10.0.0.1> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: Pawel Jakub Dawidek , freebsd-arch@freebsd.org Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 19 Aug 2007 18:34:00 -0000 On Sun, 19 Aug 2007, Jeff Roberson wrote: > On Sun, 19 Aug 2007, Bruce Evans wrote: > >> On Sun, 19 Aug 2007, Jeff Roberson wrote: >> >>> On Sun, 19 Aug 2007, Bruce Evans wrote: >> >>>> atomic_*long() shouldn't exist (and doesn't exist in my version) since >>>> longs should actually be long (twice as long as registers) and thus >>>> especially epensive to lock. >>> >>> Well unfortunately this is not how the compiler implements them on the >>> architectures that we support. So in this case long is 32bit on 32bit >>> machines and 64bit on 64bit machines and as such requires each >>> architecture to treat them specially. I don't think it's unreasonable to >>> add an atomic_fetchadd_long() that conforms to the definition of long that >>> is actually in use. >> >> The compiler has nothing to do with this. The implementation is FreeBSD's >> and it is poor, like I said. > > Well this really has little to do with the problem at hand. The long > decision has already been made and it's not practical to change it now. > Adding apis that accept the types that we've decided on should not be > crippled because you don't like the types. I agree that we can't have > atomics that are wider than architectures support, but that isn't the case > here. Long is wider than the arch supports for i386's with correctly sized longs (option _LARGE_LONG). I haven't tested that lately, since 64-bit arches provide testing for most of the unportabilities exposed by _LARGE_LONG. Removing atomic_*long is quite practical, since this mistake is rarely used. Grepping for atromic.*long outside of atomic.h shows: % ./amd64/include/pmap.h:#define pte_load_clear(pte) atomic_readandclear_long(pte) Really wants the pte type. % ./contrib/ipfilter/netinet/ip_compat.h:# define ATOMIC_INCL(x) atomic_add_long((uint32_t*)&(x), 1) % ./contrib/ipfilter/netinet/ip_compat.h:# define ATOMIC_DECL(x) atomic_add_long((uint32_t*)&(x), -1) % ./contrib/ipfilter/netinet/ip_compat.h:# define ATOMIC_INCL(x) atomic_add_long(&(x), 1) % ./contrib/ipfilter/netinet/ip_compat.h:# define ATOMIC_DECL(x) atomic_add_long(&(x), -1) % ./contrib/ipfilter/netinet/ip_compat.h:# define ATOMIC_INCL(x) atomicAddUlong(&(x), 1) % ./contrib/ipfilter/netinet/ip_compat.h:# define ATOMIC_DECL(x) atomicAddUlong(&(x), -1) The above are for non-FreeBSD (mainly Solaris) only, in ifdef tangle. % ./contrib/ipfilter/netinet/ip_compat.h:# define ATOMIC_INCL(x) atomic_add_long(&(x), 1) % ./contrib/ipfilter/netinet/ip_compat.h:# define ATOMIC_DECL(x) atomic_add_long(&(x), -1) These seem to be used mainly for statistics. % ./contrib/opensolaris/uts/common/sys/bitmap.h: { atomic_or_long(&(BT_WIM(bitmap, bitindex)), BT_BIW(bitindex)); } % ./contrib/opensolaris/uts/common/sys/bitmap.h: { atomic_and_long(&(BT_WIM(bitmap, bitindex)), ~BT_BIW(bitindex)); } % ./contrib/opensolaris/uts/common/sys/bitmap.h: { result = atomic_set_long_excl(&(BT_WIM(bitmap, bitindex)), \ % ./contrib/opensolaris/uts/common/sys/bitmap.h: { result = atomic_clear_long_excl(&(BT_WIM(bitmap, bitindex)), \ These seem to be unused. FreeBSD doesn't implement atomic ops with these spellings. Longs are an especially poor choice for bitmaps since any scalar type will work and asking for longs asks for expensive locking to implement the atomicity when longs are actually long. % ./sys/sched.h:#define SCHED_STAT_INC(var) atomic_add_long(&(var), 1) More statistics. The statistics variables are actually long, but this is useless since they are exported using SYSCTL_INT(), so userland can only see their low 32 bits (with overflow to a negative value if the highest of those bits is set). % ./sys/umtx.h: if (atomic_cmpset_acq_long(&umtx->u_owner, UMTX_UNOWNED, id) == 0) % ./sys/umtx.h: if (atomic_cmpset_acq_long(&umtx->u_owner, UMTX_UNOWNED, id) == 0) % ./sys/umtx.h: if (atomic_cmpset_acq_long(&umtx->u_owner, UMTX_UNOWNED, id) == 0) % ./sys/umtx.h: if (atomic_cmpset_rel_long(&umtx->u_owner, id, UMTX_UNOWNED) == 0) u_owner has type u_long. The bug is missing in , where uintptr_t and atomic_cmpset_acq_ptr() is used for the similar m_lock variable. % ./sparc64/include/pmap.h: atomic_add_long(&var, 1) More statistics. Correctly exported using SYSCTL_LONG() (signed counters, so not SYSCTL_ULONG). % ./amd64/amd64/pmap.c: if (!atomic_cmpset_long(pte, obits, pbits)) % ./amd64/amd64/pmap.c: atomic_set_long(pte, PG_W); % ./amd64/amd64/pmap.c: atomic_clear_long(pte, PG_W); % ./amd64/amd64/pmap.c: if (!atomic_cmpset_long(pte, oldpte, oldpte & % ./amd64/amd64/pmap.c: atomic_clear_long(pte, PG_A); % ./amd64/amd64/pmap.c: atomic_clear_long(pte, PG_M); % ./amd64/amd64/pmap.c: atomic_clear_long(pte, PG_A); % ./amd64/amd64/minidump_machdep.c: atomic_set_long(&vm_page_dump[idx], 1ul << bit); % ./amd64/amd64/minidump_machdep.c: atomic_clear_long(&vm_page_dump[idx], 1ul << bit); No problems using atomic.*long in MD code, so I didn't look at the details. There is also no need to use atomic.*long, since atomic.*64 would work identically modulo a large _LARGE_LONG option making longs actually long. Then long would be just wrong for ptes. % ./vm/redzone.c: atomic_add_long(&redzone_extra_mem, redzone_size_ntor(nsize) - nsize); % ./vm/redzone.c: atomic_subtract_long(&redzone_extra_mem, Really want the vm_size_t type? % ./compat/ndis/subr_ndis.c: atomic_add_long((u_long *)addend, 1); % ./compat/ndis/subr_ndis.c: atomic_subtract_long((u_long *)addend, 1); addend has type (uint32_t *), so the cast to (u_long *) seems to be just a type mismatch asking for buffer overrun if long is longer. % ./compat/ndis/subr_ntoskrnl.c: atomic_add_long((volatile u_long *)addend, 1); % ./compat/ndis/subr_ntoskrnl.c: atomic_subtract_long((volatile u_long *)addend, 1); As above. The bogus cast now requires volatile since addend now has type (volatile uint32_t *). % ./dev/cxgb/cxgb_offload.c: atomic_cmpset_ptr((long *)&t->tid_tab[tid].ctx, (long)NULL, (long)ctx); Not an atomic.*long call, but just 3 bogus casts which have little effect since the prototype will cause conversions to the types that the function actually takes. % ./kern/kern_clock.c: atomic_add_long(&cp_time[CP_NICE], 1); % ./kern/kern_clock.c: atomic_add_long(&cp_time[CP_USER], 1); % ./kern/kern_clock.c: atomic_add_long(&cp_time[CP_INTR], 1); % ./kern/kern_clock.c: atomic_add_long(&cp_time[CP_SYS], 1); % ./kern/kern_clock.c: atomic_add_long(&cp_time[CP_IDLE], 1); Recent mistakes. cp_time[] has always been long[] for bogus historical reasons. On 64-bit arches, a 64-bit type for cp_time[] is slightly better, but this was not very important since cp_time[] takes 480+ days to overflow 32 bits with HZ = 100. Now with HZ = 1000, it takes only 24+ days to overflow 31 bits. This shows that long is neither necessary nor sufficient for cp_time[]. It is too small for 32-bit arches. This is hard to fix without changing the cp_time[] API, except of course if longs are actually long -- then no API change is needed, but atomic_add_long() cannot be simple, efficient, or what it is now. BTW, it's bogus for the statclock counters in struct rusage_ext to be 64 bits while the above are only 32 bits. On 32-bit machines, the above counters normally overflow long before per-process statclock counters since they are system- wide and HZ is normally 8-10 times larger than stathz. % ./netgraph/ng_base.c: atomic_add_long(&ngq->q_flags, add_arg); % ./netgraph/ng_base.c: atomic_add_long(&ngq->q_flags, add_arg); % ./netgraph/ng_base.c: atomic_add_long(&ngq->q_flags, OP_PENDING); % ./netgraph/ng_base.c: atomic_add_long(&ngq->q_flags, READER_INCREMENT); % ./netgraph/ng_base.c: atomic_subtract_long(&ngq->q_flags, READER_INCREMENT); % ./netgraph/ng_base.c: atomic_add_long(&ngq->q_flags, READER_INCREMENT); % ./netgraph/ng_base.c: atomic_add_long(&ngq->q_flags, WRITER_ACTIVE); % ./netgraph/ng_base.c: atomic_subtract_long(&ngq->q_flags, WRITER_ACTIVE); % ./netgraph/ng_base.c: atomic_add_long(&ngq->q_flags, WRITER_ACTIVE - READER_INCREMENT); % ./netgraph/ng_base.c: atomic_add_long(&ngq->q_flags, % ./netgraph/ng_base.c: atomic_add_long(&ngq->q_flags, OP_PENDING); % ./netgraph/ng_base.c: atomic_add_long(&ngq->q_flags, READER_INCREMENT - WRITER_ACTIVE); % ./netgraph/ng_base.c: atomic_subtract_long(&ngq->q_flags, READER_INCREMENT); % ./netgraph/ng_base.c: atomic_subtract_long(&ngq->q_flags, WRITER_ACTIVE); % ./netgraph/ng_base.c: atomic_add_long(&ngq->q_flags, -OP_PENDING); q_flags has type u_long. I don't know if this is essential on 64-bit machines, or if so if overflow is a problem on 32-bit machines. It's strange to do addition and subrtraction on flags. % ./sparc64/sparc64/pmap.c: atomic_clear_long(&tp->tte_data, TD_CV); % ./sparc64/sparc64/pmap.c: atomic_set_long(&tp->tte_data, TD_CV); % ./sparc64/sparc64/pmap.c: data = atomic_readandclear_long(&tp->tte_data); % ./sparc64/sparc64/pmap.c: data = atomic_clear_long(&tp->tte_data, TD_REF | TD_SW | TD_W); % ./sparc64/sparc64/pmap.c: data = atomic_set_long(&tp->tte_data, TD_WIRED); % ./sparc64/sparc64/pmap.c: data = atomic_clear_long(&tp->tte_data, TD_WIRED); % ./sparc64/sparc64/pmap.c: data = atomic_clear_long(&tp->tte_data, TD_REF); % ./sparc64/sparc64/pmap.c: data = atomic_clear_long(&tp->tte_data, TD_W); % ./sparc64/sparc64/pmap.c: data = atomic_clear_long(&tp->tte_data, TD_REF); % ./sparc64/sparc64/pmap.c: data = atomic_clear_long(&tp->tte_data, TD_SW | TD_W); MD, not a problem. Summary: most uses of atomic.*long are for statistics and/or in secondary subsystems. Longs for statistics are convenient but not essential or right. Most statistics counters should be 32 bits if that is enough, else 64 bits, _not_ depending on whether long is 64 bits, but possibly depending on whether 64 bits would be too inefficient due to extra locking required to fake 64-bit atomic accesses. Bruce From owner-freebsd-arch@FreeBSD.ORG Tue Aug 21 18:08:26 2007 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id AB7D716A418; Tue, 21 Aug 2007 18:08:26 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from speedfactory.net (mail6.speedfactory.net [66.23.216.219]) by mx1.freebsd.org (Postfix) with ESMTP id 4EDEF13C4E9; Tue, 21 Aug 2007 18:08:25 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (unverified [66.23.211.162]) by speedfactory.net (SurgeMail 3.8k) with ESMTP id 204501075-1834499 for multiple; Tue, 21 Aug 2007 14:08:35 -0400 Received: from localhost.corp.yahoo.com (john@localhost [127.0.0.1]) (authenticated bits=0) by server.baldwin.cx (8.13.8/8.13.8) with ESMTP id l7LI8A1P000145; Tue, 21 Aug 2007 14:08:11 -0400 (EDT) (envelope-from jhb@freebsd.org) From: John Baldwin To: freebsd-arch@freebsd.org Date: Tue, 21 Aug 2007 14:03:28 -0400 User-Agent: KMail/1.9.6 References: <20070818120056.GA6498@garage.freebsd.pl> <20070818155041.GY90381@elvis.mu.org> <20070818161449.GE6498@garage.freebsd.pl> In-Reply-To: <20070818161449.GE6498@garage.freebsd.pl> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200708211403.29293.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [127.0.0.1]); Tue, 21 Aug 2007 14:08:11 -0400 (EDT) X-Virus-Scanned: ClamAV 0.88.3/4021/Tue Aug 21 11:42:43 2007 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=4.2 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx Cc: Alfred Perlstein , Pawel Jakub Dawidek Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Aug 2007 18:08:26 -0000 On Saturday 18 August 2007 12:14:49 pm Pawel Jakub Dawidek wrote: > On Sat, Aug 18, 2007 at 08:50:41AM -0700, Alfred Perlstein wrote: > > * Pawel Jakub Dawidek [070818 07:59] wrote: > > > Yes, to lookup uidinfo you need to hold uihashtbl_mtx mutex, so once you > > > hold it and ui_ref is 0, noone will be able to reference it, because it > > > has to wait to look it up. > > > > And the field doesn't need to be volatile to prevent cached/opportunitic > > reads? > > The only chance of something like this will be the scenario below: > > thread1 (uifind) thread2 (uifree) > ---------------- ---------------- > refcount_release(&uip->ui_ref)) > /* ui_ref == 0 */ > mtx_lock(&uihashtbl_mtx); > refcount_acquire(&uip->ui_ref); > /* ui_ref == 1 */ > mtx_unlock(&uihashtbl_mtx); > mtx_lock(&uihashtbl_mtx); > if (uip->ui_ref > 0) { > mtx_unlock(&uihashtbl_mtx); > return; > } > > Now, you suggest that ui_ref in 'if (uip->ui_ref > 0)' may still have > cached 0? I don't think it is possible, first refcount_acquire() uses > read memory bariers (but we may still need ui_ref to volatile for this > to make any difference) and second, think of ui_ref as a field protected > by uihashtbl_mtx mutex in this very case. > > Is my thinking correct? Memory barriers on another CPU don't mean anything about the CPU thread 2 is on. Memory barriers do not flush caches on other CPUs, etc. Normally when objects are refcounted in a table, the table holds a reference on the object, but that doesn't seem to be the case here. Have you tried doing something very simple in uifree(): { mtx_lock(&uihashtbl_mtx); if (refcount_release(...)) { LIST_REMOVE(); mtx_unlock(&uihashtbl_mtx); ... free(); } else mtx_unlock(&uihashtbl_mtx); } I wouldn't use a more complex algo in uifree() unless the simple one is shown to perform badly. Needless complexity is a hindrance to future maintenance. Also, even if you do go with the more complex route, I'd rather you reduce diffs with the current code by keeping the test as 'uip->ui_ref == 0' and keeping the removal code in the if-block. In chgproccnt() you should use atomic_fetchadd_long() to avoid a race when reading ui_proccnt. old = atomic_fetchadd_long(&uip->ui_proccnt, diff); if (old + diff < 0) printf("...."); OTOH, atomic_fetchadd_long() doesn't yet exist, so you will need to fix that, or just always use an atomic_cmpset() loop. -- John Baldwin From owner-freebsd-arch@FreeBSD.ORG Tue Aug 21 19:20:11 2007 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 45D1316A420; Tue, 21 Aug 2007 19:20:11 +0000 (UTC) (envelope-from pjd@garage.freebsd.pl) Received: from mail.garage.freebsd.pl (arm132.internetdsl.tpnet.pl [83.17.198.132]) by mx1.freebsd.org (Postfix) with ESMTP id 74EB113C46C; Tue, 21 Aug 2007 19:20:10 +0000 (UTC) (envelope-from pjd@garage.freebsd.pl) Received: by mail.garage.freebsd.pl (Postfix, from userid 65534) id 15639487F5; Tue, 21 Aug 2007 21:20:09 +0200 (CEST) Received: from localhost (154.81.datacomsa.pl [195.34.81.154]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.garage.freebsd.pl (Postfix) with ESMTP id 0CE2845683; Tue, 21 Aug 2007 21:20:02 +0200 (CEST) Date: Tue, 21 Aug 2007 21:19:02 +0200 From: Pawel Jakub Dawidek To: John Baldwin Message-ID: <20070821191902.GA4187@garage.freebsd.pl> References: <20070818120056.GA6498@garage.freebsd.pl> <20070818155041.GY90381@elvis.mu.org> <20070818161449.GE6498@garage.freebsd.pl> <200708211403.29293.jhb@freebsd.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="BOKacYhQ+x31HxR3" Content-Disposition: inline In-Reply-To: <200708211403.29293.jhb@freebsd.org> User-Agent: Mutt/1.4.2.3i X-PGP-Key-URL: http://people.freebsd.org/~pjd/pjd.asc X-OS: FreeBSD 7.0-CURRENT i386 X-Spam-Checker-Version: SpamAssassin 3.0.4 (2005-06-05) on mail.garage.freebsd.pl X-Spam-Level: X-Spam-Status: No, score=-2.6 required=3.0 tests=BAYES_00 autolearn=ham version=3.0.4 Cc: Alfred Perlstein , freebsd-arch@freebsd.org Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Aug 2007 19:20:11 -0000 --BOKacYhQ+x31HxR3 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Aug 21, 2007 at 02:03:28PM -0400, John Baldwin wrote: > On Saturday 18 August 2007 12:14:49 pm Pawel Jakub Dawidek wrote: > > On Sat, Aug 18, 2007 at 08:50:41AM -0700, Alfred Perlstein wrote: > > > * Pawel Jakub Dawidek [070818 07:59] wrote: > > > > Yes, to lookup uidinfo you need to hold uihashtbl_mtx mutex, so onc= e you > > > > hold it and ui_ref is 0, noone will be able to reference it, becaus= e it > > > > has to wait to look it up. > > >=20 > > > And the field doesn't need to be volatile to prevent cached/opportuni= tic > > > reads? > >=20 > > The only chance of something like this will be the scenario below: > >=20 > > thread1 (uifind) thread2 (uifree) > > ---------------- ---------------- > > refcount_release(&uip->ui_ref)) > > /* ui_ref =3D=3D 0 */ > > mtx_lock(&uihashtbl_mtx); > > refcount_acquire(&uip->ui_ref); > > /* ui_ref =3D=3D 1 */ > > mtx_unlock(&uihashtbl_mtx); > > mtx_lock(&uihashtbl_mtx); > > if (uip->ui_ref > 0) { > > mtx_unlock(&uihashtbl_mtx); > > return; > > } > >=20 > > Now, you suggest that ui_ref in 'if (uip->ui_ref > 0)' may still have > > cached 0? I don't think it is possible, first refcount_acquire() uses > > read memory bariers (but we may still need ui_ref to volatile for this > > to make any difference) and second, think of ui_ref as a field protected > > by uihashtbl_mtx mutex in this very case. > >=20 > > Is my thinking correct? >=20 > Memory barriers on another CPU don't mean anything about the CPU thread 2= is=20 > on. Memory barriers do not flush caches on other CPUs, etc. Normally wh= en=20 > objects are refcounted in a table, the table holds a reference on the obj= ect,=20 > but that doesn't seem to be the case here. [...] But the memory barrier from 'mtx_lock(&uihashtbl_mtx)' above 'if (uip->ui_ref > 0)' would do the trick and I can safely avoid using atomic read in this if statement, right? > [...] Have you tried doing something=20 > very simple in uifree(): >=20 > { > mtx_lock(&uihashtbl_mtx); > if (refcount_release(...)) { > LIST_REMOVE(); > mtx_unlock(&uihashtbl_mtx); > ... > free(); > } else > mtx_unlock(&uihashtbl_mtx); > } >=20 > I wouldn't use a more complex algo in uifree() unless the simple one is s= hown=20 > to perform badly. Needless complexity is a hindrance to future maintenan= ce. Of coure we could do that, but I was trying really hard to remove contention in the common case. Before we used UIDINFO_LOCK() in the common case, now you suggesting using global lock here, and I'd really, really prefer using one atomic only. > Also, even if you do go with the more complex route, I'd rather you reduc= e=20 > diffs with the current code by keeping the test as 'uip->ui_ref =3D=3D 0'= and=20 > keeping the removal code in the if-block. Will do. > In chgproccnt() you should use atomic_fetchadd_long() to avoid a race whe= n=20 > reading ui_proccnt. >=20 >=20 > old =3D atomic_fetchadd_long(&uip->ui_proccnt, diff); > if (old + diff < 0) > printf("...."); I'm aware of this race, but I don't find closing it that much important. We won't generate false positive here. My vote is to leave it as it is, because atomic_fetchadd_long() is slower on some archs than atomic_add_long(), ie. it is implemented using atomic_cmpset_long() loop, and as I checked by running 8 processes on 8way machine with older code that used atomic_cmpset_long() loop in 'diff > 0' case, there is almost one extra loop on every call, which makes it about 6% slower. > OTOH, atomic_fetchadd_long() doesn't yet exist, so you will need to fix t= hat,=20 > or just always use an atomic_cmpset() loop. I already implemented those. --=20 Pawel Jakub Dawidek http://www.wheel.pl pjd@FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! --BOKacYhQ+x31HxR3 Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.4 (FreeBSD) iD8DBQFGyzqmForvXbEpPzQRAquEAJ9fd9/Ys+F3sCWE22/A3ls+iLjtIACfZiJX /zfTVrohvXz+Av4X+OvInQU= =uQx4 -----END PGP SIGNATURE----- --BOKacYhQ+x31HxR3-- From owner-freebsd-arch@FreeBSD.ORG Tue Aug 21 20:22:46 2007 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B64DD16A418; Tue, 21 Aug 2007 20:22:46 +0000 (UTC) (envelope-from pjd@garage.freebsd.pl) Received: from mail.garage.freebsd.pl (arm132.internetdsl.tpnet.pl [83.17.198.132]) by mx1.freebsd.org (Postfix) with ESMTP id D1E3613C4B6; Tue, 21 Aug 2007 20:22:45 +0000 (UTC) (envelope-from pjd@garage.freebsd.pl) Received: by mail.garage.freebsd.pl (Postfix, from userid 65534) id 90AFB48804; Tue, 21 Aug 2007 22:22:44 +0200 (CEST) Received: from localhost (154.81.datacomsa.pl [195.34.81.154]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.garage.freebsd.pl (Postfix) with ESMTP id 1CF9645685; Tue, 21 Aug 2007 22:22:36 +0200 (CEST) Date: Tue, 21 Aug 2007 22:21:36 +0200 From: Pawel Jakub Dawidek To: John Baldwin Message-ID: <20070821202136.GB4187@garage.freebsd.pl> 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> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="3uo+9/B/ebqu+fSQ" Content-Disposition: inline In-Reply-To: <20070821191902.GA4187@garage.freebsd.pl> User-Agent: Mutt/1.4.2.3i X-PGP-Key-URL: http://people.freebsd.org/~pjd/pjd.asc X-OS: FreeBSD 7.0-CURRENT i386 X-Spam-Checker-Version: SpamAssassin 3.0.4 (2005-06-05) on mail.garage.freebsd.pl X-Spam-Level: X-Spam-Status: No, score=-2.6 required=3.0 tests=BAYES_00 autolearn=ham version=3.0.4 Cc: Alfred Perlstein , freebsd-arch@freebsd.org Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Aug 2007 20:22:46 -0000 --3uo+9/B/ebqu+fSQ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Aug 21, 2007 at 09:19:02PM +0200, Pawel Jakub Dawidek wrote: > > Memory barriers on another CPU don't mean anything about the CPU thread= 2 is=20 > > on. Memory barriers do not flush caches on other CPUs, etc. Normally = when=20 > > objects are refcounted in a table, the table holds a reference on the o= bject,=20 > > but that doesn't seem to be the case here. [...] >=20 > But the memory barrier from 'mtx_lock(&uihashtbl_mtx)' above > 'if (uip->ui_ref > 0)' would do the trick and I can safely avoid using > atomic read in this if statement, right? >=20 > > [...] Have you tried doing something=20 > > very simple in uifree(): > >=20 > > { > > mtx_lock(&uihashtbl_mtx); > > if (refcount_release(...)) { > > LIST_REMOVE(); > > mtx_unlock(&uihashtbl_mtx); > > ... > > free(); > > } else > > mtx_unlock(&uihashtbl_mtx); > > } > >=20 > > I wouldn't use a more complex algo in uifree() unless the simple one is= shown=20 > > to perform badly. Needless complexity is a hindrance to future mainten= ance. >=20 > Of coure we could do that, but I was trying really hard to remove > contention in the common case. Before we used UIDINFO_LOCK() in the > common case, now you suggesting using global lock here, and I'd really, > really prefer using one atomic only. >=20 > > Also, even if you do go with the more complex route, I'd rather you red= uce=20 > > diffs with the current code by keeping the test as 'uip->ui_ref =3D=3D = 0' and=20 > > keeping the removal code in the if-block. >=20 > Will do. >=20 > > In chgproccnt() you should use atomic_fetchadd_long() to avoid a race w= hen=20 > > reading ui_proccnt. > >=20 > >=20 > > old =3D atomic_fetchadd_long(&uip->ui_proccnt, diff); > > if (old + diff < 0) > > printf("...."); >=20 > I'm aware of this race, but I don't find closing it that much important. > We won't generate false positive here. My vote is to leave it as it is, > because atomic_fetchadd_long() is slower on some archs than > atomic_add_long(), ie. it is implemented using atomic_cmpset_long() > loop, and as I checked by running 8 processes on 8way machine with > older code that used atomic_cmpset_long() loop in 'diff > 0' case, > there is almost one extra loop on every call, which makes it about 6% > slower. >=20 > > OTOH, atomic_fetchadd_long() doesn't yet exist, so you will need to fix= that,=20 > > or just always use an atomic_cmpset() loop. >=20 > I already implemented those. New patch is here: http://people.freebsd.org/~pjd/patches/uidinfo_waitfree.patch --=20 Pawel Jakub Dawidek http://www.wheel.pl pjd@FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! --3uo+9/B/ebqu+fSQ Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.4 (FreeBSD) iD8DBQFGy0lQForvXbEpPzQRAgIlAKDIP5P5TWOBUAHsgLX7lIe4tYsgxwCgvmpD d/K97EVO24dghgaNS0woxDA= =rVvA -----END PGP SIGNATURE----- --3uo+9/B/ebqu+fSQ-- From owner-freebsd-arch@FreeBSD.ORG Tue Aug 21 21:12:02 2007 Return-Path: Delivered-To: freebsd-arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 7715816A417 for ; Tue, 21 Aug 2007 21:12:02 +0000 (UTC) (envelope-from prog@msobczak.com) Received: from mail1.fluidhosting.com (mx12.fluidhosting.com [204.14.89.2]) by mx1.freebsd.org (Postfix) with SMTP id F052E13C468 for ; Tue, 21 Aug 2007 21:12:01 +0000 (UTC) (envelope-from prog@msobczak.com) Received: (qmail 24586 invoked by uid 399); 21 Aug 2007 21:12:01 -0000 Received: from localhost (HELO maciej-sobczaks-computer.local) (maciej@msobczak.com@127.0.0.1) by localhost with ESMTP; 21 Aug 2007 21:12:01 -0000 X-Originating-IP: 127.0.0.1 Message-ID: <46CB5520.4090505@msobczak.com> Date: Tue, 21 Aug 2007 23:12:00 +0200 From: Maciej Sobczak User-Agent: Thunderbird 2.0.0.6 (Macintosh/20070728) MIME-Version: 1.0 To: John Baldwin References: <20070818120056.GA6498@garage.freebsd.pl> <20070818155041.GY90381@elvis.mu.org> <20070818161449.GE6498@garage.freebsd.pl> <200708211403.29293.jhb@freebsd.org> In-Reply-To: <200708211403.29293.jhb@freebsd.org> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit Cc: freebsd-arch@FreeBSD.org Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Aug 2007 21:12:02 -0000 John Baldwin wrote: > Memory barriers on another CPU don't mean anything about the CPU thread 2 is > on. Of course they do. Otherwise they would be completely useless. > Memory barriers do not flush caches on other CPUs, etc. Membars are not about caches, but about visibility and ordering. Cache is transparent and that is guaranteed by hardware - there is no need to involve any software tricks to make cache work correctly. You can influence their performance, but not correctness. -- Maciej Sobczak : http://www.msobczak.com/ Programming : http://www.msobczak.com/prog/ From owner-freebsd-arch@FreeBSD.ORG Tue Aug 21 21:53:45 2007 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 36A3916A417 for ; Tue, 21 Aug 2007 21:53:45 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from speedfactory.net (mail6.speedfactory.net [66.23.216.219]) by mx1.freebsd.org (Postfix) with ESMTP id E130213C474 for ; Tue, 21 Aug 2007 21:53:44 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (unverified [66.23.211.162]) by speedfactory.net (SurgeMail 3.8k) with ESMTP id 204537132-1834499 for multiple; Tue, 21 Aug 2007 17:53:43 -0400 Received: from localhost.corp.yahoo.com (john@localhost [127.0.0.1]) (authenticated bits=0) by server.baldwin.cx (8.13.8/8.13.8) with ESMTP id l7LLrexe001627; Tue, 21 Aug 2007 17:53:40 -0400 (EDT) (envelope-from jhb@freebsd.org) From: John Baldwin To: Maciej Sobczak Date: Tue, 21 Aug 2007 17:43:18 -0400 User-Agent: KMail/1.9.6 References: <20070818120056.GA6498@garage.freebsd.pl> <200708211403.29293.jhb@freebsd.org> <46CB5520.4090505@msobczak.com> In-Reply-To: <46CB5520.4090505@msobczak.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200708211743.19269.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [127.0.0.1]); Tue, 21 Aug 2007 17:53:40 -0400 (EDT) X-Virus-Scanned: ClamAV 0.88.3/4021/Tue Aug 21 11:42:43 2007 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=4.2 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx Cc: freebsd-arch@freebsd.org Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Aug 2007 21:53:45 -0000 On Tuesday 21 August 2007 05:12:00 pm Maciej Sobczak wrote: > John Baldwin wrote: > > > Memory barriers on another CPU don't mean anything about the CPU thread 2 is > > on. > > Of course they do. Otherwise they would be completely useless. No. If you invoke a membar on CPU 1, it does not affect "when" CPU 2 will see that write. It does not post the write out immediately or such. Some people get confused by that. All a membar does is to order writes on CPU 1. You can take advantage of that by using membar's on a lock cookie to guarantee CPU 2 will see writes done by CPU 1 when it acquires the lock, but just throwing a membar on an stand-alone atomic operation does not close any races or guarantee anything about when CPU 2 will see that write. -- John Baldwin From owner-freebsd-arch@FreeBSD.ORG Tue Aug 21 21:53:45 2007 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 97F2816A418; Tue, 21 Aug 2007 21:53:45 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from speedfactory.net (mail6.speedfactory.net [66.23.216.219]) by mx1.freebsd.org (Postfix) with ESMTP id 3925813C480; Tue, 21 Aug 2007 21:53:45 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (unverified [66.23.211.162]) by speedfactory.net (SurgeMail 3.8k) with ESMTP id 204537137-1834499 for multiple; Tue, 21 Aug 2007 17:53:44 -0400 Received: from localhost.corp.yahoo.com (john@localhost [127.0.0.1]) (authenticated bits=0) by server.baldwin.cx (8.13.8/8.13.8) with ESMTP id l7LLrexf001627; Tue, 21 Aug 2007 17:53:42 -0400 (EDT) (envelope-from jhb@freebsd.org) From: John Baldwin To: Pawel Jakub Dawidek Date: Tue, 21 Aug 2007 17:53:34 -0400 User-Agent: KMail/1.9.6 References: <20070818120056.GA6498@garage.freebsd.pl> <200708211403.29293.jhb@freebsd.org> <20070821191902.GA4187@garage.freebsd.pl> In-Reply-To: <20070821191902.GA4187@garage.freebsd.pl> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200708211753.34697.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [127.0.0.1]); Tue, 21 Aug 2007 17:53:42 -0400 (EDT) X-Virus-Scanned: ClamAV 0.88.3/4021/Tue Aug 21 11:42:43 2007 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=4.2 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx Cc: Alfred Perlstein , freebsd-arch@freebsd.org Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 21 Aug 2007 21:53:45 -0000 On Tuesday 21 August 2007 03:19:02 pm Pawel Jakub Dawidek wrote: > > Memory barriers on another CPU don't mean anything about the CPU thread 2 is > > on. Memory barriers do not flush caches on other CPUs, etc. Normally when > > objects are refcounted in a table, the table holds a reference on the object, > > but that doesn't seem to be the case here. [...] > > But the memory barrier from 'mtx_lock(&uihashtbl_mtx)' above > 'if (uip->ui_ref > 0)' would do the trick and I can safely avoid using > atomic read in this if statement, right? Yes, though that may be rather non-obvious to other people, or to yourself 6 months down the road. You should probably document it. > > [...] Have you tried doing something > > very simple in uifree(): > > > > { > > mtx_lock(&uihashtbl_mtx); > > if (refcount_release(...)) { > > LIST_REMOVE(); > > mtx_unlock(&uihashtbl_mtx); > > ... > > free(); > > } else > > mtx_unlock(&uihashtbl_mtx); > > } > > > > I wouldn't use a more complex algo in uifree() unless the simple one is shown > > to perform badly. Needless complexity is a hindrance to future maintenance. > > Of coure we could do that, but I was trying really hard to remove > contention in the common case. Before we used UIDINFO_LOCK() in the > common case, now you suggesting using global lock here, and I'd really, > really prefer using one atomic only. *sigh* You ignored my last sentence. You need to think about other people who come and read this later or yourself 12 months from now when you've paged out all the uidinfo stuff from your head. Hmm, what happens if you get this: thread A -------- refcount_release() - drops to 0 preempted thread B -------- uihold() - refcount goes to 1 ... uifree() - refcount goes to 0 removes object from table and frees it resumes mtx_lock(&uihashtbl); sees refcount of 0 so tries to destroy object again *BOOM* (corruption, panic, etc.) This is the race that the current code handles by taking a reference on the uid while it drops the uidinfo lock to acquire the table lock. Probably you need to not drop the last reference unless you hold the uihashtbl_mtx, which means not using refcount_release() and manually use an atomic_cmpset_int() if the refcount is > 1 to drop a ref to optimize the common case: old = uip->ui_refs; if (old > 1) { if (atomic_cmpset_int(&uip->ui_refs, old, old - 1)) return; } mtx_lock(&uihashtbl_mtx); if (refcount_release(&uip->ui_refs)) { /* remove from table and free */ } mtx_unlock(&uihashtbl_mtx); -- John Baldwin From owner-freebsd-arch@FreeBSD.ORG Wed Aug 22 06:37:00 2007 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6FF7116A41A; Wed, 22 Aug 2007 06:37:00 +0000 (UTC) (envelope-from pjd@garage.freebsd.pl) Received: from mail.garage.freebsd.pl (arm132.internetdsl.tpnet.pl [83.17.198.132]) by mx1.freebsd.org (Postfix) with ESMTP id 8D42813C45D; Wed, 22 Aug 2007 06:36:59 +0000 (UTC) (envelope-from pjd@garage.freebsd.pl) Received: by mail.garage.freebsd.pl (Postfix, from userid 65534) id 0878B48800; Wed, 22 Aug 2007 08:36:58 +0200 (CEST) Received: from localhost (154.81.datacomsa.pl [195.34.81.154]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.garage.freebsd.pl (Postfix) with ESMTP id 6D5CB45683; Wed, 22 Aug 2007 08:36:53 +0200 (CEST) Date: Wed, 22 Aug 2007 08:35:52 +0200 From: Pawel Jakub Dawidek To: John Baldwin Message-ID: <20070822063552.GC4187@garage.freebsd.pl> References: <20070818120056.GA6498@garage.freebsd.pl> <200708211403.29293.jhb@freebsd.org> <20070821191902.GA4187@garage.freebsd.pl> <200708211753.34697.jhb@freebsd.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="PuGuTyElPB9bOcsM" Content-Disposition: inline In-Reply-To: <200708211753.34697.jhb@freebsd.org> User-Agent: Mutt/1.4.2.3i X-PGP-Key-URL: http://people.freebsd.org/~pjd/pjd.asc X-OS: FreeBSD 7.0-CURRENT i386 X-Spam-Checker-Version: SpamAssassin 3.0.4 (2005-06-05) on mail.garage.freebsd.pl X-Spam-Level: X-Spam-Status: No, score=-2.6 required=3.0 tests=BAYES_00 autolearn=ham version=3.0.4 Cc: Alfred Perlstein , freebsd-arch@freebsd.org Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Aug 2007 06:37:00 -0000 --PuGuTyElPB9bOcsM Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Aug 21, 2007 at 05:53:34PM -0400, John Baldwin wrote: > On Tuesday 21 August 2007 03:19:02 pm Pawel Jakub Dawidek wrote: > > > Memory barriers on another CPU don't mean anything about the CPU thre= ad 2=20 > is=20 > > > on. Memory barriers do not flush caches on other CPUs, etc. Normall= y=20 > when=20 > > > objects are refcounted in a table, the table holds a reference on the= =20 > object,=20 > > > but that doesn't seem to be the case here. [...] > >=20 > > But the memory barrier from 'mtx_lock(&uihashtbl_mtx)' above > > 'if (uip->ui_ref > 0)' would do the trick and I can safely avoid using > > atomic read in this if statement, right? >=20 > Yes, though that may be rather non-obvious to other people, or to yoursel= f 6=20 > months down the road. You should probably document it. Agreed, I added a comment that should explain it. > > > I wouldn't use a more complex algo in uifree() unless the simple one = is=20 > shown=20 > > > to perform badly. Needless complexity is a hindrance to future=20 > maintenance. > >=20 > > Of coure we could do that, but I was trying really hard to remove > > contention in the common case. Before we used UIDINFO_LOCK() in the > > common case, now you suggesting using global lock here, and I'd really, > > really prefer using one atomic only. >=20 > *sigh* You ignored my last sentence. You need to think about other peop= le=20 > who come and read this later or yourself 12 months from now when you've p= aged=20 > out all the uidinfo stuff from your head. >=20 > Hmm, what happens if you get this: >=20 > thread A > -------- >=20 > refcount_release() - drops to 0 >=20 > preempted >=20 > thread B > -------- > uihold() - refcount goes to 1 > ... > uifree() - refcount goes to 0 > removes object from table and frees it >=20 > resumes >=20 > mtx_lock(&uihashtbl); >=20 > sees refcount of 0 so tries to destroy object again >=20 > *BOOM* (corruption, panic, etc.) Grr, good catch. > This is the race that the current code handles by taking a reference > on the uid while it drops the uidinfo lock to acquire the table lock. >=20 > Probably you need to not drop the last reference unless you hold the=20 > uihashtbl_mtx, which means not using refcount_release() and manually use = an=20 > atomic_cmpset_int() if the refcount is > 1 to drop a ref to optimize the= =20 > common case: >=20 > old =3D uip->ui_refs; > if (old > 1) { > if (atomic_cmpset_int(&uip->ui_refs, old, old - 1)) > return; > } >=20 > mtx_lock(&uihashtbl_mtx); > if (refcount_release(&uip->ui_refs)) { > /* remove from table and free */ > } > mtx_unlock(&uihashtbl_mtx); How about we relookup it after acquiring the uihashtbl_mtx lock? Something like this (comments removed): uid_t uid; uid =3D uip->ui_uid; if (!refcount_release(&uip->ui_ref)) return; mtx_lock(&uihashtbl_mtx); uip =3D uilookup(uid); if (uip !=3D NULL && uip->ui_ref =3D=3D 0) { LIST_REMOVE(uip, ui_hash); mtx_unlock(&uihashtbl_mtx); FREE(uip, M_UIDINFO); return; } mtx_unlock(&uihashtbl_mtx); I updated the patch at: http://people.freebsd.org/~pjd/patches/uidinfo_waitfree.patch --=20 Pawel Jakub Dawidek http://www.wheel.pl pjd@FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! --PuGuTyElPB9bOcsM Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.4 (FreeBSD) iD8DBQFGy9lIForvXbEpPzQRAkyOAJoCwVBu1Fnr1tcYreempKrSrzJg+ACg2Vw9 T95TsOerhH6jnArM9tD3pno= =kw9m -----END PGP SIGNATURE----- --PuGuTyElPB9bOcsM-- From owner-freebsd-arch@FreeBSD.ORG Wed Aug 22 07:10:22 2007 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 244BD16A417 for ; Wed, 22 Aug 2007 07:10:22 +0000 (UTC) (envelope-from nobody@host.datatriangle.com) Received: from host.datatriangle.com (host.datatriangle.com [69.16.229.230]) by mx1.freebsd.org (Postfix) with ESMTP id CD4A113C461 for ; Wed, 22 Aug 2007 07:10:21 +0000 (UTC) (envelope-from nobody@host.datatriangle.com) Received: from nobody by host.datatriangle.com with local (Exim 4.63) (envelope-from ) id 1INimp-00007E-4T for freebsd-arch@freebsd.org; Wed, 22 Aug 2007 01:29:47 -0400 To: freebsd-arch@freebsd.org Date: Wed, 22 Aug 2007 01:29:47 -0400 From: Dr.Dawn-Elise_Snipes@AllCEUs.com Message-ID: <86fdc813ec69e352f076b62a1113f5d5@www.allceus.com> X-Priority: 3 X-Mailer: PHPMailer [version 1.73] X-Mailer: phplist v2.10.4 X-MessageID: 6 X-ListMember: freebsd-arch@freebsd.org Precedence: bulk MIME-Version: 1.0 X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - host.datatriangle.com X-AntiAbuse: Original Domain - freebsd.org X-AntiAbuse: Originator/Caller UID/GID - [99 99] / [47 12] X-AntiAbuse: Sender Address Domain - host.datatriangle.com Content-Type: text/plain; charset = "UTF-8" Content-Transfer-Encoding: 8bit X-Content-Filtered-By: Mailman/MimeDel 2.1.5 Subject: Unlimited CEUs for $49.99 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Aug 2007 07:10:22 -0000 Affordable, High-Quality CEUs! -- If you do not want to receive any more special offers or newsletters, http://allceus.com/mail_list/?p=unsubscribe&uid=16b19b1c2960302c31bfd3791bbb9939 To update your preferences and to unsubscribe visit http://allceus.com/mail_list/?p=preferences&uid=16b19b1c2960302c31bfd3791bbb9939 Forward a Message to Someone http://allceus.com/mail_list/?p=forward&uid=16b19b1c2960302c31bfd3791bbb9939&mid=6 Dr. Dawn-Elise Snipes dr.dawn-elise_snipes@allceus.com PO BOX 1688 Alachua, FL 32616 -- Powered by PHPlist, www.phplist.com -- From owner-freebsd-arch@FreeBSD.ORG Wed Aug 22 15:30:27 2007 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id C524716A420; Wed, 22 Aug 2007 15:30:27 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from speedfactory.net (mail6.speedfactory.net [66.23.216.219]) by mx1.freebsd.org (Postfix) with ESMTP id 5AA5313C4CE; Wed, 22 Aug 2007 15:30:26 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (unverified [66.23.211.162]) by speedfactory.net (SurgeMail 3.8k) with ESMTP id 204651572-1834499 for multiple; Wed, 22 Aug 2007 11:30:18 -0400 Received: from localhost.corp.yahoo.com (john@localhost [127.0.0.1]) (authenticated bits=0) by server.baldwin.cx (8.13.8/8.13.8) with ESMTP id l7MFUCFU009663; Wed, 22 Aug 2007 11:30:13 -0400 (EDT) (envelope-from jhb@freebsd.org) From: John Baldwin To: Pawel Jakub Dawidek Date: Wed, 22 Aug 2007 10:16:33 -0400 User-Agent: KMail/1.9.6 References: <20070818120056.GA6498@garage.freebsd.pl> <200708211753.34697.jhb@freebsd.org> <20070822063552.GC4187@garage.freebsd.pl> In-Reply-To: <20070822063552.GC4187@garage.freebsd.pl> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200708221016.34107.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [127.0.0.1]); Wed, 22 Aug 2007 11:30:13 -0400 (EDT) X-Virus-Scanned: ClamAV 0.88.3/4029/Wed Aug 22 09:37:05 2007 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=4.2 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx Cc: Alfred Perlstein , freebsd-arch@freebsd.org Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Aug 2007 15:30:28 -0000 On Wednesday 22 August 2007 02:35:52 am Pawel Jakub Dawidek wrote: > On Tue, Aug 21, 2007 at 05:53:34PM -0400, John Baldwin wrote: > > On Tuesday 21 August 2007 03:19:02 pm Pawel Jakub Dawidek wrote: > > > > Memory barriers on another CPU don't mean anything about the CPU thread 2 > > is > > > > on. Memory barriers do not flush caches on other CPUs, etc. Normally > > when > > > > objects are refcounted in a table, the table holds a reference on the > > object, > > > > but that doesn't seem to be the case here. [...] > > > > > > But the memory barrier from 'mtx_lock(&uihashtbl_mtx)' above > > > 'if (uip->ui_ref > 0)' would do the trick and I can safely avoid using > > > atomic read in this if statement, right? > > > > Yes, though that may be rather non-obvious to other people, or to yourself 6 > > months down the road. You should probably document it. > > Agreed, I added a comment that should explain it. > > > > > I wouldn't use a more complex algo in uifree() unless the simple one is > > shown > > > > to perform badly. Needless complexity is a hindrance to future > > maintenance. > > > > > > Of coure we could do that, but I was trying really hard to remove > > > contention in the common case. Before we used UIDINFO_LOCK() in the > > > common case, now you suggesting using global lock here, and I'd really, > > > really prefer using one atomic only. > > > > *sigh* You ignored my last sentence. You need to think about other people > > who come and read this later or yourself 12 months from now when you've paged > > out all the uidinfo stuff from your head. > > > > Hmm, what happens if you get this: > > > > thread A > > -------- > > > > refcount_release() - drops to 0 > > > > preempted > > > > thread B > > -------- > > uihold() - refcount goes to 1 > > ... > > uifree() - refcount goes to 0 > > removes object from table and frees it > > > > resumes > > > > mtx_lock(&uihashtbl); > > > > sees refcount of 0 so tries to destroy object again > > > > *BOOM* (corruption, panic, etc.) > > Grr, good catch. > > > This is the race that the current code handles by taking a reference > > on the uid while it drops the uidinfo lock to acquire the table lock. > > > > Probably you need to not drop the last reference unless you hold the > > uihashtbl_mtx, which means not using refcount_release() and manually use an > > atomic_cmpset_int() if the refcount is > 1 to drop a ref to optimize the > > common case: > > > > old = uip->ui_refs; > > if (old > 1) { > > if (atomic_cmpset_int(&uip->ui_refs, old, old - 1)) > > return; > > } > > > > mtx_lock(&uihashtbl_mtx); > > if (refcount_release(&uip->ui_refs)) { > > /* remove from table and free */ > > } > > mtx_unlock(&uihashtbl_mtx); > > How about we relookup it after acquiring the uihashtbl_mtx lock? > Something like this (comments removed): > > uid_t uid; > > uid = uip->ui_uid; > if (!refcount_release(&uip->ui_ref)) > return; > mtx_lock(&uihashtbl_mtx); > uip = uilookup(uid); > if (uip != NULL && uip->ui_ref == 0) { > LIST_REMOVE(uip, ui_hash); > mtx_unlock(&uihashtbl_mtx); > FREE(uip, M_UIDINFO); > return; > } > mtx_unlock(&uihashtbl_mtx); > > I updated the patch at: > > http://people.freebsd.org/~pjd/patches/uidinfo_waitfree.patch That actually adds more overhead than what I suggested above to the case where you are going to free it. Also, I'm leery of having an object hang around with a zero ref count while it is in the table with the lock not held. -- John Baldwin From owner-freebsd-arch@FreeBSD.ORG Wed Aug 22 19:31:55 2007 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 7D38416A417 for ; Wed, 22 Aug 2007 19:31:55 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: from nf-out-0910.google.com (nf-out-0910.google.com [64.233.182.190]) by mx1.freebsd.org (Postfix) with ESMTP id EAF1313C48E for ; Wed, 22 Aug 2007 19:31:54 +0000 (UTC) (envelope-from asmrookie@gmail.com) Received: by nf-out-0910.google.com with SMTP id b2so240748nfb for ; Wed, 22 Aug 2007 12:31:53 -0700 (PDT) DKIM-Signature: a=rsa-sha1; c=relaxed/relaxed; d=gmail.com; s=beta; h=domainkey-signature:received:received:message-id:date:from:sender:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references:x-google-sender-auth; b=N2N+/FpMRuwVW62cMEdnWXT9hc8jgCu1aPsBHkdd9SV97yXoYMGbOxtTO5iCKas5PfuJZrZoNHDFeZt06qrxTMMV322gN6s+HlvG3gmpyCDLc338HJczR58Yis2k9GKvqHfkEg/b82dAmnkD92tKMCvGjSvnFPKqdkP7A+cFlh8= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=received:message-id:date:from:sender:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references:x-google-sender-auth; b=adeq/78YsS6doBrcTqFCBbAyOKxpaFamwUClxRe+eIGdoCK0zrwixOqFT5yELuxRnHOQB6gIPgdnBVnaftaLJmZgmYdtZoQdHMCdgyJueVuhau9IHOrwJAsrvVt8UTFRItS8OwgJAczvug4IOzm9Sfe18MEOvqyglx1/yc0dPDc= Received: by 10.78.172.20 with SMTP id u20mr679550hue.1187809374087; Wed, 22 Aug 2007 12:02:54 -0700 (PDT) Received: by 10.78.97.18 with HTTP; Wed, 22 Aug 2007 12:02:53 -0700 (PDT) Message-ID: <3bbf2fe10708221202h44b3258cyf5ca5e9b867ac0e7@mail.gmail.com> Date: Wed, 22 Aug 2007 21:02:53 +0200 From: "Attilio Rao" Sender: asmrookie@gmail.com To: "Pawel Jakub Dawidek" In-Reply-To: <20070821202136.GB4187@garage.freebsd.pl> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Content-Disposition: inline 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> X-Google-Sender-Auth: 142814e6a6dc283f Cc: Alfred Perlstein , freebsd-arch@freebsd.org Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Aug 2007 19:31:55 -0000 2007/8/21, Pawel Jakub Dawidek : > > 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(); } Thanks, Attilio -- Peace can only be achieved by understanding - A. Einstein From owner-freebsd-arch@FreeBSD.ORG Thu Aug 23 09:49:39 2007 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 93BF816A41A; Thu, 23 Aug 2007 09:49:39 +0000 (UTC) (envelope-from pjd@garage.freebsd.pl) Received: from mail.garage.freebsd.pl (arm132.internetdsl.tpnet.pl [83.17.198.132]) by mx1.freebsd.org (Postfix) with ESMTP id 2545C13C4D9; Thu, 23 Aug 2007 09:49:36 +0000 (UTC) (envelope-from pjd@garage.freebsd.pl) Received: by mail.garage.freebsd.pl (Postfix, from userid 65534) id 11CFC4569A; Thu, 23 Aug 2007 11:49:33 +0200 (CEST) Received: from localhost (pjd.wheel.pl [10.0.1.1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.garage.freebsd.pl (Postfix) with ESMTP id C9BBF45685; Thu, 23 Aug 2007 11:49:24 +0200 (CEST) Date: Thu, 23 Aug 2007 11:48:25 +0200 From: Pawel Jakub Dawidek To: John Baldwin Message-ID: <20070823094825.GA33164@garage.freebsd.pl> References: <20070818120056.GA6498@garage.freebsd.pl> <200708211753.34697.jhb@freebsd.org> <20070822063552.GC4187@garage.freebsd.pl> <200708221016.34107.jhb@freebsd.org> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="zYM0uCDKw75PZbzx" Content-Disposition: inline In-Reply-To: <200708221016.34107.jhb@freebsd.org> User-Agent: Mutt/1.4.2.3i X-PGP-Key-URL: http://people.freebsd.org/~pjd/pjd.asc X-OS: FreeBSD 7.0-CURRENT i386 X-Spam-Checker-Version: SpamAssassin 3.0.4 (2005-06-05) on mail.garage.freebsd.pl X-Spam-Level: X-Spam-Status: No, score=-5.9 required=3.0 tests=ALL_TRUSTED,BAYES_00 autolearn=ham version=3.0.4 Cc: Alfred Perlstein , freebsd-arch@freebsd.org Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 23 Aug 2007 09:49:39 -0000 --zYM0uCDKw75PZbzx Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Aug 22, 2007 at 10:16:33AM -0400, John Baldwin wrote: > On Wednesday 22 August 2007 02:35:52 am Pawel Jakub Dawidek wrote: > > I updated the patch at: > >=20 > > http://people.freebsd.org/~pjd/patches/uidinfo_waitfree.patch >=20 > That actually adds more overhead than what I suggested above to the case = where=20 > you are going to free it. Also, I'm leery of having an object hang aroun= d=20 > with a zero ref count while it is in the table with the lock not held. I just felt it's easier to understand when we don't bypass the refcount KPI, but I'm fine with your method. --=20 Pawel Jakub Dawidek http://www.wheel.pl pjd@FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! --zYM0uCDKw75PZbzx Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.4 (FreeBSD) iD8DBQFGzVfpForvXbEpPzQRAmHLAJwNFHrgpEeTthvEQtdxstctnABe4ACeJ0Tq OdM+BbIx/kBpJWaRMj4+gf4= =GBsH -----END PGP SIGNATURE----- --zYM0uCDKw75PZbzx-- From owner-freebsd-arch@FreeBSD.ORG Fri Aug 24 14:10:38 2007 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E25F716A417; Fri, 24 Aug 2007 14:10:38 +0000 (UTC) (envelope-from pjd@garage.freebsd.pl) Received: from mail.garage.freebsd.pl (arm132.internetdsl.tpnet.pl [83.17.198.132]) by mx1.freebsd.org (Postfix) with ESMTP id 36CF713C442; Fri, 24 Aug 2007 14:10:38 +0000 (UTC) (envelope-from pjd@garage.freebsd.pl) Received: by mail.garage.freebsd.pl (Postfix, from userid 65534) id D5CC54880A; Fri, 24 Aug 2007 16:10:36 +0200 (CEST) Received: from localhost (pjd.wheel.pl [10.0.1.1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.garage.freebsd.pl (Postfix) with ESMTP id CF8B3487F8; Fri, 24 Aug 2007 16:10:28 +0200 (CEST) Date: Fri, 24 Aug 2007 16:09:27 +0200 From: Pawel Jakub Dawidek To: Attilio Rao Message-ID: <20070824140927.GC14536@garage.freebsd.pl> 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> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="NU0Ex4SbNnrxsi6C" Content-Disposition: inline In-Reply-To: <3bbf2fe10708221202h44b3258cyf5ca5e9b867ac0e7@mail.gmail.com> User-Agent: Mutt/1.4.2.3i X-PGP-Key-URL: http://people.freebsd.org/~pjd/pjd.asc X-OS: FreeBSD 7.0-CURRENT i386 X-Spam-Checker-Version: SpamAssassin 3.0.4 (2005-06-05) on mail.garage.freebsd.pl X-Spam-Level: X-Spam-Status: No, score=-5.9 required=3.0 tests=ALL_TRUSTED,BAYES_00 autolearn=ham version=3.0.4 Cc: Alfred Perlstein , freebsd-arch@freebsd.org Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 24 Aug 2007 14:10:39 -0000 --NU0Ex4SbNnrxsi6C Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Aug 22, 2007 at 09:02:53PM +0200, Attilio Rao wrote: > 2007/8/21, Pawel Jakub Dawidek : > > > > New patch is here: > > > > http://people.freebsd.org/~pjd/patches/uidinfo_waitfree.patch >=20 > --- sys/ia64/include/atomic.h.orig > +++ sys/ia64/include/atomic.h > @@ -370,4 +370,15 @@ >=20 > #define atomic_fetchadd_int atomic_fetchadd_32 >=20 > +static __inline u_long > +atomic_fetchadd_long(volatile u_long *p, u_long v) > +{ > + u_long value; > + > + do { > + value =3D *p; > + } while (!atomic_cmpset_64(p, value, value + v)); > + return (value); > +} > + >=20 > In cycles like those, as you get spinning, I would arrange things in > order to do a cpu_spinwait(). Like this: >=20 > for (;;) { > value =3D *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. --=20 Pawel Jakub Dawidek http://www.wheel.pl pjd@FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! --NU0Ex4SbNnrxsi6C Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.4 (FreeBSD) iD8DBQFGzuaXForvXbEpPzQRArq0AKC5/SuL8uxctj5tvAnblJ+nI0qN7wCePxwB r8unBjbr2PRLARTXynf0gEE= =937F -----END PGP SIGNATURE----- --NU0Ex4SbNnrxsi6C-- From owner-freebsd-arch@FreeBSD.ORG Fri Aug 24 14:29:52 2007 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 758) id 8B5C616A46B; Fri, 24 Aug 2007 14:29:52 +0000 (UTC) Date: Fri, 24 Aug 2007 14:29:52 +0000 From: Kris Kennaway To: Pawel Jakub Dawidek Message-ID: <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> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20070824140927.GC14536@garage.freebsd.pl> User-Agent: Mutt/1.4.2.1i Cc: Attilio Rao , Alfred Perlstein , freebsd-arch@freebsd.org Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 24 Aug 2007 14:29:52 -0000 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 : > > > > > > 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. Better to still do it in case that changes. Kris From owner-freebsd-arch@FreeBSD.ORG Sat Aug 25 22:20:45 2007 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 27F4416A417; Sat, 25 Aug 2007 22:20:45 +0000 (UTC) (envelope-from jroberson@chesapeake.net) Received: from webaccess-cl.virtdom.com (webaccess-cl.virtdom.com [216.240.101.25]) by mx1.freebsd.org (Postfix) with ESMTP id C9C7413C461; Sat, 25 Aug 2007 22:20:44 +0000 (UTC) (envelope-from jroberson@chesapeake.net) Received: from [192.168.1.103] (c-67-160-44-208.hsd1.wa.comcast.net [67.160.44.208]) (authenticated bits=0) by webaccess-cl.virtdom.com (8.13.6/8.13.6) with ESMTP id l7PMKenJ000627 (version=TLSv1/SSLv3 cipher=DHE-DSS-AES256-SHA bits=256 verify=NO); Sat, 25 Aug 2007 18:20:41 -0400 (EDT) (envelope-from jroberson@chesapeake.net) Date: Sat, 25 Aug 2007 15:23:10 -0700 (PDT) From: Jeff Roberson X-X-Sender: jroberson@10.0.0.1 To: Kris Kennaway In-Reply-To: <20070824142952.GA24469@hub.freebsd.org> Message-ID: <20070825151913.S568@10.0.0.1> 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> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: Attilio Rao , Alfred Perlstein , Pawel Jakub Dawidek , freebsd-arch@freebsd.org Subject: Re: Lockless uidinfo. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 25 Aug 2007 22:20:45 -0000 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 : >>>> >>>> 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" >