Page MenuHomeFreeBSD

vtnet: Pre-allocate debugnet data immediately
ClosedPublic

Authored by kp on Jan 7 2020, 10:06 PM.

Details

Summary

Don't wait until the vtnet_debugnet_init() call happens, because at that
point we might already have allocated something from
vtnet_tx_header_zone.

Some systems showed this panic:

vtnet0: link state changed to UP
panic: keg vtnet_tx_hdr initialization after use.
cpuid = 5
time = 1578427700
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe004db427f0
vpanic() at vpanic+0x17e/frame 0xfffffe004db42850
panic() at panic+0x43/frame 0xfffffe004db428b0
uma_zone_reserve() at uma_zone_reserve+0xf6/frame 0xfffffe004db428f0
vtnet_debugnet_init() at vtnet_debugnet_init+0x77/frame 0xfffffe004db42930
debugnet_any_ifnet_update() at debugnet_any_ifnet_update+0x42/frame 0xfffffe004db42980
do_link_state_change() at do_link_state_change+0x1b3/frame 0xfffffe004db429d0
taskqueue_run_locked() at taskqueue_run_locked+0x178/frame 0xfffffe004db42a30
taskqueue_run() at taskqueue_run+0x4d/frame 0xfffffe004db42a50
ithread_loop() at ithread_loop+0x1d6/frame 0xfffffe004db42ab0
fork_exit() at fork_exit+0x80/frame 0xfffffe004db42af0
fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe004db42af0

  • trap 0, rip = 0, rsp = 0, rbp = 0 ---

KDB: enter: panic
[ thread pid 12 tid 100011 ]
Stopped at kdb_enter+0x37: movq $0,0x1084eb6(%rip)
db>

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 28524
Build 26577: arc lint + arc unit

Event Timeline

kp created this revision.Jan 7 2020, 10:06 PM
kp added reviewers: cem, markj.Jan 7 2020, 10:09 PM
kp set the repository for this revision to rS FreeBSD src repository.
cem accepted this revision.Jan 7 2020, 10:22 PM

This seems fine to me, although I'm not super familiar with vtnet / mbufs / uma. I think it might also be reasonable for uma_zone_reserve / uma_prealloc to work on zones that have already been allocated from? Maybe I misunderstand how they work.

Hopefully Mark can take a look as well.

This revision is now accepted and ready to land.Jan 7 2020, 10:22 PM
kp added a comment.Jan 7 2020, 10:46 PM
In D23073#505713, @cem wrote:

This seems fine to me, although I'm not super familiar with vtnet / mbufs / uma. I think it might also be reasonable for uma_zone_reserve / uma_prealloc to work on zones that have already been allocated from? Maybe I misunderstand how they work.

uma_zone_reserve() asserts that the keg is cold (no allocations done from it). I can only assume there's a good reason for that, but I can't say that I know why.

markj added a comment.Jan 7 2020, 11:09 PM
In D23073#505746, @kp wrote:
In D23073#505713, @cem wrote:

This seems fine to me, although I'm not super familiar with vtnet / mbufs / uma. I think it might also be reasonable for uma_zone_reserve / uma_prealloc to work on zones that have already been allocated from? Maybe I misunderstand how they work.

uma_zone_reserve() asserts that the keg is cold (no allocations done from it). I can only assume there's a good reason for that, but I can't say that I know why.

Basically because kegs now use per-domain locking, and the reservation is global. Requiring that the keg be cold ensures that you don't have to worry about synchronization.

I don't object to this change, but it has the disadvantage that it reserves memory even when debugnet is not configured on the interface. Another solution would be to address the XXX comment and introduce a separate zone specifically for debugnet.

Please go ahead and commit it for now. It prevents the obnoxious boot-time panic in vtnet related to the recent change in UMA semantics. The XXX comment is just as puntable now as it was when it was written ;-).

markj accepted this revision.Jan 8 2020, 1:23 AM
In D23073#505788, @cem wrote:

Please go ahead and commit it for now. It prevents the obnoxious boot-time panic in vtnet related to the recent change in UMA semantics. The XXX comment is just as puntable now as it was when it was written ;-).

Sure. I will address it after this diff goes in.

This revision was automatically updated to reflect the committed changes.