Date: Thu, 22 Dec 2011 22:47:28 +0100 From: Ed Schouten <ed@80386.nl> To: pjd@FreeBSD.org, freebsd-fs@FreeBSD.org Subject: Changing refcount(9) to use a refcount_t Message-ID: <20111222214728.GV1771@hoeg.nl>
next in thread | raw e-mail | index | archive | help
--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
<stdatomic.h> that works with both Clang and GCC (except ARM and MIPS,
for some reason). My first test subject: <sys/refcount.h>.
It seems that porting <sys/refcount.h> 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 <sys/refcount.h> 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 <ed@80386.nl>
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 <sys/list.h>
#include <sys/zfs_context.h>
=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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111222214728.GV1771>