Re: mbuf LOR

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
> _at__at_ -703,10 +703,14 _at__at_
>  		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