[<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(®->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(®->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(®->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(®->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(®->list, &dpll->registration_list);
> if (!first_registration) {
>@@ -455,6 +492,7 @@ void dpll_device_unregister(struct dpll_device *dpll,
> return;
> }
> list_del(®->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