Date: Tue, 18 Jan 2011 14:04:26 -0500 From: John Baldwin <jhb@freebsd.org> To: Sergey Kandaurov <pluknet@gmail.com> 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: <201101181404.26768.jhb@freebsd.org> In-Reply-To: <AANLkTi=DtUa5%2BfhOrOW_N11rn6WVp8fHZvL%2BOE6n-S6c@mail.gmail.com> References: <AANLkTi=dZ4vOSx82Wp%2BqAwmT97mucqwtf6dDu4pnOCR2@mail.gmail.com> <201101180954.46903.jhb@freebsd.org> <AANLkTi=DtUa5%2BfhOrOW_N11rn6WVp8fHZvL%2BOE6n-S6c@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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 the
> > allocation after calling if_grow()? This compiles, but I haven't booted
it
> > yet:
> >
> > Index: if.c
> > ===================================================================
> > --- if.c (revision 217400)
> > +++ if.c (working copy)
> > @@ -266,6 +266,7 @@ 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.
> > @@ -278,10 +279,11 @@ ifindex_alloc_locked(u_short *idxp)
> > /* Catch if_index overflow. */
> > if (idx < 1)
> > return (ENOSPC);
> > - if (idx > V_if_index)
> > - V_if_index = idx;
> > - if (V_if_index >= V_if_indexlim)
> > + if (idx > V_if_index) {
> > if_grow();
> > + goto retry;
> > + }
> > + V_if_index = idx;
> > *idxp = idx;
> > return (0);
> > }
> > @@ -385,16 +387,25 @@ 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;
> > }
>
> vnet_if_init() calls if_grow() without lock.
So it does. :( I've added locking to the sysinit to handle this:
Index: if.c
===================================================================
--- if.c (revision 217544)
+++ if.c (working copy)
@@ -266,6 +266,7 @@ 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.
@@ -278,10 +279,11 @@ ifindex_alloc_locked(u_short *idxp)
/* Catch if_index overflow. */
if (idx < 1)
return (ENOSPC);
- if (idx > V_if_index)
- V_if_index = idx;
- if (V_if_index >= V_if_indexlim)
+ if (idx > V_if_index) {
if_grow();
+ goto retry;
+ }
+ V_if_index = idx;
*idxp = idx;
return (0);
}
@@ -351,7 +353,9 @@ 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,
@@ -385,16 +389,25 @@ 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;
}
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201101181404.26768.jhb>