Re: uma_zalloc_arg: zone "256" with non-sleepable exclusive rw ifnet_rw @ /usr/src/sys/net/if.c:414

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:
> >
> > Index: if.c
> > ===================================================================
> > --- if.c        (revision 217400)
> > +++ 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,11 _at__at_ 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);
> >  }
> > _at__at_ -385,16 +387,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;
> >  }
> 
> 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)
_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,11 _at__at_ 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);
 }
_at__at_ -351,7 +353,9 _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,
_at__at_ -385,16 +389,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;
 }
 

-- 
John Baldwin