Date: Thu, 2 Jul 2015 23:30:49 +0200 From: Mateusz Guzik <mjguzik@gmail.com> To: Bruce Evans <brde@optusnet.com.au>, freebsd-arch@freebsd.org, Konstantin Belousov <kostikbel@gmail.com>, jhb@freebsd.org Subject: new api for reference counters (was: atomic v_usecount and v_holdcnt) Message-ID: <20150702213048.GB7666@dft-labs.eu> In-Reply-To: <20150625195352.GB1042@brick.home> References: <20141122092527.GT17068@kib.kiev.ua> <20141122211147.GA23623@dft-labs.eu> <20141124095251.GH17068@kib.kiev.ua> <20150314225226.GA15302@dft-labs.eu> <20150316094643.GZ2379@kib.kiev.ua> <20150317014412.GA10819@dft-labs.eu> <20150318104442.GS2379@kib.kiev.ua> <20150625123156.GA29667@dft-labs.eu> <20150626042546.Q2820@besplex.bde.org> <20150625195352.GB1042@brick.home>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Jun 25, 2015 at 09:53:52PM +0200, Edward Tomasz Napierała wrote:
> On 0626T0429, Bruce Evans wrote:
> > On Thu, 25 Jun 2015, Mateusz Guzik wrote:
> >
> > > On Wed, Mar 18, 2015 at 12:44:42PM +0200, Konstantin Belousov wrote:
> > >> On Tue, Mar 17, 2015 at 02:44:12AM +0100, Mateusz Guzik wrote:
> >
> > >>> I replaced them with refcount_acquire_if_not_zero and
> > >>> refcount_release_if_not_last.
> > >> I dislike the length of the names. Can you propose something shorter ?
> > >
> > > Unfortunately the original API is alreday quite verbose and I don't have
> > > anything readable which would retain "refcount_acquire" (instead of a
> > > "ref_get" or "ref_acq"). Adding "_nz" as a suffix does not look good
> > > ("refcount_acquire_if_nz").
> >
> > refcount -> rc
> > acquire -> acq
> >
> > The "acq" abbreviation is already used a lot for atomic ops.
>
> How about refcount_acquire_gt_0() and refcount_release_gt_1()1?
>
Current refcount(9) api is not only quite limited, but it also operates
on u_int instead of a type opaque to its consumers. This gives fewer
places which can assert counter sanity and increases chances of an
accidental abuse/type mismatch.
As such, how about deprecating current refcount_* funcs and introducing
a new family instead.
Say funcs would be prefixed with ref_. Consumers would use a ref_t type
(the obvious refcount_t is stolen by zfs).
Apart from aforementioned 0->1 and 1->0 funcs, this introduces ref_read
to obtain the state of passed counter. A cast to volatile pointer +
dereference of that guarantees us a read at that point, so we don't have
to put the qualifier in the struct.
Finally, a blessed type is provided for use by all consumers so that the
correct type is enforced at compile time.
Roughly speaking:
diff --git a/sys/sys/refcount.h b/sys/sys/refcount.h
index 4611664..c7c3732 100644
--- a/sys/sys/refcount.h
+++ b/sys/sys/refcount.h
@@ -38,6 +38,11 @@
#define KASSERT(exp, msg) /* */
#endif
+struct ref {
+ u_int r_count;
+};
+typedef struct ref ref_t;
+
static __inline void
refcount_init(volatile u_int *count, u_int value)
{
@@ -64,4 +69,64 @@ refcount_release(volatile u_int *count)
return (old == 1);
}
+static __inline void
+ref_init(ref_t *ref, u_int value)
+{
+
+ refcount_init(&ref->r_count, value);
+}
+
+static __inline u_int
+ref_read(ref_t *ref)
+{
+ u_int count;
+
+ count = *(volatile u_int *)&(ref->r_count);
+ KASSERT(count < UINT_MAX, ("refcount %p overflowed", ref));
+
+ return (count);
+}
+
+static __inline void
+ref_acq(ref_t *ref)
+{
+
+ refcount_acquire(&ref->r_count);
+}
+
+static __inline int
+ref_rel(ref_t *ref)
+{
+
+ return (refcount_release(&ref->r_count));
+}
+
+static __inline int
+ref_acq_gt_0(ref_t *ref)
+{
+ u_int old;
+
+ for (;;) {
+ old = ref_read(ref);
+ if (old == 0)
+ return (0);
+ if (atomic_cmpset_int(&ref->r_count, old, old + 1))
+ return (1);
+ }
+}
+
+static __inline int
+ref_rel_gt_1(ref_t *ref)
+{
+ u_int old;
+
+ for (;;) {
+ old = ref_read(ref);
+ if (old == 1)
+ return (0);
+ if (atomic_cmpset_int(&ref->r_count, old, old - 1))
+ return (1);
+ }
+}
+
#endif /* ! __SYS_REFCOUNT_H__ */
--
Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150702213048.GB7666>