Skip to content

Commit 38d7b94

Browse files
kubalewskikuba-moo
kubalewski
authored and
kuba-moo
committed
dpll: fix dpll_pin_on_pin_register() for multiple parent pins
In scenario where pin is registered with multiple parent pins via dpll_pin_on_pin_register(..), all belonging to the same dpll device. A second call to dpll_pin_on_pin_unregister(..) would cause a call trace, as it tries to use already released registration resources (due to fix introduced in b446631). In this scenario pin was registered twice, so resources are not yet expected to be release until each registered pin/pin pair is unregistered. Currently, the following crash/call trace is produced when ice driver is removed on the system with installed E810T NIC which includes dpll device: WARNING: CPU: 51 PID: 9155 at drivers/dpll/dpll_core.c:809 dpll_pin_ops+0x20/0x30 RIP: 0010:dpll_pin_ops+0x20/0x30 Call Trace: ? __warn+0x7f/0x130 ? dpll_pin_ops+0x20/0x30 dpll_msg_add_pin_freq+0x37/0x1d0 dpll_cmd_pin_get_one+0x1c0/0x400 ? __nlmsg_put+0x63/0x80 dpll_pin_event_send+0x93/0x140 dpll_pin_on_pin_unregister+0x3f/0x100 ice_dpll_deinit_pins+0xa1/0x230 [ice] ice_remove+0xf1/0x210 [ice] Fix by adding a parent pointer as a cookie when creating a registration, also when searching for it. For the regular pins pass NULL, this allows to create separated registration for each parent the pin is registered with. Fixes: b446631 ("dpll: fix dpll_xa_ref_*_del() for multiple registrations") Signed-off-by: Arkadiusz Kubalewski <[email protected]> Reviewed-by: Jiri Pirko <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 0c81ea5 commit 38d7b94

File tree

1 file changed

+33
-25
lines changed

1 file changed

+33
-25
lines changed

‎drivers/dpll/dpll_core.c‎

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ struct dpll_pin_registration {
4242
struct list_head list;
4343
const struct dpll_pin_ops *ops;
4444
void *priv;
45+
void *cookie;
4546
};
4647

4748
struct dpll_device *dpll_device_get_by_id(int id)
@@ -54,20 +55,23 @@ struct dpll_device *dpll_device_get_by_id(int id)
5455

5556
static struct dpll_pin_registration *
5657
dpll_pin_registration_find(struct dpll_pin_ref *ref,
57-
const struct dpll_pin_ops *ops, void *priv)
58+
const struct dpll_pin_ops *ops, void *priv,
59+
void *cookie)
5860
{
5961
struct dpll_pin_registration *reg;
6062

6163
list_for_each_entry(reg, &ref->registration_list, list) {
62-
if (reg->ops == ops && reg->priv == priv)
64+
if (reg->ops == ops && reg->priv == priv &&
65+
reg->cookie == cookie)
6366
return reg;
6467
}
6568
return NULL;
6669
}
6770

