From owner-freebsd-arch@FreeBSD.ORG Sat Oct 25 18:44:54 2014 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 0C0F7412 for ; Sat, 25 Oct 2014 18:44:54 +0000 (UTC) Received: from mail-wg0-x229.google.com (mail-wg0-x229.google.com [IPv6:2a00:1450:400c:c00::229]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 9133EA14 for ; Sat, 25 Oct 2014 18:44:53 +0000 (UTC) Received: by mail-wg0-f41.google.com with SMTP id b13so3121507wgh.24 for ; Sat, 25 Oct 2014 11:44:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:subject:message-id:mime-version:content-type :content-disposition:user-agent; bh=IcJA+YTYxPUo+6RkvNm0+aWEVSd+1ylUVGPVDPSEl1w=; b=Lq2Wo7ZT6/ZlBocvRvvwrw18mwkfR92XyEez/m+ZqZtRHdCMbN2SQ7fdns5yakBbmr 86EvBNqwnspsPda/ByJUXka4rSBVqYgzhgqTrC1z7+Bp+mCSYi/YaDyehGuPl9zbW2A7 vj5a9SNt9wVIpeU2iI5lWiI6rxJ8ZH1bQTaGHTRQbSYY6GXhL+4pnrvjaJ7X6P9HxU/W kdqubfvqbKYsbJpAutfUYpF4V2W1YldAn6yNsRe2JetIpXbS5cV09++QKwbguWH/mOQi UH9N1XiKagFsugwloNN+j5yOpYa58UGpjbG5gsjA9Q+lxaBB3aJhMKeQfNrrn58tubw4 IS8w== X-Received: by 10.194.172.234 with SMTP id bf10mr12818503wjc.81.1414262691846; Sat, 25 Oct 2014 11:44:51 -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 pc8sm9526535wjb.36.2014.10.25.11.44.50 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Sat, 25 Oct 2014 11:44:51 -0700 (PDT) Date: Sat, 25 Oct 2014 20:44:48 +0200 From: Mateusz Guzik To: freebsd-arch@freebsd.org Subject: refcount_release_take_##lock Message-ID: <20141025184448.GA19066@dft-labs.eu> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.18-1 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 Oct 2014 18:44:54 -0000 The following idiom is used here and there: int old; old = obj->ref; if (old > 1 && atomic_cmpset_int(&obj->ref, old, old -1)) return; lock(&something); if (refcount_release(&obj->ref) == 0) { unlock(&something); return; } free up unlock(&something); ========== I decided to implement it as a common function. We have only refcount.h and I didn't want to bloat all including code with additional definitions and as such I came up with a macro that has to be used in .c file and that will define appropriate inline func. I'm definitely looking for better names for REFCOUNT_RELEASE_TAKE_USE_ macro, assuming it has to stay. Comments? diff --git a/sys/kern/kern_jail.c b/sys/kern/kern_jail.c index f8ae0e6..cf16344 100644 --- a/sys/kern/kern_jail.c +++ b/sys/kern/kern_jail.c @@ -81,6 +81,8 @@ __FBSDID("$FreeBSD$"); MALLOC_DEFINE(M_PRISON, "prison", "Prison structures"); static MALLOC_DEFINE(M_PRISON_RACCT, "prison_racct", "Prison racct structures"); +REFCOUNT_RELEASE_TAKE_USE_SX; + /* Keep struct prison prison0 and some code in kern_jail_set() readable. */ #ifdef INET #ifdef INET6 @@ -4466,15 +4468,12 @@ prison_racct_free_locked(struct prison_racct *prr) void prison_racct_free(struct prison_racct *prr) { - int old; sx_assert(&allprison_lock, SA_UNLOCKED); - old = prr->prr_refcount; - if (old > 1 && atomic_cmpset_int(&prr->prr_refcount, old, old - 1)) + if (refcount_release_take_sx(&prr->prr_refcount, &allprison_lock) == 0) return; - sx_xlock(&allprison_lock); prison_racct_free_locked(prr); sx_xunlock(&allprison_lock); } diff --git a/sys/kern/kern_loginclass.c b/sys/kern/kern_loginclass.c index b20f60b..8cc4b40 100644 --- a/sys/kern/kern_loginclass.c +++ b/sys/kern/kern_loginclass.c @@ -70,6 +70,7 @@ LIST_HEAD(, loginclass) loginclasses; */ static struct mtx loginclasses_lock; MTX_SYSINIT(loginclasses_init, &loginclasses_lock, "loginclasses lock", MTX_DEF); +REFCOUNT_RELEASE_TAKE_USE_MTX; void loginclass_hold(struct loginclass *lc) @@ -81,22 +82,14 @@ loginclass_hold(struct loginclass *lc) void loginclass_free(struct loginclass *lc) { - int old; - old = lc->lc_refcount; - if (old > 1 && atomic_cmpset_int(&lc->lc_refcount, old, old - 1)) + if (refcount_release_take_mtx(&lc->lc_refcount, &loginclasses_lock) == 0) return; - mtx_lock(&loginclasses_lock); - if (refcount_release(&lc->lc_refcount)) { - racct_destroy(&lc->lc_racct); - LIST_REMOVE(lc, lc_next); - mtx_unlock(&loginclasses_lock); - free(lc, M_LOGINCLASS); - - return; - } + racct_destroy(&lc->lc_racct); + LIST_REMOVE(lc, lc_next); mtx_unlock(&loginclasses_lock); + free(lc, M_LOGINCLASS); } /* diff --git a/sys/kern/kern_resource.c b/sys/kern/kern_resource.c index f4914fa..efbaa87 100644 --- a/sys/kern/kern_resource.c +++ b/sys/kern/kern_resource.c @@ -71,6 +71,7 @@ static MALLOC_DEFINE(M_PLIMIT, "plimit", "plimit structures"); static MALLOC_DEFINE(M_UIDINFO, "uidinfo", "uidinfo structures"); #define UIHASH(uid) (&uihashtbl[(uid) & uihash]) static struct rwlock uihashtbl_lock; +REFCOUNT_RELEASE_TAKE_USE_RWLOCK; static LIST_HEAD(uihashhead, uidinfo) *uihashtbl; static u_long uihash; /* size of hash table - 1 */ @@ -1327,37 +1328,24 @@ void uifree(uip) struct uidinfo *uip; { - int old; - /* Prepare for optimal case. */ - old = uip->ui_ref; - if (old > 1 && atomic_cmpset_int(&uip->ui_ref, old, old - 1)) + if (refcount_release_take_rwlock(&uip->ui_ref, &uihashtbl_lock) == 0) return; - /* Prepare for suboptimal case. */ - rw_wlock(&uihashtbl_lock); - if (refcount_release(&uip->ui_ref)) { - racct_destroy(&uip->ui_racct); - LIST_REMOVE(uip, ui_hash); - rw_wunlock(&uihashtbl_lock); - if (uip->ui_sbsize != 0) - printf("freeing uidinfo: uid = %d, sbsize = %ld\n", - uip->ui_uid, uip->ui_sbsize); - if (uip->ui_proccnt != 0) - printf("freeing uidinfo: uid = %d, proccnt = %ld\n", - uip->ui_uid, uip->ui_proccnt); - if (uip->ui_vmsize != 0) - printf("freeing uidinfo: uid = %d, swapuse = %lld\n", - uip->ui_uid, (unsigned long long)uip->ui_vmsize); - mtx_destroy(&uip->ui_vmsize_mtx); - free(uip, M_UIDINFO); - return; - } - /* - * Someone added a reference between atomic_cmpset_int() and - * rw_wlock(&uihashtbl_lock). - */ + racct_destroy(&uip->ui_racct); + LIST_REMOVE(uip, ui_hash); rw_wunlock(&uihashtbl_lock); + if (uip->ui_sbsize != 0) + printf("freeing uidinfo: uid = %d, sbsize = %ld\n", + uip->ui_uid, uip->ui_sbsize); + if (uip->ui_proccnt != 0) + printf("freeing uidinfo: uid = %d, proccnt = %ld\n", + uip->ui_uid, uip->ui_proccnt); + if (uip->ui_vmsize != 0) + printf("freeing uidinfo: uid = %d, swapuse = %lld\n", + uip->ui_uid, (unsigned long long)uip->ui_vmsize); + mtx_destroy(&uip->ui_vmsize_mtx); + free(uip, M_UIDINFO); } void diff --git a/sys/sys/refcount.h b/sys/sys/refcount.h index 4611664..9dff576 100644 --- a/sys/sys/refcount.h +++ b/sys/sys/refcount.h @@ -64,4 +64,29 @@ refcount_release(volatile u_int *count) return (old == 1); } +#define REFCOUNT_RELEASE_DEFINE(NAME, TYPE, LOCK, UNLOCK) \ +static __inline int \ +refcount_release_take_##NAME(volatile u_int *count, TYPE *v) \ +{ \ + u_int old; \ + \ + old = *count; \ + if (old > 1 && atomic_cmpset_int(count, old, old - 1)) \ + return (0); \ + LOCK(v); \ + if (refcount_release(count)) \ + return (1); \ + UNLOCK(v); \ + return (0); \ +} + +#define REFCOUNT_RELEASE_TAKE_USE_MTX \ + REFCOUNT_RELEASE_DEFINE(mtx, struct mtx, mtx_lock, mtx_unlock); +#define REFCOUNT_RELEASE_TAKE_USE_RWLOCK \ + REFCOUNT_RELEASE_DEFINE(rwlock, struct rwlock, rw_wlock, rw_wunlock); +#define REFCOUNT_RELEASE_TAKE_USE_RMLOCK \ + REFCOUNT_RELEASE_DEFINE(rmlock, struct rmlock, rm_wlock, rm_wunlock); +#define REFCOUNT_RELEASE_TAKE_USE_SX \ + REFCOUNT_RELEASE_DEFINE(sx, struct sx, sx_xlock, sx_xunlock); + #endif /* ! __SYS_REFCOUNT_H__ */