From owner-freebsd-fs@FreeBSD.ORG Thu Dec 22 21:47:29 2011 Return-Path: Delivered-To: freebsd-fs@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id DFAD2106564A; Thu, 22 Dec 2011 21:47:29 +0000 (UTC) (envelope-from ed@hoeg.nl) Received: from mx0.hoeg.nl (mx0.hoeg.nl [IPv6:2a01:4f8:101:5343::aa]) by mx1.freebsd.org (Postfix) with ESMTP id 2A4A18FC17; Thu, 22 Dec 2011 21:47:29 +0000 (UTC) Received: by mx0.hoeg.nl (Postfix, from userid 1000) id 658472A298B6; Thu, 22 Dec 2011 22:47:28 +0100 (CET) Date: Thu, 22 Dec 2011 22:47:28 +0100 From: Ed Schouten To: pjd@FreeBSD.org, freebsd-fs@FreeBSD.org Message-ID: <20111222214728.GV1771@hoeg.nl> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="H23uHpCUqgUcHMpK" Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Subject: Changing refcount(9) to use a refcount_t X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Dec 2011 21:47:30 -0000 --H23uHpCUqgUcHMpK Content-Type: multipart/mixed; boundary="rjqqsQBSnnHPMzvu" Content-Disposition: inline --rjqqsQBSnnHPMzvu Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi all, As some of you may know, the upcoming C standard has support for atomic operations. Looking at how they implemented it, it seems that they tried to keep it somehow compatible with existing compiler standards. For example, one could implement a poor-man's version of it as follows: | #define _Atomic(T) struct { pthread_mutex_t m; T v; } | | #define ATOMIC_VAR_INIT(value) { PTHREAD_MUTEX_INITIALIZER, (value) } |=20 | #define atomic_store(object, value) do { | pthread_mutex_lock(&(object)->m); | (object)->v =3D (value); | pthread_mutex_unlock(&(object)->m); | } while(0) | | ... Voila; atomics! Just out of curiosity, I did some experiments with this, where I have a that works with both Clang and GCC (except ARM and MIPS, for some reason). My first test subject: . It seems that porting to use the ISO C1X atomic operations is a bit hard, as the refcount_* functions operate on volatile u_ints instead of some opaque object. Looking ahead, I think it would be a good idea to already add a typedef from volatile uint to refcount_t and already merge that back to 9. That way it won't matter how we actually implement by the time C1X is finalised. The reason why I'm emailing this to fs@, is because this change breaks one of the existing file system drivers, namely ZFS. Solaris also implements a refcount_t, but unlike FreeBSD's, it has a more complex API and is 64-bits in size. Still, I suspect it's hard to overflow a 32-bit reference counter, right? Even if it is, we can fix this in the long run by making refcount_t a truly opaque object of type u_long. Can any of you ZFS user please try the following patch? Do any of you object if I commit it to SVN and merge it in a couple of months from now? Thanks, --=20 Ed Schouten WWW: http://80386.nl/ --rjqqsQBSnnHPMzvu Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="refcount.diff" Content-Transfer-Encoding: quoted-printable Index: share/man/man9/refcount.9 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- share/man/man9/refcount.9 (revision 228798) +++ share/man/man9/refcount.9 (working copy) @@ -39,11 +39,11 @@ .In sys/param.h .In sys/refcount.h .Ft void -.Fn refcount_init "volatile u_int *count, u_int value" +.Fn refcount_init "refcount_t *count, u_int value" .Ft void -.Fn refcount_acquire "volatile u_int *count" +.Fn refcount_acquire "refcount_t *count" .Ft int -.Fn refcount_release "volatile u_int *count" +.Fn refcount_release "refcount_t *count" .Sh DESCRIPTION The .Nm Index: sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/refcount.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/refcount.h (revision= 228798) +++ sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/refcount.h (working = copy) @@ -31,10 +31,6 @@ #include #include =20 -#ifdef __cplusplus -extern "C" { -#endif - /* * If the reference is held only by the calling function and not any * particular object, use FTAG (which is a string) for the holder_tag. @@ -42,68 +38,21 @@ */ #define FTAG ((char *)__func__) =20 -#ifdef ZFS_DEBUG -typedef struct reference { - list_node_t ref_link; - void *ref_holder; - uint64_t ref_number; - uint8_t *ref_removed; -} reference_t; - -typedef struct refcount { - kmutex_t rc_mtx; - list_t rc_list; - list_t rc_removed; - int64_t rc_count; - int64_t rc_removed_count; -} refcount_t; - -/* Note: refcount_t must be initialized with refcount_create() */ - -void refcount_create(refcount_t *rc); -void refcount_destroy(refcount_t *rc); -void refcount_destroy_many(refcount_t *rc, uint64_t number); -int refcount_is_zero(refcount_t *rc); -int64_t refcount_count(refcount_t *rc); -int64_t refcount_add(refcount_t *rc, void *holder_tag); -int64_t refcount_remove(refcount_t *rc, void *holder_tag); -int64_t refcount_add_many(refcount_t *rc, uint64_t number, void *holder_ta= g); -int64_t refcount_remove_many(refcount_t *rc, uint64_t number, void *holder= _tag); -void refcount_transfer(refcount_t *dst, refcount_t *src); - -void refcount_sysinit(void); -void refcount_fini(void); - -#else /* ZFS_DEBUG */ - -typedef struct refcount { - uint64_t rc_count; -} refcount_t; - -#define refcount_create(rc) ((rc)->rc_count =3D 0) -#define refcount_destroy(rc) ((rc)->rc_count =3D 0) -#define refcount_destroy_many(rc, number) ((rc)->rc_count =3D 0) -#define refcount_is_zero(rc) ((rc)->rc_count =3D=3D 0) -#define refcount_count(rc) ((rc)->rc_count) -#define refcount_add(rc, holder) atomic_add_64_nv(&(rc)->rc_count, 1) -#define refcount_remove(rc, holder) atomic_add_64_nv(&(rc)->rc_count, -1) +#define refcount_create(rc) refcount_init(rc, 0) +#define refcount_destroy(rc) refcount_init(rc, 0) +#define refcount_destroy_many(rc, number) refcount_init(rc, 0) +#define refcount_is_zero(rc) ((*rc) =3D=3D 0) +#define refcount_count(rc) (uint64_t)(*rc) +#define refcount_add(rc, holder) refcount_add_many(rc, 1, holder) +#define refcount_remove(rc, holder) refcount_remove_many(rc, 1, holder) #define refcount_add_many(rc, number, holder) \ - atomic_add_64_nv(&(rc)->rc_count, number) + (uint64_t)(atomic_fetchadd_int(rc, number) + (number)) #define refcount_remove_many(rc, number, holder) \ - atomic_add_64_nv(&(rc)->rc_count, -number) -#define refcount_transfer(dst, src) { \ - uint64_t __tmp =3D (src)->rc_count; \ - atomic_add_64(&(src)->rc_count, -__tmp); \ - atomic_add_64(&(dst)->rc_count, __tmp); \ -} + (uint64_t)(atomic_fetchadd_int(rc, -(number)) - (number)) +#define refcount_transfer(dst, src) \ + atomic_add_int(dst, atomic_readandclear_int(src)) =20 #define refcount_sysinit() #define refcount_fini() =20 -#endif /* ZFS_DEBUG */ - -#ifdef __cplusplus -} -#endif - #endif /* _SYS_REFCOUNT_H */ Index: sys/sys/refcount.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- sys/sys/refcount.h (revision 228798) +++ sys/sys/refcount.h (working copy) @@ -40,22 +40,24 @@ #define KASSERT(exp, msg) /* */ #endif =20 +typedef volatile u_int refcount_t; + static __inline void -refcount_init(volatile u_int *count, u_int value) +refcount_init(refcount_t *count, u_int value) { =20 *count =3D value; } =20 static __inline void -refcount_acquire(volatile u_int *count) +refcount_acquire(refcount_t *count) { =20 atomic_add_acq_int(count, 1);=09 } =20 static __inline int -refcount_release(volatile u_int *count) +refcount_release(refcount_t *count) { u_int old; =20 --rjqqsQBSnnHPMzvu-- --H23uHpCUqgUcHMpK Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (FreeBSD) iQIcBAEBAgAGBQJO86VwAAoJEG5e2P40kaK7F5MP/1Ott3zXSvYyRGKpn0t/SJ8w k8fYEkwA2DHjfPBwt3TGjAK5TK4OIycHfncIfLFSU0c/geDodnWk10ppsESTguF5 KO8dyw+X8ocfITeCNJ7ngoicdUGOeyvi1m9rMfogb2SX5tt+/p+i46x19ogAogj+ hnqKtT9PFolw7Up6nT2gSeWRjdiVJm+7rGmY2Ult3YXf9vm4b0lwGmcqxvPph5jr n6C9dZBmnsy0x9ULfj3z46ec+5rJnokJSSZQPWBpKi9m3McWSGSIBieWnkANqTTt rJpzdy5lihwNYf0xVWpiMwHeNEt2YNkto1xS07Mc+G87goCqo2rwyNaHmkq7Tde6 QMrNWFDDPumFO/JuTEdO0C2P2DVqZ1AA+fa0MBqehaAZur0SUnsxqIOoHdsonZi+ WlQjnmWkvJeqDsUY6PisVocNSwmTIEp1c+/xHvnLHyQ5ddPgg/hbMFkZgYlJDcbl TiHxEUZ6IK2R1ir8jL/AaU57upbKBNM566XOuE9Tp+YLyGdpL7Orwyt8Wt3+PZk4 neMUEFfNIR/bCd3+IggHuTglH3DHX6zdIJ7ksHDPlYkmjkyZEbY2EzmCVfUrxU8O mfV5RSfApXbxfDlUunnsQmzYMnQct2gf2osVQ1u92wFiYv/upFWiAROrFw/4nkW5 AM4xGJwMde74jHRzzz+t =8Fi0 -----END PGP SIGNATURE----- --H23uHpCUqgUcHMpK--