From owner-dev-commits-src-branches@freebsd.org Wed Mar 10 22:27:48 2021 Return-Path: Delivered-To: dev-commits-src-branches@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 305795791D6; Wed, 10 Mar 2021 22:27:48 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Dwmt94rTpz4ljg; Wed, 10 Mar 2021 22:27:45 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 6E5671979A; Wed, 10 Mar 2021 22:27:44 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 12AMRien032416; Wed, 10 Mar 2021 22:27:44 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 12AMRimd032415; Wed, 10 Mar 2021 22:27:44 GMT (envelope-from git) Date: Wed, 10 Mar 2021 22:27:44 GMT Message-Id: <202103102227.12AMRimd032415@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: "Alexander V. Chernikov" Subject: git: d9bcd8e7a2dd - stable/13 - Add ifa_try_ref() to simplify ifa handling inside epoch. MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: melifaro X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: d9bcd8e7a2dd77969f8ac940fe8ec5e90588e3ea Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-branches@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commits to the stable branches of the FreeBSD src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Mar 2021 22:27:48 -0000 The branch stable/13 has been updated by melifaro: URL: https://cgit.FreeBSD.org/src/commit/?id=d9bcd8e7a2dd77969f8ac940fe8ec5e90588e3ea commit d9bcd8e7a2dd77969f8ac940fe8ec5e90588e3ea Author: Alexander V. Chernikov AuthorDate: 2021-02-16 20:12:58 +0000 Commit: Alexander V. Chernikov CommitDate: 2021-03-10 21:47:54 +0000 Add ifa_try_ref() to simplify ifa handling inside epoch. More and more code migrates from lock-based protection to the NET_EPOCH umbrella. It requires some logic changes, including, notably, refcount handling. When we have an `ifa` pointer and we're running inside epoch we're guaranteed that this pointer will not be freed. However, the following case can still happen: * in thread 1 we drop to 0 refcount for ifa and schedule its deletion. * in thread 2 we use this ifa and reference it * destroy callout kicks in * unhappy user reports bug To address it, new `ifa_try_ref()` function is added, allowing to return failure when we try to reference `ifa` with 0 refcount. Additionally, existing `ifa_ref()` is enforced with `KASSERT` to provide cleaner error in such scenarious. Reviewed By: rstone, donner Differential Revision: https://reviews.freebsd.org/D28639 (cherry picked from commit 600eade2fb4faacfcd408a01140ef15f85f0c817) --- sys/net/if.c | 12 +++++++++++- sys/net/if_var.h | 1 + 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/sys/net/if.c b/sys/net/if.c index c85cfab19bf6..948be6876b65 100644 --- a/sys/net/if.c +++ b/sys/net/if.c @@ -1857,8 +1857,18 @@ fail: void ifa_ref(struct ifaddr *ifa) { + u_int old; - refcount_acquire(&ifa->ifa_refcnt); + old = refcount_acquire(&ifa->ifa_refcnt); + KASSERT(old > 0, ("%s: ifa %p has 0 refs", __func__, ifa)); +} + +int +ifa_try_ref(struct ifaddr *ifa) +{ + + NET_EPOCH_ASSERT(); + return (refcount_acquire_if_not_zero(&ifa->ifa_refcnt)); } static void diff --git a/sys/net/if_var.h b/sys/net/if_var.h index beb9596895ee..bb364fd10974 100644 --- a/sys/net/if_var.h +++ b/sys/net/if_var.h @@ -575,6 +575,7 @@ struct ifaddr { struct ifaddr * ifa_alloc(size_t size, int flags); void ifa_free(struct ifaddr *ifa); void ifa_ref(struct ifaddr *ifa); +int ifa_try_ref(struct ifaddr *ifa); /* * Multicast address structure. This is analogous to the ifaddr