lagg: Remove redundant rmlock

[shurd]

Summary

LAGG_WLOCK was completely contained inside LAGG_XLOCK, and
the rmlock is triggering LORs in the multicast stuff. Switch LAGG_RLOCK
to LAGG_SLOCK.

Test Plan

Test for performance regressions and check for LAGG_SLOCK recursion.

[shurd]

Rather than remove LAGG_WLOCK_ASSERT()s, convert to LAGG_XLOCK_ASSERT()

[mmacy]

sys/net/if_lagg.c

1654 ↗

(On Diff #42282)

I don't think we want to be acquiring sleep locks in the data path. A more substantive refactoring is in order.

[mmacy]

What we need to do is not be traversing the list in the data path and just hold a reference from an array to all the ports. At that point we're safe just using an sx lock. I think we can handle this with EBR + SX. I'll hack something up with subr_epoch

[shurd]

Yeah, all the LAGG_RLOCK -> LAGG_SLOCK conversions need to be fixed up... right now it's a LOR generator for sure.

[shurd]

Ressurect the rwlock, but only to protect the new sc_slowpath boolean.
If the slowpath is currently running, DELAY(1) until it's not. Still a
bit of a hack, but now it doesn't (shouldn't) cause LORs.

[shurd]

Protect against multiple slowpath entries.

[shurd]

Use bool and true/false rather than _Bool and 1/0

[melifaro]

Would you mind describe the proposed locking model somewhere in the file explicitly?
In particular, 1) what does _XLOCK or _SLOCK locks protect? 2) How does the sc_slowpath work? 3) What is "right" lock order?
It would be beneficial to describe some examples of problematic potential LORs as well.

sys/net/if_lagg.c

1658 ↗

