Re: [net-next,v2,08/12] dpll: Enhance and consolidate reference counting logic
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-01-21 00:18:54
Also in:
intel-wired-lan, linux-devicetree, linux-rdma, lkml
This is an AI-generated review of your patch. Dunno if there's a reason for having this fixed by a later patch, if not let's fix. I'm sending the review mostly because of the comments on patch 12. For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html --- dpll: Enhance and consolidate reference counting logic This commit refactors DPLL reference counting by introducing centralized __dpll_{device,pin}_{hold,put}() helpers and updating dpll_xa_ref_* functions to automatically manage references when adding/removing entries.
quoted hunk ↗ jump to hunk
diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c index 156f95de8e8e..f2a77eb1b991 100644
[ ... ]
quoted hunk ↗ jump to hunk
@@ -155,6 +194,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);
The new __dpll_pin_hold(pin) call is added here for the success path, but
looking at the context above this hunk, there is an early return path when
a duplicate registration is found:
reg = dpll_pin_registration_find(ref, ops, priv, cookie);
if (reg) {
refcount_inc(&ref->refcount);
return 0;
}
This early return path increments ref->refcount and returns success, but
does not call __dpll_pin_hold(pin). Meanwhile, dpll_xa_ref_pin_del() always
calls __dpll_pin_put(pin). Could this cause a refcount underflow if a caller
attempts duplicate registration and then later unregistration is attempted?
Note: This appears to be addressed by 934c22045c2a ("dpll: Prevent duplicate
registrations") later in the series, which changes this early return to
return -EEXIST instead of success.
[ ... ]
quoted hunk ↗ jump to hunk
@@ -236,6 +277,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);
The same pattern exists in dpll_xa_ref_dpll_add(): the early return path for duplicate registrations increments ref->refcount without calling __dpll_device_hold(dpll), while dpll_xa_ref_dpll_del() always calls __dpll_device_put(dpll). The same fix in 934c22045c2a addresses this. -- pw-bot: cr