From owner-freebsd-current@FreeBSD.ORG Fri Apr 4 00:03:10 2003 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 90A9137B401 for ; Fri, 4 Apr 2003 00:03:10 -0800 (PST) Received: from rootlabs.com (root.org [67.118.192.226]) by mx1.FreeBSD.org (Postfix) with SMTP id 1807D43FB1 for ; Fri, 4 Apr 2003 00:03:10 -0800 (PST) (envelope-from nate@rootlabs.com) Received: (qmail 15840 invoked by uid 1000); 4 Apr 2003 08:03:12 -0000 Date: Fri, 4 Apr 2003 00:03:12 -0800 (PST) From: Nate Lawson To: Andrew Gallatin In-Reply-To: <16012.32534.331966.216694@grasshopper.cs.duke.edu> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII cc: current@freebsd.org Subject: Re: mbuf LOR X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 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: Fri, 04 Apr 2003 08:03:10 -0000 On Thu, 3 Apr 2003, Andrew Gallatin wrote: > Nate Lawson writes: > > I was testing some changes to make fxp MPSAFE and got a LOR in allocating > > the mbuf cluster and then finally a panic when trying to dereference the > > cluster header. Is the mbuf system MPSAFE? Is it ok to call m_getcl > > with a device lock held (but not Giant)? > > > > The lock reversal was: 1. fxp softc lock, 2. Giant. > > I think the only place it can be coming from is slab_zalloc(). > Does the appended (untested) patch help? > > BTW, I don't think that there is any need to get Giant for the zone > allocators in the M_NOWAIT case, but I'm not really familar with the > code, and I don't know if the sparc64 uma_small_alloc needs Giant. > > BTW, my MPSAFE driver never sees this, but then again, I never > allocate clusters. I use jumbo frames, and carve out my own recv > buffers, so I'm only allocating mbufs, not clusters. > > Drew > > > Index: uma_core.c > =================================================================== > RCS file: /home/ncvs/src/sys/vm/uma_core.c,v > retrieving revision 1.51 > diff -u -r1.51 uma_core.c > --- uma_core.c 26 Mar 2003 18:44:53 -0000 1.51 > +++ uma_core.c 3 Apr 2003 18:22:14 -0000 > @@ -703,10 +703,14 @@ > wait &= ~M_ZERO; > > if (booted || (zone->uz_flags & UMA_ZFLAG_PRIVALLOC)) { > - mtx_lock(&Giant); > - mem = zone->uz_allocf(zone, > - zone->uz_ppera * UMA_SLAB_SIZE, &flags, wait); > - mtx_unlock(&Giant); > + if ((wait & M_NOWAIT) == 0) { > + mtx_lock(&Giant); > + mem = zone->uz_allocf(zone, > + zone->uz_ppera * UMA_SLAB_SIZE, &flags, wait); > + mtx_unlock(&Giant); > + } else { > + mem = NULL; > + } > if (mem == NULL) { > ZONE_LOCK(zone); > return (NULL); You're right about where the problem is (top of stack trace and listing below). However, your patch causes an immediate panic on boot due to a NULL deref. I don't think you want it to always return NULL if called with M_NOWAIT set. :) Other ideas? slab_zalloc + 0xdf uma_zone_slab + 0xd8 uma_zalloc_bucket + 0x15d uma_zalloc_arg + 0x307 malloc ... m_getcl (gdb) l *slab_zalloc+0xdf 0xc02f646f is in slab_zalloc (../../../vm/uma_core.c:707). 702 else 703 wait &= ~M_ZERO; 704 705 if (booted || (zone->uz_flags & UMA_ZFLAG_PRIVALLOC)) { 706 mtx_lock(&Giant); 707 mem = zone->uz_allocf(zone, 708 zone->uz_ppera * UMA_SLAB_SIZE, &flags, wait); 709 mtx_unlock(&Giant); 710 if (mem == NULL) { 711 ZONE_LOCK(zone); -Nate