6871
static int
6972
dpll_xa_ref_pin_add(struct xarray *xa_pins, struct dpll_pin *pin,
70-
const struct dpll_pin_ops *ops, void *priv)
73+
const struct dpll_pin_ops *ops, void *priv,
74+
void *cookie)
7175
{
7276
struct dpll_pin_registration *reg;
7377
struct dpll_pin_ref *ref;
@@ -78,7 +82,7 @@ dpll_xa_ref_pin_add(struct xarray *xa_pins, struct dpll_pin *pin,
7882
xa_for_each(xa_pins, i, ref) {
7983
if (ref->pin != pin)
8084
continue;
81-
reg = dpll_pin_registration_find(ref, ops, priv);
85+
reg = dpll_pin_registration_find(ref, ops, priv, cookie);
8286
if (reg) {
8387
refcount_inc(&ref->refcount);
8488
return 0;
@@ -111,6 +115,7 @@ dpll_xa_ref_pin_add(struct xarray *xa_pins, struct dpll_pin *pin,
111115
}
112116
reg->ops = ops;
113117
reg->priv = priv;
118+
reg->cookie = cookie;
114119
if (ref_exists)
115120
refcount_inc(&ref->refcount);
116121
list_add_tail(&reg->list, &ref->registration_list);
@@ -119,7 +124,8 @@ dpll_xa_ref_pin_add(struct xarray *xa_pins, struct dpll_pin *pin,
119124
}
120125

121126
static int dpll_xa_ref_pin_del(struct xarray *xa_pins, struct dpll_pin *pin,
122-
const struct dpll_pin_ops *ops, void *priv)
127+
const struct dpll_pin_ops *ops, void *priv,
128+
void *cookie)
123129
{
124130
struct dpll_pin_registration *reg;
125131
struct dpll_pin_ref *ref;
@@ -128,7 +134,7 @@ static int dpll_xa_ref_pin_del(struct xarray *xa_pins, struct dpll_pin *pin,
128134
xa_for_each(xa_pins, i, ref) {
129135
if (ref->pin != pin)
130136
continue;
131-
reg = dpll_pin_registration_find(ref, ops, priv);
137+
reg = dpll_pin_registration_find(ref, ops, priv, cookie);
132138
if (WARN_ON(!reg))
133139
return -EINVAL;
134140
list_del(&reg->list);
@@ -146,7 +152,7 @@ static int dpll_xa_ref_pin_del(struct xarray *xa_pins, struct dpll_pin *pin,
146152

147153
static int
148154
dpll_xa_ref_dpll_add(struct xarray *xa_dplls, struct dpll_device *dpll,
149-
const struct dpll_pin_ops *ops, void *priv)
155+
const struct dpll_pin_ops *ops, void *priv, void *cookie)
150156
{
151157
struct dpll_pin_registration *reg;
152158
struct dpll_pin_ref *ref;
@@ -157,7 +163,7 @@ dpll_xa_ref_dpll_add(struct xarray *xa_dplls, struct dpll_device *dpll,
157163
xa_for_each(xa_dplls, i, ref) {
158164
if (ref->dpll != dpll)
159165
continue;
160-
reg = dpll_pin_registration_find(ref, ops, priv);
166+
reg = dpll_pin_registration_find(ref, ops, priv, cookie);
161167
if (reg) {
162168
refcount_inc(&ref->refcount);
163169
return 0;
@@ -190,6 +196,7 @@ dpll_xa_ref_dpll_add(struct xarray *xa_dplls, struct dpll_device *dpll,
190196
}
191197
reg->ops = ops;
192198
reg->priv = priv;
199+
reg->cookie = cookie;
193200
if (ref_exists)
194201
refcount_inc(&ref->refcount);
195202
list_add_tail(&reg->list, &ref->registration_list);
@@ -199,7 +206,7 @@ dpll_xa_ref_dpll_add(struct xarray *xa_dplls, struct dpll_device *dpll,
199206

200207
static void
201208
dpll_xa_ref_dpll_del(struct xarray *xa_dplls, struct dpll_device *dpll,
202-
const struct dpll_pin_ops *ops, void *priv)
209+
const struct dpll_pin_ops *ops, void *priv, void *cookie)
203210
{
204211
struct dpll_pin_registration *reg;
205212
struct dpll_pin_ref *ref;
@@ -208,7 +215,7 @@ dpll_xa_ref_dpll_del(struct xarray *xa_dplls, struct dpll_device *dpll,
208215
xa_for_each(xa_dplls, i, ref) {
209216
if (ref->dpll != dpll)
210217
continue;
211-
reg = dpll_pin_registration_find(ref, ops, priv);
218+
reg = dpll_pin_registration_find(ref, ops, priv, cookie);
212219
if (WARN_ON(!reg))
213220
return;
214221
list_del(&reg->list);
@@ -594,14 +601,14 @@ EXPORT_SYMBOL_GPL(dpll_pin_put);
594601

595602
static int
596603
__dpll_pin_register(struct dpll_device *dpll, struct dpll_pin *pin,
597-
const struct dpll_pin_ops *ops, void *priv)
604+
const struct dpll_pin_ops *ops, void *priv, void *cookie)
598605
{
599606
int ret;
600607

601-
ret = dpll_xa_ref_pin_add(&dpll->pin_refs, pin, ops, priv);
608+
ret = dpll_xa_ref_pin_add(&dpll->pin_refs, pin, ops, priv, cookie);
602609
if (ret)
603610
return ret;
604-
ret = dpll_xa_ref_dpll_add(&pin->dpll_refs, dpll, ops, priv);
611+
ret = dpll_xa_ref_dpll_add(&pin->dpll_refs, dpll, ops, priv, cookie);
605612
if (ret)
606613
goto ref_pin_del;
607614
xa_set_mark(&dpll_pin_xa, pin->id, DPLL_REGISTERED);
@@ -610,7 +617,7 @@ __dpll_pin_register(struct dpll_device *dpll, struct dpll_pin *pin,
610617
return ret;
611618

612619
ref_pin_del:
613-
dpll_xa_ref_pin_del(&dpll->pin_refs, pin, ops, priv);
620+
dpll_xa_ref_pin_del(&dpll->pin_refs, pin, ops, priv, cookie);
614621
return ret;
615622
}
616623

@@ -642,7 +649,7 @@ dpll_pin_register(struct dpll_device *dpll, struct dpll_pin *pin,
642649
dpll->clock_id == pin->clock_id)))
643650
ret = -EINVAL;
644651
else
645-
ret = __dpll_pin_register(dpll, pin, ops, priv);
652+
ret = __dpll_pin_register(dpll, pin, ops, priv, NULL);
646653
mutex_unlock(&dpll_lock);
647654

648655
return ret;
@@ -651,11 +658,11 @@ EXPORT_SYMBOL_GPL(dpll_pin_register);
651658

652659
static void
653660
__dpll_pin_unregister(struct dpll_device *dpll, struct dpll_pin *pin,
654-
const struct dpll_pin_ops *ops, void *priv)
661+
const struct dpll_pin_ops *ops, void *priv, void *cookie)
655662
{
656663
ASSERT_DPLL_PIN_REGISTERED(pin);
657-
dpll_xa_ref_pin_del(&dpll->pin_refs, pin, ops, priv);
658-
dpll_xa_ref_dpll_del(&pin->dpll_refs, dpll, ops, priv);
664+
dpll_xa_ref_pin_del(&dpll->pin_refs, pin, ops, priv, cookie);
665+
dpll_xa_ref_dpll_del(&pin->dpll_refs, dpll, ops, priv, cookie);
659666
if (xa_empty(&pin->dpll_refs))
660667
xa_clear_mark(&dpll_pin_xa, pin->id, DPLL_REGISTERED);
661668
}
@@ -680,7 +687,7 @@ void dpll_pin_unregister(struct dpll_device *dpll, struct dpll_pin *pin,
680687

681688
mutex_lock(&dpll_lock);
682689
dpll_pin_delete_ntf(pin);
683-
__dpll_pin_unregister(dpll, pin, ops, priv);
690+
__dpll_pin_unregister(dpll, pin, ops, priv, NULL);
684691
mutex_unlock(&dpll_lock);
685692
}
686693
EXPORT_SYMBOL_GPL(dpll_pin_unregister);
@@ -716,12 +723,12 @@ int dpll_pin_on_pin_register(struct dpll_pin *parent, struct dpll_pin *pin,
716723
return -EINVAL;
717724

718725
mutex_lock(&dpll_lock);
719-
ret = dpll_xa_ref_pin_add(&pin->parent_refs, parent, ops, priv);
726+
ret = dpll_xa_ref_pin_add(&pin->parent_refs, parent, ops, priv, pin);
720727
if (ret)
721728
goto unlock;
722729
refcount_inc(&pin->refcount);
723730
xa_for_each(&parent->dpll_refs, i, ref) {
724-
ret = __dpll_pin_register(ref->dpll, pin, ops, priv);
731+
ret = __dpll_pin_register(ref->dpll, pin, ops, priv, parent);
725732
if (ret) {
726733
stop = i;
727734
goto dpll_unregister;
@@ -735,11 +742,12 @@ int dpll_pin_on_pin_register(struct dpll_pin *parent, struct dpll_pin *pin,
735742
dpll_unregister:
736743
xa_for_each(&parent->dpll_refs, i, ref)
737744
if (i < stop) {
738-
__dpll_pin_unregister(ref->dpll, pin, ops, priv);
745+
__dpll_pin_unregister(ref->dpll, pin, ops, priv,
746+
parent);
739747
dpll_pin_delete_ntf(pin);
740748
}
741749
refcount_dec(&pin->refcount);
742-
dpll_xa_ref_pin_del(&pin->parent_refs, parent, ops, priv);
750+
dpll_xa_ref_pin_del(&pin->parent_refs, parent, ops, priv, pin);
743751
unlock:
744752
mutex_unlock(&dpll_lock);
745753
return ret;
@@ -764,10 +772,10 @@ void dpll_pin_on_pin_unregister(struct dpll_pin *parent, struct dpll_pin *pin,
764772

765773
mutex_lock(&dpll_lock);
766774
dpll_pin_delete_ntf(pin);
767-
dpll_xa_ref_pin_del(&pin->parent_refs, parent, ops, priv);
775+
dpll_xa_ref_pin_del(&pin->parent_refs, parent, ops, priv, pin);
768776
refcount_dec(&pin->refcount);
769777
xa_for_each(&pin->dpll_refs, i, ref)
770-
__dpll_pin_unregister(ref->dpll, pin, ops, priv);
778+
__dpll_pin_unregister(ref->dpll, pin, ops, priv, parent);
771779
mutex_unlock(&dpll_lock);
772780
}
773781
EXPORT_SYMBOL_GPL(dpll_pin_on_pin_unregister);

0 commit comments

Comments
 (0)