lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <LV4PR11MB94917B69A82A442F762221959B99A@LV4PR11MB9491.namprd11.prod.outlook.com>
Date: Thu, 5 Feb 2026 08:58:30 +0000
From: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@...el.com>
To: "Vecera, Ivan" <ivecera@...hat.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>
CC: "Loktionov, Aleksandr" <aleksandr.loktionov@...el.com>, "Lobakin,
 Aleksander" <aleksander.lobakin@...el.com>, Andrew Lunn
	<andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>, "Eric
 Dumazet" <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Jiri Pirko
	<jiri@...nulli.us>, Jonathan Lemon <jonathan.lemon@...il.com>, "Leon
 Romanovsky" <leon@...nel.org>, Mark Bloch <mbloch@...dia.com>, Paolo Abeni
	<pabeni@...hat.com>, Prathosh Satish <Prathosh.Satish@...rochip.com>,
	"Kitszel, Przemyslaw" <przemyslaw.kitszel@...el.com>, Richard Cochran
	<richardcochran@...il.com>, Saeed Mahameed <saeedm@...dia.com>, Tariq Toukan
	<tariqt@...dia.com>, "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>, "Vadim
 Fedorenko" <vadim.fedorenko@...ux.dev>, "intel-wired-lan@...ts.osuosl.org"
	<intel-wired-lan@...ts.osuosl.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-rdma@...r.kernel.org"
	<linux-rdma@...r.kernel.org>
Subject: RE: [PATCH net-next v5 6/9] dpll: Enhance and consolidate reference
 counting logic

>From: Ivan Vecera <ivecera@...hat.com>
>Sent: Tuesday, February 3, 2026 6:40 PM
>
>Refactor the reference counting mechanism for DPLL devices and pins to
>improve consistency and prevent potential lifetime issues.
>
>Introduce internal helpers __dpll_{device,pin}_{hold,put}() to
>centralize reference management.
>
>Update the internal XArray reference helpers (dpll_xa_ref_*) to
>automatically grab a reference to the target object when it is added to
>a list, and release it when removed. This ensures that objects linked
>internally (e.g., pins referenced by parent pins) are properly kept
>alive without relying on the caller to manually manage the count.
>
>Consequently, remove the now redundant manual `refcount_inc/dec` calls
>in dpll_pin_on_pin_{,un}register()`, as ownership is now correctly handled
>by the dpll_xa_ref_* functions.
>
>Additionally, ensure that dpll_device_{,un}register()` takes/releases
>a reference to the device, ensuring the device object remains valid for
>the duration of its registration.
>
>Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>

LGTM,
Reviewed-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>

