On 19 January 2011 18:17, John Baldwin <jhb_at_freebsd.org> wrote: > On Wednesday, January 19, 2011 9:51:59 am Sergey Kandaurov wrote: >> On 18 January 2011 22:04, John Baldwin <jhb_at_freebsd.org> wrote: >> > On Tuesday, January 18, 2011 1:22:24 pm Sergey Kandaurov wrote: >> >> On 18 January 2011 17:54, John Baldwin <jhb_at_freebsd.org> wrote: >> >> > On Monday, January 17, 2011 12:55:26 pm Sergey Kandaurov wrote: >> >> >> Hi, >> >> >> >> >> >> I see this "malloc with non-sleepable" on current during boot. >> >> >> It's strange that I don't see it if I boot via pxe/nfs. >> >> >> >> >> >> if_alloc() calls ifindex_alloc_locked() under IFNET_WLOCK() which >> >> >> might call if_grow(). >> >> >> Looks like a regression from r196553. >> >> > >> >> > I'm guessing that ifindex_alloc() should drop the lock and retry the >> >> > allocation after calling if_grow()? This compiles, but I haven't > booted >> > it >> >> > yet: >> >> >> >> vnet_if_init() calls if_grow() without lock. >> > >> > So it does. :( I've added locking to the sysinit to handle this: >> >> Seems a bit more work there. The vnet_if_init() sysinit cannot use ifnet >> locking since it runs before another sysinit initialized those ifnet locks. >> On the other side it's safe to call if_grow() from vnet_if_init() w/o > locking: >> >> db> bt >> Tracing pid 0 tid 100000 td 0xffffffff80ccffc0 >> _sx_xlock_hard() at _sx_xlock_hard+0xa0 >> _sx_xlock() at _sx_xlock+0xbb >> vnet_if_init() at vnet_if_init+0x4a >> mi_startup() at mi_startup+0x77 >> btext() at btext+0x2c >> >> After said certain locking changes I came to the next problem. >> A modified ifindex_alloc_locked() function doesn't update/increment > V_if_index >> in successful if_grow() case, so it ends up in quick memory exhaustion: >> >> bce0: <Broadcom NetXtreme II BCM5709 1000Base-T (C0)> mem >> 0x92000000-0x93ffffff irq 28 at device 0.0 on pci4 >> panic: kmem_malloc(-2147483648): kmem_map too small: 1110716416 total > allocated >> cpuid = 0 >> KDB: enter: panic >> [ thread pid 0 tid 100000 ] >> Stopped at kdb_enter+0x3d: movq $0,0x700140(%rip) >> db> bt >> Tracing pid 0 tid 100000 td 0xffffffff80ccffc0 >> kdb_enter() at kdb_enter+0x3d >> panic() at panic+0x180 >> kmem_malloc() at kmem_malloc+0x25d >> uma_large_malloc() at uma_large_malloc+0x4a >> malloc() at malloc+0x15d >> if_grow() at if_grow+0x98 >> if_alloc() at if_alloc+0xd8 >> bce_attach() at bce_attach+0x18de >> [...] >> > show malloc >> ifnet 2 1048578K 25 >> >> So I added V_if_index increment into if_grow() itself. >> At least it boots now :) > > Ah, the increment belongs in ifindex_alloc_locked() not if_grow(), but that > was a bug certainly. :) The issue is that we should only grow if the new > index would exceed V_if_indexlim, not V_if_index and that should fix it. For > the locking, I just swapped the order of the SYSINIT's so that the locks are > initialized before the various data structures. > > Index: if.c > =================================================================== > --- if.c (revision 217544) > +++ if.c (working copy) > _at__at_ -266,6 +266,7 _at__at_ ifindex_alloc_locked(u_short *idxp) > > IFNET_WLOCK_ASSERT(); > > +retry: > /* > * Try to find an empty slot below V_if_index. If we fail, take the > * next slot. > _at__at_ -278,10 +279,12 _at__at_ ifindex_alloc_locked(u_short *idxp) > /* Catch if_index overflow. */ > if (idx < 1) > return (ENOSPC); > + if (idx >= V_if_indexlim) { > + if_grow(); > + goto retry; > + } > if (idx > V_if_index) > V_if_index = idx; > - if (V_if_index >= V_if_indexlim) > - if_grow(); > *idxp = idx; > return (0); > } > _at__at_ -351,10 +354,12 _at__at_ vnet_if_init(const void *unused __unused) > > TAILQ_INIT(&V_ifnet); > TAILQ_INIT(&V_ifg_head); > + IFNET_WLOCK(); > if_grow(); /* create initial table */ > + IFNET_WUNLOCK(); > vnet_if_clone_init(); > } > -VNET_SYSINIT(vnet_if_init, SI_SUB_INIT_IF, SI_ORDER_FIRST, vnet_if_init, > +VNET_SYSINIT(vnet_if_init, SI_SUB_INIT_IF, SI_ORDER_SECOND, vnet_if_init, > NULL); > > /* ARGSUSED*/ > _at__at_ -365,7 +370,7 _at__at_ if_init(void *dummy __unused) > IFNET_LOCK_INIT(); > if_clone_init(); > } > -SYSINIT(interfaces, SI_SUB_INIT_IF, SI_ORDER_SECOND, if_init, NULL); > +SYSINIT(interfaces, SI_SUB_INIT_IF, SI_ORDER_FIRST, if_init, NULL); > > > #ifdef VIMAGE > _at__at_ -385,16 +390,25 _at__at_ VNET_SYSUNINIT(vnet_if_uninit, SI_SUB_INIT_IF, SI_ > static void > if_grow(void) > { > + int oldlim; > u_int n; > struct ifindex_entry *e; > > - V_if_indexlim <<= 1; > - n = V_if_indexlim * sizeof(*e); > + IFNET_WLOCK_ASSERT(); > + oldlim = V_if_indexlim; > + IFNET_WUNLOCK(); > + n = (oldlim << 1) * sizeof(*e); > e = malloc(n, M_IFNET, M_WAITOK | M_ZERO); > + IFNET_WLOCK(); > + if (V_if_indexlim != oldlim) { > + free(e, M_IFNET); > + return; > + } > if (V_ifindex_table != NULL) { > memcpy((caddr_t)e, (caddr_t)V_ifindex_table, n/2); > free((caddr_t)V_ifindex_table, M_IFNET); > } > + V_if_indexlim <<= 1; > V_ifindex_table = e; > } This patch works for me. Thank you! -- wbr, pluknetReceived on Wed Jan 19 2011 - 15:20:50 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:10 UTC