RE: [PATCH v14 net-next 04/13] dpll: send delete notification before unregister in on-pin rollback

[Nitka, Grzegorz]

> -----Original Message-----
> From: Nitka, Grzegorz
> Sent: Wednesday, June 10, 2026 9:42 PM
> To: Nitka, Grzegorz <grzegorz.nitka@xxxxxxxxx>; Kubalewski, Arkadiusz
> <Arkadiusz.Kubalewski@xxxxxxxxx>; netdev@xxxxxxxxxxxxxxx
> Cc: Vecera, Ivan <ivecera@xxxxxxxxxx>; vadim.fedorenko@xxxxxxxxx;
> kuba@xxxxxxxxxx; jiri@xxxxxxxxxxx; edumazet@xxxxxxxxxx; Kitszel,
> Przemyslaw <przemyslaw.kitszel@xxxxxxxxx>; richardcochran@xxxxxxxxx;
> donald.hunter@xxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> andrew+netdev@xxxxxxx; intel-wired-lan@xxxxxxxxxxxxxxxx;
> horms@xxxxxxxxxx; Prathosh.Satish@xxxxxxxxxxxxx; Nguyen, Anthony L
> <anthony.l.nguyen@xxxxxxxxx>; pabeni@xxxxxxxxxx; davem@xxxxxxxxxxxxx
> Subject: RE: [PATCH v14 net-next 04/13] dpll: send delete notification before
> unregister in on-pin rollback
> 
> 
> 
> > -----Original Message-----
> > From: Intel-wired-lan <intel-wired-lan-bounces@xxxxxxxxxx> On Behalf Of
> > Nitka, Grzegorz
> > Sent: Tuesday, June 9, 2026 9:10 AM
> > To: Kubalewski, Arkadiusz <arkadiusz.kubalewski@xxxxxxxxx>;
> > netdev@xxxxxxxxxxxxxxx
> > Cc: Vecera, Ivan <ivecera@xxxxxxxxxx>; vadim.fedorenko@xxxxxxxxx;
> > kuba@xxxxxxxxxx; jiri@xxxxxxxxxxx; edumazet@xxxxxxxxxx; Kitszel,
> > Przemyslaw <przemyslaw.kitszel@xxxxxxxxx>; richardcochran@xxxxxxxxx;
> > donald.hunter@xxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> > andrew+netdev@xxxxxxx; intel-wired-lan@xxxxxxxxxxxxxxxx;
> > horms@xxxxxxxxxx; Prathosh.Satish@xxxxxxxxxxxxx; Nguyen, Anthony L
> > <anthony.l.nguyen@xxxxxxxxx>; pabeni@xxxxxxxxxx;
> davem@xxxxxxxxxxxxx
> > Subject: Re: [Intel-wired-lan] [PATCH v14 net-next 04/13] dpll: send delete
> > notification before unregister in on-pin rollback
> >
> >
> >
> > > -----Original Message-----
> > > From: Kubalewski, Arkadiusz <arkadiusz.kubalewski@xxxxxxxxx>
> > > Sent: Monday, June 8, 2026 6:45 PM
> > > To: Nitka, Grzegorz <grzegorz.nitka@xxxxxxxxx>; netdev@xxxxxxxxxxxxxxx
> > > Cc: linux-kernel@xxxxxxxxxxxxxxx; intel-wired-lan@xxxxxxxxxxxxxxxx; Oros,
> > Petr
> > > <poros@xxxxxxxxxx>; richardcochran@xxxxxxxxx;
> > andrew+netdev@xxxxxxx;
> > > Kitszel, Przemyslaw <przemyslaw.kitszel@xxxxxxxxx>; Nguyen, Anthony L
> > > <anthony.l.nguyen@xxxxxxxxx>; Prathosh.Satish@xxxxxxxxxxxxx; Vecera,
> > > Ivan <ivecera@xxxxxxxxxx>; jiri@xxxxxxxxxxx; vadim.fedorenko@xxxxxxxxx;
> > > donald.hunter@xxxxxxxxx; horms@xxxxxxxxxx; pabeni@xxxxxxxxxx;
> > > kuba@xxxxxxxxxx; davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx
> > > Subject: RE: [PATCH v14 net-next 04/13] dpll: send delete notification
> > before
> > > unregister in on-pin rollback
> > >
> > > >From: Nitka, Grzegorz <grzegorz.nitka@xxxxxxxxx>
> > > >Sent: Sunday, June 7, 2026 8:31 PM
> > > >
> > > >The rollback path in dpll_pin_on_pin_register() called
> > > >__dpll_pin_unregister() before dpll_pin_delete_ntf(). When the
> > > >unregister dropped the pin's last DPLL reference it cleared the
> > > >DPLL_REGISTERED mark in dpll_pin_xa, so the subsequent
> > > >dpll_pin_event_send() failed dpll_pin_available() and aborted with
> > > >-ENODEV. As a result userspace was never notified of the rollback
> > > >deletion and remained out of sync with the kernel.
> > > >
> > > >Send the delete notification first, matching the order used by
> > > >dpll_pin_unregister() and dpll_pin_on_pin_unregister().
> > > >
> > > >Fixes: 9d71b54b65b1 ("dpll: netlink: Add DPLL framework base
> functions")
> > > >Signed-off-by: Grzegorz Nitka <grzegorz.nitka@xxxxxxxxx>
> > > >---
> > > > drivers/dpll/dpll_core.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > >diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c
> > > >index cea7e2be2cbc..80195f3a84f3 100644
> > > >--- a/drivers/dpll/dpll_core.c
> > > >+++ b/drivers/dpll/dpll_core.c
> > > >@@ -1007,9 +1007,9 @@ int dpll_pin_on_pin_register(struct dpll_pin
> > > >*parent, struct dpll_pin *pin,
> > > > dpll_unregister:
> > > > 	xa_for_each(&parent->dpll_refs, i, ref)
> > > > 		if (i < stop) {
> > > >+			dpll_pin_delete_ntf(pin);
> > > > 			__dpll_pin_unregister(ref->dpll, pin, ops, priv,
> > > > 					      parent);
> > > >-			dpll_pin_delete_ntf(pin);
> > >
> > > Hey Grzegorz,
> > >
> > > Since patch 7/13 this is already part of __dpll_pin_unregister ?
> > > The call should be removed at all? Please make sure all calls to
> > > __dpll_pin_unregister(..) are aware of that, also maybe better to
> > > put this all related to one patch?
> > >
> >
> > Yes, I think this patch is a good candidate to squash with 7/13 (with
> > proper commit message edit).
> > Same 'Fixes' tag and both touch notifications fix.
> >
> > Thanks
> >
> > Grzegorz
> 
> Hi Arek,
> 
> I revisited your comment, from the perspective of redundant calls
> to dpll_pin_delete_ntf after patch 7/13 is applied to.
> Yes, there are still 2 "redundant" calls to dpll_pin_delete_ntf on top of
> __dpll_pin_unregister.
> I'm going to send v14 with this fix and reordered commits in the patchset
> (7/13 moved up).
> 
> Regards
> 
> Grzegorz
> 

Wait ... those 2 mentioned calls to dpll_pin_delete_ntf  are symmetric behavior
for calls to dpll_pin_create_ntf on success path in dpll_pin_on_pin_register.
I don't understand why there are two pin create notifications needed (inside and 
Outside of __dpll_pin_register).
However it's long-standing code, maybe there is a consensus to have two calls.
So I'm skeptical to change it, unless someone tell me it's the right thing to do.
It's out of scope of this series, in my opinion. I'd leave it as it is.

Regards

Grzegorz

> > > > 		}
> > > > 	dpll_xa_ref_pin_del(&pin->parent_refs, parent, ops, priv, pin);
> > > > unlock:
> > > >--
> > > >2.39.3