Date: Wed, 19 Jan 2011 19:20:48 +0300 From: Sergey Kandaurov <pluknet@gmail.com> To: John Baldwin <jhb@freebsd.org> Cc: freebsd-current@freebsd.org, Robert Watson <rwatson@freebsd.org> Subject: Re: uma_zalloc_arg: zone "256" with non-sleepable exclusive rw ifnet_rw @ /usr/src/sys/net/if.c:414 Message-ID: <AANLkTi=x4=3XB=sRrwds4qTuZxk7PBiTBy=3J8Bowy-5@mail.gmail.com> In-Reply-To: <201101191017.12144.jhb@freebsd.org> References: <AANLkTi=dZ4vOSx82Wp%2BqAwmT97mucqwtf6dDu4pnOCR2@mail.gmail.com> <201101181404.26768.jhb@freebsd.org> <AANLkTimj_6VR2gwb4oxhiFYvV%2B8gdrU2-V8b%2BE5_6%2BXT@mail.gmail.com> <201101191017.12144.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 19 January 2011 18:17, John Baldwin <jhb@freebsd.org> wrote:
> On Wednesday, January 19, 2011 9:51:59 am Sergey Kandaurov wrote:
>> On 18 January 2011 22:04, John Baldwin <jhb@freebsd.org> wrote:
>> > On Tuesday, January 18, 2011 1:22:24 pm Sergey Kandaurov wrote:
>> >> On 18 January 2011 17:54, John Baldwin <jhb@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 th=
e
>> >> > allocation after calling if_grow()? =A0This compiles, but I haven't
> booted
>> > it
>> >> > yet:
>> >>
>> >> vnet_if_init() calls if_grow() without lock.
>> >
>> > So it does. :( =A0I'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 loc=
ks.
>> 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 =3D 0
>> KDB: enter: panic
>> [ thread pid 0 tid 100000 ]
>> Stopped at =A0 =A0 =A0kdb_enter+0x3d: movq =A0 =A0$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
>> =A0 =A0ifnet =A0 =A0 =A0 =A0 =A0 =A02 =A0 =A0 =A01048578K =A0 =A0 =A0 =
=A0 =A0 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 th=
at
> was a bug certainly. :) =A0The issue is that we should only grow if the n=
ew
> index would exceed V_if_indexlim, not V_if_index and that should fix it. =
=A0For
> 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
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> --- if.c =A0 =A0 =A0 =A0(revision 217544)
> +++ if.c =A0 =A0 =A0 =A0(working copy)
> @@ -266,6 +266,7 @@ ifindex_alloc_locked(u_short *idxp)
>
> =A0 =A0 =A0 =A0IFNET_WLOCK_ASSERT();
>
> +retry:
> =A0 =A0 =A0 =A0/*
> =A0 =A0 =A0 =A0 * Try to find an empty slot below V_if_index. =A0If we fa=
il, take the
> =A0 =A0 =A0 =A0 * next slot.
> @@ -278,10 +279,12 @@ ifindex_alloc_locked(u_short *idxp)
> =A0 =A0 =A0 =A0/* Catch if_index overflow. */
> =A0 =A0 =A0 =A0if (idx < 1)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return (ENOSPC);
> + =A0 =A0 =A0 if (idx >=3D V_if_indexlim) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if_grow();
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto retry;
> + =A0 =A0 =A0 }
> =A0 =A0 =A0 =A0if (idx > V_if_index)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0V_if_index =3D idx;
> - =A0 =A0 =A0 if (V_if_index >=3D V_if_indexlim)
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if_grow();
> =A0 =A0 =A0 =A0*idxp =3D idx;
> =A0 =A0 =A0 =A0return (0);
> =A0}
> @@ -351,10 +354,12 @@ vnet_if_init(const void *unused __unused)
>
> =A0 =A0 =A0 =A0TAILQ_INIT(&V_ifnet);
> =A0 =A0 =A0 =A0TAILQ_INIT(&V_ifg_head);
> + =A0 =A0 =A0 IFNET_WLOCK();
> =A0 =A0 =A0 =A0if_grow(); =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 =A0 =A0/* create initial table */
> + =A0 =A0 =A0 IFNET_WUNLOCK();
> =A0 =A0 =A0 =A0vnet_if_clone_init();
> =A0}
> -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=
,
> =A0 =A0 NULL);
>
> =A0/* ARGSUSED*/
> @@ -365,7 +370,7 @@ if_init(void *dummy __unused)
> =A0 =A0 =A0 =A0IFNET_LOCK_INIT();
> =A0 =A0 =A0 =A0if_clone_init();
> =A0}
> -SYSINIT(interfaces, SI_SUB_INIT_IF, SI_ORDER_SECOND, if_init, NULL);
> +SYSINIT(interfaces, SI_SUB_INIT_IF, SI_ORDER_FIRST, if_init, NULL);
>
>
> =A0#ifdef VIMAGE
> @@ -385,16 +390,25 @@ VNET_SYSUNINIT(vnet_if_uninit, SI_SUB_INIT_IF, SI_
> =A0static void
> =A0if_grow(void)
> =A0{
> + =A0 =A0 =A0 int oldlim;
> =A0 =A0 =A0 =A0u_int n;
> =A0 =A0 =A0 =A0struct ifindex_entry *e;
>
> - =A0 =A0 =A0 V_if_indexlim <<=3D 1;
> - =A0 =A0 =A0 n =3D V_if_indexlim * sizeof(*e);
> + =A0 =A0 =A0 IFNET_WLOCK_ASSERT();
> + =A0 =A0 =A0 oldlim =3D V_if_indexlim;
> + =A0 =A0 =A0 IFNET_WUNLOCK();
> + =A0 =A0 =A0 n =3D (oldlim << 1) * sizeof(*e);
> =A0 =A0 =A0 =A0e =3D malloc(n, M_IFNET, M_WAITOK | M_ZERO);
> + =A0 =A0 =A0 IFNET_WLOCK();
> + =A0 =A0 =A0 if (V_if_indexlim !=3D oldlim) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 free(e, M_IFNET);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;
> + =A0 =A0 =A0 }
> =A0 =A0 =A0 =A0if (V_ifindex_table !=3D NULL) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0memcpy((caddr_t)e, (caddr_t)V_ifindex_tabl=
e, n/2);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0free((caddr_t)V_ifindex_table, M_IFNET);
> =A0 =A0 =A0 =A0}
> + =A0 =A0 =A0 V_if_indexlim <<=3D 1;
> =A0 =A0 =A0 =A0V_ifindex_table =3D e;
> =A0}
This patch works for me. Thank you!
--=20
wbr,
pluknet
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTi=x4=3XB=sRrwds4qTuZxk7PBiTBy=3J8Bowy-5>