>Signed-off-by: Ivan Vecera <ivecera@...hat.com>
>---
> drivers/dpll/dpll_core.c | 74 +++++++++++++++++++++++++++-------------
> 1 file changed, 50 insertions(+), 24 deletions(-)
>
>diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
>index 59081cf2c73ae..f6ab4f0cad84d 100644
>--- a/drivers/dpll/dpll_core.c
>+++ b/drivers/dpll/dpll_core.c
>@@ -83,6 +83,45 @@ void dpll_pin_notify(struct dpll_pin *pin, unsigned
>long action)
> 	call_dpll_notifiers(action, &info);
> }
>
>+static void __dpll_device_hold(struct dpll_device *dpll)
>+{
>+	refcount_inc(&dpll->refcount);
>+}
>+
>+static void __dpll_device_put(struct dpll_device *dpll)
>+{
>+	if (refcount_dec_and_test(&dpll->refcount)) {
>+		ASSERT_DPLL_NOT_REGISTERED(dpll);
>+		WARN_ON_ONCE(!xa_empty(&dpll->pin_refs));
>+		xa_destroy(&dpll->pin_refs);
>+		xa_erase(&dpll_device_xa, dpll->id);
>+		WARN_ON(!list_empty(&dpll->registration_list));
>+		kfree(dpll);
>+	}
>+}
>+
>+static void __dpll_pin_hold(struct dpll_pin *pin)
>+{
>+	refcount_inc(&pin->refcount);
>+}
>+
>+static void dpll_pin_idx_free(u32 pin_idx);
>+static void dpll_pin_prop_free(struct dpll_pin_properties *prop);
>+
>+static void __dpll_pin_put(struct dpll_pin *pin)
>+{
>+	if (refcount_dec_and_test(&pin->refcount)) {
>+		xa_erase(&dpll_pin_xa, pin->id);
>+		xa_destroy(&pin->dpll_refs);
>+		xa_destroy(&pin->parent_refs);
>+		xa_destroy(&pin->ref_sync_pins);
>+		dpll_pin_prop_free(&pin->prop);
>+		fwnode_handle_put(pin->fwnode);
>+		dpll_pin_idx_free(pin->pin_idx);
>+		kfree_rcu(pin, rcu);
>+	}
>+}
>+
> struct dpll_device *dpll_device_get_by_id(int id)
> {
> 	if (xa_get_mark(&dpll_device_xa, id, DPLL_REGISTERED))
>@@ -152,6 +191,7 @@ dpll_xa_ref_pin_add(struct xarray *xa_pins, struct
>dpll_pin *pin,
> 	reg->ops = ops;
> 	reg->priv = priv;
> 	reg->cookie = cookie;
>+	__dpll_pin_hold(pin);
> 	if (ref_exists)
> 		refcount_inc(&ref->refcount);
> 	list_add_tail(&reg->list, &ref->registration_list);
>@@ -174,6 +214,7 @@ static int dpll_xa_ref_pin_del(struct xarray *xa_pins,
>struct dpll_pin *pin,
> 		if (WARN_ON(!reg))
> 			return -EINVAL;
> 		list_del(&reg->list);
>+		__dpll_pin_put(pin);
> 		kfree(reg);
> 		if (refcount_dec_and_test(&ref->refcount)) {
> 			xa_erase(xa_pins, i);
>@@ -231,6 +272,7 @@ dpll_xa_ref_dpll_add(struct xarray *xa_dplls, struct
>dpll_device *dpll,
> 	reg->ops = ops;
> 	reg->priv = priv;
> 	reg->cookie = cookie;
>+	__dpll_device_hold(dpll);
> 	if (ref_exists)
> 		refcount_inc(&ref->refcount);
> 	list_add_tail(&reg->list, &ref->registration_list);
>@@ -253,6 +295,7 @@ dpll_xa_ref_dpll_del(struct xarray *xa_dplls, struct
>dpll_device *dpll,
> 		if (WARN_ON(!reg))
> 			return;
> 		list_del(&reg->list);
>+		__dpll_device_put(dpll);
> 		kfree(reg);
> 		if (refcount_dec_and_test(&ref->refcount)) {
> 			xa_erase(xa_dplls, i);
>@@ -323,8 +366,8 @@ dpll_device_get(u64 clock_id, u32 device_idx, struct
>module *module)
> 		if (dpll->clock_id == clock_id &&
> 		    dpll->device_idx == device_idx &&
> 		    dpll->module == module) {
>+			__dpll_device_hold(dpll);
> 			ret = dpll;
>-			refcount_inc(&ret->refcount);
> 			break;
> 		}
> 	}
>@@ -347,14 +390,7 @@ EXPORT_SYMBOL_GPL(dpll_device_get);
> void dpll_device_put(struct dpll_device *dpll)
> {
> 	mutex_lock(&dpll_lock);
>-	if (refcount_dec_and_test(&dpll->refcount)) {
>-		ASSERT_DPLL_NOT_REGISTERED(dpll);
>-		WARN_ON_ONCE(!xa_empty(&dpll->pin_refs));
>-		xa_destroy(&dpll->pin_refs);
>-		xa_erase(&dpll_device_xa, dpll->id);
>-		WARN_ON(!list_empty(&dpll->registration_list));
>-		kfree(dpll);
>-	}
>+	__dpll_device_put(dpll);
> 	mutex_unlock(&dpll_lock);
> }
> EXPORT_SYMBOL_GPL(dpll_device_put);
>@@ -416,6 +452,7 @@ int dpll_device_register(struct dpll_device *dpll,
>enum dpll_type type,
> 	reg->ops = ops;
> 	reg->priv = priv;
> 	dpll->type = type;
>+	__dpll_device_hold(dpll);
> 	first_registration = list_empty(&dpll->registration_list);
> 	list_add_tail(&reg->list, &dpll->registration_list);
> 	if (!first_registration) {
>@@ -455,6 +492,7 @@ void dpll_device_unregister(struct dpll_device *dpll,
> 		return;
> 	}
> 	list_del(&reg->list);
>+	__dpll_device_put(dpll);
> 	kfree(reg);
>
> 	if (!list_empty(&dpll->registration_list)) {
>@@ -666,8 +704,8 @@ dpll_pin_get(u64 clock_id, u32 pin_idx, struct module
>*module,
> 		if (pos->clock_id == clock_id &&
> 		    pos->pin_idx == pin_idx &&
> 		    pos->module == module) {
>+			__dpll_pin_hold(pos);
> 			ret = pos;
>-			refcount_inc(&ret->refcount);
> 			break;
> 		}
> 	}
>@@ -690,16 +728,7 @@ EXPORT_SYMBOL_GPL(dpll_pin_get);
> void dpll_pin_put(struct dpll_pin *pin)
> {
> 	mutex_lock(&dpll_lock);
>-	if (refcount_dec_and_test(&pin->refcount)) {
>-		xa_erase(&dpll_pin_xa, pin->id);
>-		xa_destroy(&pin->dpll_refs);
>-		xa_destroy(&pin->parent_refs);
>-		xa_destroy(&pin->ref_sync_pins);
>-		dpll_pin_prop_free(&pin->prop);
>-		fwnode_handle_put(pin->fwnode);
>-		dpll_pin_idx_free(pin->pin_idx);
>-		kfree_rcu(pin, rcu);
>-	}
>+	__dpll_pin_put(pin);
> 	mutex_unlock(&dpll_lock);
> }
> EXPORT_SYMBOL_GPL(dpll_pin_put);
>@@ -740,8 +769,8 @@ struct dpll_pin *fwnode_dpll_pin_find(struct
>fwnode_handle *fwnode)
> 	mutex_lock(&dpll_lock);
> 	xa_for_each(&dpll_pin_xa, index, pin) {
> 		if (pin->fwnode == fwnode) {
>+			__dpll_pin_hold(pin);
> 			ret = pin;
>-			refcount_inc(&ret->refcount);
> 			break;
> 		}
> 	}
>@@ -893,7 +922,6 @@ int dpll_pin_on_pin_register(struct dpll_pin *parent,
>struct dpll_pin *pin,
> 	ret = dpll_xa_ref_pin_add(&pin->parent_refs, parent, ops, priv,
>pin);
> 	if (ret)
> 		goto unlock;
>-	refcount_inc(&pin->refcount);
> 	xa_for_each(&parent->dpll_refs, i, ref) {
> 		ret = __dpll_pin_register(ref->dpll, pin, ops, priv, parent);
> 		if (ret) {
>@@ -913,7 +941,6 @@ int dpll_pin_on_pin_register(struct dpll_pin *parent,
>struct dpll_pin *pin,
> 					      parent);
> 			dpll_pin_delete_ntf(pin);
> 		}
>-	refcount_dec(&pin->refcount);
> 	dpll_xa_ref_pin_del(&pin->parent_refs, parent, ops, priv, pin);
> unlock:
> 	mutex_unlock(&dpll_lock);
>@@ -940,7 +967,6 @@ void dpll_pin_on_pin_unregister(struct dpll_pin
>*parent, struct dpll_pin *pin,
> 	mutex_lock(&dpll_lock);
> 	dpll_pin_delete_ntf(pin);
> 	dpll_xa_ref_pin_del(&pin->parent_refs, parent, ops, priv, pin);
>-	refcount_dec(&pin->refcount);
> 	xa_for_each(&pin->dpll_refs, i, ref)
> 		__dpll_pin_unregister(ref->dpll, pin, ops, priv, parent);
> 	mutex_unlock(&dpll_lock);
>--
>2.52.0


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