From owner-freebsd-arch@freebsd.org Thu Jul 2 21:30:56 2015 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 32F3E993EE0 for ; Thu, 2 Jul 2015 21:30:56 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-wi0-x231.google.com (mail-wi0-x231.google.com [IPv6:2a00:1450:400c:c05::231]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id BA2591551; Thu, 2 Jul 2015 21:30:55 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by wiwl6 with SMTP id l6so209279946wiw.0; Thu, 02 Jul 2015 14:30:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:subject:message-id:references:mime-version :content-type:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=JXGOsap2xwfIkf98y7afGeCMc6p1ONug0S/nLMWCi50=; b=dqbp1139VhKhFy+hl+1KWeC72SLMmEgP4+yOAWrIjuBGmem7H+UIVvYSrvo67DUTd+ hpP7QtFf/EmKZYCNbfWVtx6zg1JqqegkYcATMehWYScNPZPII1e2e2b26xzef8YswwZS h73AQdFR/3AXMgl1+0WoFI6Ik9s+Am+WpD67V6ewiu4bxNjlRnAsajjTHlBZ+ThdhMBH 6z3Zyn7kZ6aYWIx5ZrDxpl7DIlLA+Tpnw5dmvd9Arr1dTBIjOSv07n6qoIMNWO4UHAor L8l+HVjMyWSw55dnM7ki/kaOzYjuNUPil+MyuXWhlHoS6IHMj/4ZzPJBRb7UkHhhEe2u yiEg== X-Received: by 10.194.8.40 with SMTP id o8mr37238119wja.100.1435872653910; Thu, 02 Jul 2015 14:30:53 -0700 (PDT) Received: from dft-labs.eu (n1x0n-1-pt.tunnel.tserv5.lon1.ipv6.he.net. [2001:470:1f08:1f7::2]) by mx.google.com with ESMTPSA id m4sm9963118wjb.37.2015.07.02.14.30.51 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Thu, 02 Jul 2015 14:30:51 -0700 (PDT) Date: Thu, 2 Jul 2015 23:30:49 +0200 From: Mateusz Guzik To: Bruce Evans , freebsd-arch@freebsd.org, Konstantin Belousov , jhb@freebsd.org Subject: new api for reference counters (was: atomic v_usecount and v_holdcnt) Message-ID: <20150702213048.GB7666@dft-labs.eu> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20150625195352.GB1042@brick.home> User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Jul 2015 21:30:56 -0000 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