Re: Witness finds "malloc(M_WAITOK) with non-sleepable lock held" in FreeBSD 7.0-CURRENT (amd64)

Wow, I didn't realize how screwed up that was.  Yes, if_em is definitely
wrong.  It's not clear to me if the number of rx and tx descriptors that
the driver wants will change with each call to em_init, but at the very
least the busdma tag allocation should move to the attach routine.

Scott

David Christensen wrote:
> Actually I was following the example in sys/dev/if_em.c.  The call
> chain is:
> 
> 	bus_dma_tag_create() is called from
> 	em_allocate_receive_structures() is called from
> 	em_setup_receive_structures() is called from
> 	em_init_locked()
> 
> The em driver doesn't release its lock before calling
> bus_dma_tag_create()
> and it definitely does it outside of the attach routine.  Is the em
> driver also FUBAR or is there something else going on?
> 
> David Christensen
> 
> -----Original Message-----
> From: Scott Long [mailto:scottl_at_samsco.org] 
> Sent: Wednesday, March 22, 2006 11:23 PM
> To: John-Mark Gurney
> Cc: David Christensen; freebsd-current_at_freebsd.org
> Subject: Re: Witness finds "malloc(M_WAITOK) with non-sleepable lock
> held" in FreeBSD 7.0-CURRENT (amd64)
> 
> John-Mark Gurney wrote:
> 
>>David Christensen wrote this message on Wed, Mar 22, 2006 at 21:55
> 
> -0800:
> 
>>>I'm developing an Ethernet driver with FreeBSD 7.0-CURRENT (amd64) and
>>>I'm 
>>>receiving many of the following witness errors:
>>>
>>>malloc(M_WAITOK) of "128", forcing M_NOWAIT with the following
>>>non-sleepable locks held:
>>>exclusive sleep mutex bce0 (network driver) r = 0 (0xffffffff8111e068)
>>>locked _at_ if_bce.c:4607
>>>KDB: stack backtrace:
>>>kdb_backtrace() at kdb_backtrace+0x37
>>>witness_warn() at witness_warn+0x2c1
>>>uma_zalloc_arg() at uma_zalloc_arg+0x69
>>>malloc() at malloc+0xf5
>>>sysctl_add_oid() at sysctl_add_oid+0xa9
>>>alloc_bounce_zone() at alloc_bounce_zone+0x16b
>>>bus_dma_tag_create() at bus_dma_tag_create+0x1ea
>>>bce_init_rx_chain() at bce_init_rx_chain+0x8e
>>>bce_init_locked() at bce_init_locked+0x1e2
>>>bce_init() at bce_init+0x39
>>>ether_ioctl() at ether_ioctl+0x87
>>>bce_ioctl() at bce_ioctl+0x48e
>>>in6_ifinit() at in6_ifinit+0xbd
>>>in6_update_ifa() at in6_update_ifa+0x563
>>>in6_ifattach_linklocal() at in6_ifattach_linklocal+0x126
>>>in6_ifattach() at in6_ifattach+0xdf
>>>in6_if_up() at in6_if_up+0x59
>>>if_route() at if_route+0x8a
>>>if_up() at if_up+0x13
>>>ifhwioctl() at ifhwioctl+0x2f4
>>>ifioctl() at ifioctl+0x10b
>>>soo_ioctl() at soo_ioctl+0x38c
>>>ioctl() at ioctl+0x436
>>>syscall() at syscall+0x350
>>>Xfast_syscall() at Xfast_syscall+0xa8
>>>--- syscall (54, FreeBSD ELF64, ioctl), rip = 0x8008219ac, rsp =
>>>0x7fffffffe6b8, rbp = 0x1 ---
>>>
>>>The bus_dma_tag_create looks like this:
>>>
>>>bus_dma_tag_create(
>>>	sc->parent_tag,		/* parent      */
>>>   	4096,				/* alignment   */
>>>   	0,				/* boundary    */
>>>	BUS_SPACE_MAXADDR,	/* lowaddr     */
>>>	BUS_SPACE_MAX_ADDR,	/* lowaddr     */
>>>	NULL,				/* filter      */
>>>	NULL, 			/* filterarg   */
>>>	4096,				/* maxsize     */
>>>	1, 				/* nsegments   */
>>>	4096,				/* maxsegsize  */
>>>	BUS_DMA_ALLOCNOW, 	/* flags       */
>>>	NULL,				/* lockfunc    */
>>>	NULL,				/* lockarg     */
>>>	&sc->rx_bd_chain_tag));
>>>
>>>Am I doing something wrong?  The function bce_init_rx_chain is called
>>
>>>from with
>>
>>>a lock but isn't that normal?
>>
>>
>>Yeh, you have to unlock your driver lock before calling
>>bus_dma_tag_create..  If you look at the other ethernet drivers, some
>>call _tag_create as part of attach, not in _init...  at this point,
>>it's safe to release your lock and allocate memory...
>>
> 
> 
> In fact, it's really bad to be initializing the rx data structures like 
> this in if_init.  It should be done in dev_attach.  The reason is that
> if_init can be called at any time and will almost certainly be called
> multiple times.  Also, do not use the BUS_DMA_ALLOCNOW flag here, as I
> assume that you are trying to use the busdma tag to allocate a static
> piece of memory for the rx chain/ring.  The flag should only be used
> for flags that deal with dynamic buffers like mbufs and bio_data
> objects, or memory that has been allocated in the kernel with normal
> malloc.
> 
> Scott
> 
> 
> 
>