(On Diff #42324)

Why do you think it is possible to have DELAY(1) on the datapath?

sys/net/if_lagg.h

272 ↗

(On Diff #42324)

These primitives seem to be used in the control path. Why not convert them to functions?
This kind of spaghetti is unmaintainable.

[shurd]

Address feedback.

[shurd]

sys/net/if_lagg.c

1658 ↗

(On Diff #42324)

Fixed (lagg_rlock() uses cpu_spinwait() now).

sys/net/if_lagg.h

272 ↗

(On Diff #42324)

Done. I also did the lagg_rlock and lagg_runlock() as inline.

[shurd]

Destroy rm lock, clean up spurious changes.

[shurd]

Use the new epoch based reclamation API. Now the hot paths will not
block at all, and the sx lock is used for the softc data.

Obtained from mmacy@, I'm testing now.

[mmacy]

epoch(9) documentation is forthcoming but in the meantime some clarification of semantics:

epoch does not have lock ordering issues its only restrictions are:

• While in an epoch section one can't do a preemptible epoch_enter on a different epoch_t
• No sleeping or sx lock acquisition in an epoch section
• One must use safe list traversal and removal routines
• Object destruction must follow a grace period

[sbruno]

dumbbell@ has reported that this fixes his current problems with lagg(4) panicing on his hosts.

[shurd]

Update patch to latest head

[dumbbell]

Thank you for the quick fix!

Since then I hit two panics on two laptops with the same kernel, but I'm not sure they are related to lagg(4).

The first one where lagg(4) is used with one em(4) only underneath:

Fatal trap 12: page fault while in kernel mode
cpuid = 7; apic id = 07
fault virtual address   = 0x10
fault code              = supervisor read data, page not present
instruction pointer     = 0x20:0xffffffff80d1c876
stack pointer           = 0x28:0xfffffe000042d650
frame pointer           = 0x28:0xfffffe000042d670
code segment            = base 0x0, limit 0xfffff, type 0x1b
                        = DPL 0, pres 1, long 1, def32 0, gran 1
processor eflags        = interrupt enabled, resume, IOPL = 0
current process         = 0 (if_io_tqg_7)
trap number             = 12
panic: page fault
cpuid = 7
time = 1526247877
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe000042d300
vpanic() at vpanic+0x1a3/frame 0xfffffe000042d360
panic() at panic+0x43/frame 0xfffffe000042d3c0
trap_fatal() at trap_fatal+0x35f/frame 0xfffffe000042d410
trap_pfault() at trap_pfault+0x49/frame 0xfffffe000042d470
trap() at trap+0x2e3/frame 0xfffffe000042d580
calltrap() at calltrap+0x8/frame 0xfffffe000042d580
--- trap 0xc, rip = 0xffffffff80d1c876, rsp = 0xfffffe000042d650, rbp = 0xfffffe000042d670 ---
inm_lookup() at inm_lookup+0x46/frame 0xfffffe000042d670
igmp_input() at igmp_input+0x48f/frame 0xfffffe000042d730
ip_input() at ip_input+0x145/frame 0xfffffe000042d790
netisr_dispatch_src() at netisr_dispatch_src+0xd6/frame 0xfffffe000042d7e0
ether_demux() at ether_demux+0x163/frame 0xfffffe000042d810
ether_nh_input() at ether_nh_input+0x363/frame 0xfffffe000042d870
netisr_dispatch_src() at netisr_dispatch_src+0xd6/frame 0xfffffe000042d8c0
ether_input() at ether_input+0x54/frame 0xfffffe000042d8f0
_task_fn_rx() at _task_fn_rx+0xaf0/frame 0xfffffe000042d9e0
gtaskqueue_run_locked() at gtaskqueue_run_locked+0x144/frame 0xfffffe000042da40
gtaskqueue_thread_loop() at gtaskqueue_thread_loop+0x98/frame 0xfffffe000042da70
fork_exit() at fork_exit+0x83/frame 0xfffffe000042dab0
fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe000042dab0
--- trap 0, rip = 0, rsp = 0, rbp = 0 ---
Uptime: 19s
Dumping 788 out of 16030 MB:..3%..11%..21%..31%..41%..51%..61%..72%..82%..92%

__curthread () at ./machine/pcpu.h:231
231     ./machine/pcpu.h: No such file or directory.
(kgdb) #0  __curthread () at ./machine/pcpu.h:231
#1  doadump (textdump=1)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/kern/kern_shutdown.c:366
#2  0xffffffff80b9e1e0 in kern_reboot (howto=260)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/kern/kern_shutdown.c:446
#3  0xffffffff80b9e673 in vpanic (fmt=<optimized out>, ap=0xfffffe000042d3a0)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/kern/kern_shutdown.c:863
#4  0xffffffff80b9e463 in panic (fmt=<unavailable>)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/kern/kern_shutdown.c:790
#5  0xffffffff810658bf in trap_fatal (frame=0xfffffe000042d590, eva=16)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/amd64/amd64/trap.c:880
#6  0xffffffff81065919 in trap_pfault (frame=0xfffffe000042d590, usermode=0)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/amd64/amd64/trap.c:717
#7  0xffffffff810650f3 in trap (frame=0xfffffe000042d590)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/amd64/amd64/trap.c:418
#8  <signal handler called>
#9  inm_lookup_locked (ifp=<optimized out>, ina=...)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/netinet/in_mcast.c:349
#10 inm_lookup (ifp=<optimized out>, ina=...)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/netinet/in_mcast.c:368
#11 0xffffffff80d1476f in igmp_input_v2_report (ifp=<optimized out>, 
    igmp=<optimized out>, ip=<optimized out>)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/netinet/igmp.c:1364
#12 igmp_input (mp=0xfffffe000042d760, offp=0xfffffe000042d75c, proto=2)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/netinet/igmp.c:1576
#13 0xffffffff80d29ad5 in ip_input (m=0x0)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/netinet/ip_input.c:825
#14 0xffffffff80cbf286 in netisr_dispatch_src (proto=1, 
    source=<optimized out>, m=0xfffff8001cbd9ec0)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/net/netisr.c:1122
#15 0xffffffff80ca4273 in ether_demux (ifp=0xfffff8002a974800, 
    m=0xfffff800035fb580)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/net/if_ethersubr.c:873
#16 0xffffffff80ca53f3 in ether_input_internal (ifp=0xfffff8002a974800, 
    m=0xfffff800035fb580)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/net/if_ethersubr.c:661
#17 ether_nh_input (m=<optimized out>)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/net/if_ethersubr.c:691
#18 0xffffffff80cbf286 in netisr_dispatch_src (proto=5, 
    source=<optimized out>, m=0xfffff8001cbd9ec0)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/net/netisr.c:1122
#19 0xffffffff80ca4674 in ether_input (ifp=0xfffff8000397a000, m=0x0)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/net/if_ethersubr.c:781
#20 0xffffffff80cb7140 in iflib_rxeof (rxq=<optimized out>, 
    budget=<optimized out>)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/net/iflib.c:2700
#21 _task_fn_rx (context=<optimized out>)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/net/iflib.c:3750
#22 0xffffffff80be6904 in gtaskqueue_run_locked (queue=0xfffff800030bd700)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/kern/subr_gtaskqueue.c:332
#23 0xffffffff80be6568 in gtaskqueue_thread_loop (arg=<optimized out>)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/kern/subr_gtaskqueue.c:507
#24 0xffffffff80b5f1c3 in fork_exit (
    callout=0xffffffff80be64d0 <gtaskqueue_thread_loop>, 
    arg=0xfffffe00047f90b0, frame=0xfffffe000042dac0)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/kern/kern_fork.c:1039
