On Tue, Jan 02, 2024 at 08:29:50PM +0300, Vitaliy Makkoveev wrote:
> > On 2 Jan 2024, at 20:16, Alexander Bluhm <[email protected]> wrote:
> >
> > On Tue, Jan 02, 2024 at 03:45:10PM +0000, Stuart Henderson wrote:
> >> panic: rw_enter: netlock locking against myself
> >> Stopped at db_enter+0x14: popq %rbp
> >> TID PID UID PRFLAGS PFLAGS CPU COMMAND
> >> *388963 11506 755 0x2 0 0 snmpbulkwalk
> >> 320140 61046 0 0x14000 0x200 1 reaper
> >> db_enter() at db_enter+0x14
> >> panic(ffffffff820ebaaa) at panic+0xc3
> >> rw_enter(ffffffff824bd2d0,2) at rw_enter+0x252
> >> hv_kvp(ffffffff8248d738) at hv_kvp+0x705
> >> hv_event_intr(ffff80000008d000) at hv_event_intr+0x1ab
> >> hv_intr() at hv_intr+0x1a
> >> Xresume_hyperv_upcall() at Xresume_hyperv_upcall+0x27
> >> Xspllower() at Xspllower+0x1d
> >> task_add(ffff800000036200,ffff8000001533c0) at task_add+0x83
> >> if_enqueue_ifq(ffff800000153060,fffffd80f6642900) at if_enqueue_ifq+0x6f
> >> ether_output(ffff800000153060,fffffd80f6642900,fffffd80c8ef5c78,fffffd811e6414d8)
> >> at ether_output+0x82
> >> if_output_tso(ffff800000153060,ffff800025395d08,fffffd80c8ef5c78,fffffd811e6414d8,5dc)
> >> at if_output_tso+0xe1
> >> ip_output(fffffd80f6642900,0,fffffd80c8ef5c68,0,0,fffffd80c8ef5bf0,1da98542b9d8f733)
> >> at ip_output+0x817
> >> udp_output(fffffd80c8ef5bf0,fffffd80b468b400,fffffd80ac452c00,0) at
> >> udp_output+0x3be
> >> end trace frame: 0xffff800025395ee0, count: 0
> >> https://www.openbsd.org/ddb.html describes the minimum info required in bug
> >> reports. Insufficient info makes it difficult to find and fix bugs.
> >> ddb{0}> Stopped at x86_ipi_db+0x16: leave
> >> x86_ipi_db(ffff80002250dff0) at x86_ipi_db+0x16
> >> x86_ipi_handler() at x86_ipi_handler+0x80
> >> Xresume_lapic_ipi() at Xresume_lapic_ipi+0x27
> >> _kernel_lock() at _kernel_lock+0xc2
> >> reaper(ffff80002473c008) at reaper+0x133
> >> end trace frame: 0x0, count: 10
> >
> > It is caused by this commit.
> >
> > ----------------------------
> > revision 1.20
> > date: 2023/09/23 13:01:12; author: mvs; state: Exp; lines: +5 -9;
> > commitid:
> > Gj5WdkySyube1RkO;
> > Use shared netlock to protect if_list and ifa_list walkthrough and ifnet
> > data access within kvp_get_ip_info().
> >
> > ok bluhm
> > ----------------------------
> >
> > Net lock must not be acquired in interrupt context. It is wrong
> > that hv tries to perform network operations from interrupt context.
> > Actually it does not do much networking, it is just reads some
> > interface addresses. The kernel lock that was used before is also
> > questionable. Maybe some writers do not hold it anymore. There
> > was much conversion from kernel to net lock.
> >
> > bluhm
>
> This is not interrupt context. tsleep_nsec() points that. The
> problem lays in Xresume_hyperv_upcall() which can run this
> from interrupt context.
>
> hv_wait(struct hv_softc *sc, int (*cond)(struct hv_softc *, struct hv_msg *),
> struct hv_msg *msg, void *wchan, const char *wmsg)
> {
> int s;
>
> KASSERT(cold ? msg->msg_flags & MSGF_NOSLEEP : 1);
>
> while (!cond(sc, msg)) {
> if (msg->msg_flags & MSGF_NOSLEEP) {
> delay(1000);
> s = splnet();
> hv_intr();
> splx(s);
> } else {
> tsleep_nsec(wchan, PRIBIO, wmsg ? wmsg : "hvwait",
> USEC_TO_NSEC(1000));
> }
> }
> }
>
The diff to revert my commit.
Index: sys/dev/pv/hypervic.c
===================================================================
RCS file: /cvs/src/sys/dev/pv/hypervic.c,v
retrieving revision 1.20
diff -u -p -r1.20 hypervic.c
--- sys/dev/pv/hypervic.c 23 Sep 2023 13:01:12 -0000 1.20
+++ sys/dev/pv/hypervic.c 2 Jan 2024 17:47:15 -0000
@@ -846,7 +846,7 @@ kvp_get_ip_info(struct hv_kvp *kvp, cons
struct sockaddr_in6 *sin6, sa6;
uint8_t enaddr[ETHER_ADDR_LEN];
uint8_t ipaddr[INET6_ADDRSTRLEN];
- int i, j, lo, hi, af;
+ int i, j, lo, hi, s, af;
/* Convert from the UTF-16LE string format to binary */
for (i = 0, j = 0; j < ETHER_ADDR_LEN; i += 6) {
@@ -870,14 +870,16 @@ kvp_get_ip_info(struct hv_kvp *kvp, cons
return (-1);
}
- NET_LOCK_SHARED();
+ KERNEL_LOCK();
+ s = splnet();
TAILQ_FOREACH(ifp, &ifnetlist, if_list) {
if (!memcmp(LLADDR(ifp->if_sadl), enaddr, ETHER_ADDR_LEN))
break;
}
if (ifp == NULL) {
- NET_UNLOCK_SHARED();
+ splx(s);
+ KERNEL_UNLOCK();
return (-1);
}
@@ -917,7 +919,8 @@ kvp_get_ip_info(struct hv_kvp *kvp, cons
else if (ifa6ll != NULL)
ifa = ifa6ll;
else {
- NET_UNLOCK_SHARED();
+ splx(s);
+ KERNEL_UNLOCK();
return (-1);
}
}
@@ -953,7 +956,8 @@ kvp_get_ip_info(struct hv_kvp *kvp, cons
break;
}
- NET_UNLOCK_SHARED();
+ splx(s);
+ KERNEL_UNLOCK();
return (0);
}
