From owner-freebsd-current@FreeBSD.ORG Tue Jan 18 19:04:28 2011 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id ED895106566C; Tue, 18 Jan 2011 19:04:28 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id BAE098FC08; Tue, 18 Jan 2011 19:04:28 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 4F63F46B45; Tue, 18 Jan 2011 14:04:28 -0500 (EST) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 410158A009; Tue, 18 Jan 2011 14:04:27 -0500 (EST) From: John Baldwin To: Sergey Kandaurov Date: Tue, 18 Jan 2011 14:04:26 -0500 User-Agent: KMail/1.13.5 (FreeBSD/7.4-CBSD-20110107; KDE/4.4.5; amd64; ; ) References: <201101180954.46903.jhb@freebsd.org> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201101181404.26768.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.6 (bigwig.baldwin.cx); Tue, 18 Jan 2011 14:04:27 -0500 (EST) X-Virus-Scanned: clamav-milter 0.96.3 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-1.9 required=4.2 tests=BAYES_00 autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on bigwig.baldwin.cx Cc: freebsd-current@freebsd.org, Robert Watson Subject: Re: uma_zalloc_arg: zone "256" with non-sleepable exclusive rw ifnet_rw @ /usr/src/sys/net/if.c:414 X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 18 Jan 2011 19:04:29 -0000 On Tuesday, January 18, 2011 1:22:24 pm Sergey Kandaurov wrote: > On 18 January 2011 17:54, John Baldwin 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