#25 <signal handler called>

The second one where lagg(4) is used with one em(4) and one iwm(4):

Fatal trap 12: page fault while in kernel mode
cpuid = 4; apic id = 04
fault virtual address   = 0x8000000
fault code              = supervisor write data, page not present
instruction pointer     = 0x20:0xffffffff833ddabb
stack pointer           = 0x28:0xfffffe008c4fe800
frame pointer           = 0x28:0xfffffe008c4fe8e0
code segment            = base 0x0, limit 0xfffff, type 0x1b
                        = DPL 0, pres 1, long 1, def32 0, gran 1
processor eflags        = interrupt enabled, resume, IOPL = 0
current process         = 12 (irq277: iwm0)
trap number             = 12
panic: page fault
cpuid = 4
time = 1526332705
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe008c4fe4b0
vpanic() at vpanic+0x1a3/frame 0xfffffe008c4fe510
panic() at panic+0x43/frame 0xfffffe008c4fe570
trap_fatal() at trap_fatal+0x35f/frame 0xfffffe008c4fe5c0
trap_pfault() at trap_pfault+0x49/frame 0xfffffe008c4fe620
trap() at trap+0x2e3/frame 0xfffffe008c4fe730
calltrap() at calltrap+0x8/frame 0xfffffe008c4fe730
--- trap 0xc, rip = 0xffffffff833ddabb, rsp = 0xfffffe008c4fe800, rbp = 0xfffffe008c4fe8e0 ---
iwm_intr() at iwm_intr+0x10ab/frame 0xfffffe008c4fe8e0
intr_event_execute_handlers() at intr_event_execute_handlers+0xe9/frame 0xfffffe008c4fe920
ithread_loop() at ithread_loop+0xe7/frame 0xfffffe008c4fe970
fork_exit() at fork_exit+0x83/frame 0xfffffe008c4fe9b0
fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe008c4fe9b0
--- trap 0, rip = 0, rsp = 0, rbp = 0 ---
Uptime: 3s
Dumping 848 out of 16221 MB:..2%..12%..21%..31%..42%..51%..61%..72%..82%..91%

__curthread () at ./machine/pcpu.h:231
231             __asm("movq %%gs:%1,%0" : "=r" (td)
(kgdb) #0  __curthread () at ./machine/pcpu.h:231
#1  doadump (textdump=1)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/kern/kern_shutdown.c:366
#2  0xffffffff80b9e1e0 in kern_reboot (howto=260)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/kern/kern_shutdown.c:446
#3  0xffffffff80b9e673 in vpanic (fmt=<optimized out>, ap=0xfffffe008c4fe550)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/kern/kern_shutdown.c:863
#4  0xffffffff80b9e463 in panic (fmt=<unavailable>)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/kern/kern_shutdown.c:790
#5  0xffffffff810658bf in trap_fatal (frame=0xfffffe008c4fe740, eva=134217728)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/amd64/amd64/trap.c:880
#6  0xffffffff81065919 in trap_pfault (frame=0xfffffe008c4fe740, usermode=0)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/amd64/amd64/trap.c:717
#7  0xffffffff810650f3 in trap (frame=0xfffffe008c4fe740)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/amd64/amd64/trap.c:418
#8  <signal handler called>
#9  iwm_intr (arg=<optimized out>)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/dev/iwm/if_iwm.c:5667
#10 0xffffffff80b61f99 in intr_event_execute_handlers (p=<optimized out>, 
    ie=0xfffff800035b4400)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/kern/kern_intr.c:1338
#11 0xffffffff80b62687 in ithread_execute_handlers (ie=<optimized out>, 
    p=<optimized out>)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/kern/kern_intr.c:1351
#12 ithread_loop (arg=0xfffff8000a024160)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/kern/kern_intr.c:1432
#13 0xffffffff80b5f1c3 in fork_exit (
    callout=0xffffffff80b625a0 <ithread_loop>, arg=0xfffff8000a024160, 
    frame=0xfffffe008c4fe9c0)
    at /home/dumbbell/Projects/freebsd/src/GIT/sys/kern/kern_fork.c:1039
#14 <signal handler called>

Both panics occured during boot, never after. Also, they occured once each in the past two days, despite numerous reboots.

Kernel is compiled from rS333556 and local modifications are:

• the patch to lagg(4) from this review
• the patch from D15070 (evdev)
• the patch from D15302 (vt(4))