Re: [Intel-wired-lan] [net-next, v2, 08/12] dpll: Enhance and consolidate reference counting logic
On Fri, Jan 23, 2026 at 04:27:21PM +0100, Ivan Vecera wrote: > On 1/23/26 3:58 PM, Simon Horman wrote: > > On Wed, Jan 21, 2026 at 09:18:02AM +0100, Ivan Vecera wrote: > > > On 1/21/26 1:16 AM, Jakub Kicinski wrote: > > > > 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. > > > Will reorder these patches... Maybe it would be better to send patch 9 > > > separately to net as this is the fix for the bug we found during > > > development of this series. > > > > Hi Ivan, > > > > If it is a but in net, then yes, that sounds like a good idea to me. > > > > Please include a Fixes tag if you take that route. > > > Yes, it was submitted to net with Fixes and recently merged as > > https://git.kernel.org/netdev/net/c/f3ddbf4d Thanks, sorry for missing that earlier.
Re: [Intel-wired-lan] [net-next, v2, 08/12] dpll: Enhance and consolidate reference counting logic
On 1/23/26 3:58 PM, Simon Horman wrote: On Wed, Jan 21, 2026 at 09:18:02AM +0100, Ivan Vecera wrote: On 1/21/26 1:16 AM, Jakub Kicinski wrote: 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. Will reorder these patches... Maybe it would be better to send patch 9 separately to net as this is the fix for the bug we found during development of this series. Hi Ivan, If it is a but in net, then yes, that sounds like a good idea to me. Please include a Fixes tag if you take that route. Yes, it was submitted to net with Fixes and recently merged as https://git.kernel.org/netdev/net/c/f3ddbf4d Ivan
Re: [Intel-wired-lan] [net-next, v2, 08/12] dpll: Enhance and consolidate reference counting logic
On Wed, Jan 21, 2026 at 09:18:02AM +0100, Ivan Vecera wrote: > On 1/21/26 1:16 AM, Jakub Kicinski wrote: > > 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. > Will reorder these patches... Maybe it would be better to send patch 9 > separately to net as this is the fix for the bug we found during > development of this series. Hi Ivan, If it is a but in net, then yes, that sounds like a good idea to me. Please include a Fixes tag if you take that route.
Re: [Intel-wired-lan] [net-next, v2, 08/12] dpll: Enhance and consolidate reference counting logic
On 1/21/26 1:16 AM, Jakub Kicinski wrote: 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. Will reorder these patches... Maybe it would be better to send patch 9 separately to net as this is the fix for the bug we found during development of this series. Ivan
Re: [Intel-wired-lan] [net-next, v2, 08/12] dpll: Enhance and consolidate reference counting logic
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.
> diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
> index 156f95de8e8e..f2a77eb1b991 100644
[ ... ]
> @@ -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.
[ ... ]
> @@ -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
