Hi,
There is a lock order problem in ixl(4) show by witness.
Here I replaced a wrong net lock with kernel lock.
----------------------------
revision 1.84
date: 2022/08/05 13:57:16; author: bluhm; state: Exp; lines: +6 -3;
commitid: sAcV6NsO35L03mLS;
The netlock for SIOCSIFMEDIA and SIOCGIFMEDIA ioctl is not necessary.
Legacy drivers run with kernel lock, interface media is MP safe or
has kernel lock. Assert kernel lock in ix(4) and ixl(4).
OK kettenis@
----------------------------
And then jan@ added a mutex to fix memory curruption in the queue.
----------------------------
revision 1.88
date: 2023/07/19 20:22:05; author: jan; state: Exp; lines: +21 -4;
commitid: NuYpolqO3BOrbWpt;
Protect ixl(4) admin queue with mutex(9).
with tweaks from bluhm
tested by bluhm
ok bluhm@
----------------------------
The consequence is that we aquire kernel lock while a mutex is held.
This is illegal. Witness output below.
The kernel lock can be replaced with a mutex. sc_media_status and
sc_media_active have to be protected. I use sc_link_state_mtx as
it is there anyway and the path is not performace critical.
ifm->ifm_status and ifm->ifm_active are protected by kernel lock,
so I keep the assert in ixl_media_status().
ok?
bluhm
Index: dev/pci/if_ixl.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/pci/if_ixl.c,v
retrieving revision 1.88
diff -u -p -r1.88 if_ixl.c
--- dev/pci/if_ixl.c 19 Jul 2023 20:22:05 -0000 1.88
+++ dev/pci/if_ixl.c 28 Sep 2023 13:18:39 -0000
@@ -2074,8 +2074,10 @@ ixl_media_status(struct ifnet *ifp, stru
KERNEL_ASSERT_LOCKED();
+ mtx_enter(&sc->sc_link_state_mtx);
ifm->ifm_status = sc->sc_media_status;
ifm->ifm_active = sc->sc_media_active;
+ mtx_leave(&sc->sc_link_state_mtx);
}
static void
@@ -3482,9 +3484,7 @@ ixl_link_state_update_iaq(struct ixl_sof
return;
}
- KERNEL_LOCK();
link_state = ixl_set_link_status(sc, iaq);
- KERNEL_UNLOCK();
mtx_enter(&sc->sc_link_state_mtx);
if (ifp->if_link_state != link_state) {
ifp->if_link_state = link_state;
@@ -4468,9 +4468,6 @@ ixl_set_link_status(struct ixl_softc *sc
{
const struct ixl_aq_link_status *status;
const struct ixl_phy_type *itype;
-
- KERNEL_ASSERT_LOCKED();
-
uint64_t ifm_active = IFM_ETHER;
uint64_t ifm_status = IFM_AVALID;
int link_state = LINK_STATE_DOWN;
@@ -4496,9 +4493,11 @@ ixl_set_link_status(struct ixl_softc *sc
baudrate = ixl_search_link_speed(status->link_speed);
done:
+ mtx_enter(&sc->sc_link_state_mtx);
sc->sc_media_active = ifm_active;
sc->sc_media_status = ifm_status;
sc->sc_ac.ac_if.if_baudrate = baudrate;
+ mtx_leave(&sc->sc_link_state_mtx);
return (link_state);
}
ixl1 at pci5 dev 0 function 1 "Intel X710 SFP+" rev 0x02: port 1, FW 6.0.48442
API 1.7, msix, 8 queues, address 40:a6:b7:6e:ad:a1
witness: lock_object uninitialized: 0xffff8000002df808
Starting stack trace...
witness_checkorder(ffff8000002df808,9,0,ffff8000002df808,e2333bf47b0dd339,ffffffff8247aff0)
at witness_checkorder+0xb1
mtx_enter(ffff8000002df7f8,ffff8000002df7f8,526c9cbeb61c34a2,ffff8000002df000,0,0)
at mtx_enter+0x38
ixl_set_link_status(ffff8000002df000,ffffffff8299c5e0,391863f80fb351e5,38700083,c6904,c6804)
at ixl_set_link_status+0x15c
ixl_attach(ffff8000000c1900,ffff8000002df000,ffffffff8299c700,ffff8000000c1900,c84159f5901cc70f,ffff8000000c1900)
at ixl_attach+0x1f9d
config_attach(ffff8000000c1900,ffffffff824c6150,ffffffff8299c700,ffffffff81d75c20,112fd13f28adcf4b,80030100)
at config_attach+0x1f4
pci_probe_device(ffff8000000c1900,80030100,0,0,e75e9768b7491c85,0) at
pci_probe_device+0x4f0
pci_enumerate_bus(ffff8000000c1900,0,0,ffff8000000c1900,bd3c6014bc8a4290,ffff8000002d8400)
at pci_enumerate_bus+0x189
config_attach(ffff8000002d8400,ffffffff824c54d8,ffffffff8299c920,ffffffff81368aa0,112fd13f284cd40e,ffffffff8299ca90)
at config_attach+0x1f4
ppbattach(ffff8000000c1200,ffff8000002d8400,ffffffff8299ca90,ffff8000000c1200,20134e0abec38d14,ffff8000000c1200)
at ppbattach+0x755
config_attach(ffff8000000c1200,ffffffff824c5dd0,ffffffff8299ca90,ffffffff81d75c20,112fd13f28adcf4b,80002800)
at config_attach+0x1f4
pci_probe_device(ffff8000000c1200,80002800,0,0,e75e9768b7491c85,0) at
pci_probe_device+0x4f0
pci_enumerate_bus(ffff8000000c1200,0,0,ffff8000000c1200,bd3c6014bc8a4290,ffff80000002e280)
at pci_enumerate_bus+0x189
config_attach(ffff80000002e280,ffffffff824c54d8,ffffffff8299cca8,ffffffff81abbfc0,112fd13f28d1128b,ffff800000086c00)
at config_attach+0x1f4
acpipci_attach_bus(ffff80000002e280,ffff800000086c00,55efbcc160c143c3,2,ffff80000002e280,ffffffff824cc708)
at acpipci_attach_bus+0x1b0
acpipci_attach_busses(ffff80000002e280,ffff80000002e280,d29eed02be6a9f22,ffffffff8299cdc8,ffff80000002e280,0)
at acpipci_attach_busses+0x5d
mainbus_attach(0,ffff80000002e280,0,0,3a41ac4af0340abc,0) at
mainbus_attach+0x1c7
config_attach(0,ffffffff824c5008,0,0,112fd13f28cbfd0c,0) at config_attach+0x1f4
cpu_configure(995cb44cafcc5808,0,0,ffff800000031000,ffffffff81b15027,ffffffff8299cf10)
at cpu_configure+0x37
main(0,0,0,1,ffffffff81de9fcc,ffffffff8299cf40) at main+0x3bc
