Re: if_ral + wpa_supplicant stack backtrace

Tai-hwa Liang wrote:
> On Fri, 24 Jun 2005, John Baldwin wrote:
> 
>> On Saturday 18 June 2005 11:56 am, Pascal Hofstee wrote:
>>
>>> Hi,
>>>
>>> I am seeing period occurances of the same system call with the same
>>> WITNESS warning and similar backtrace on yesterday's AMD64 CURRENT.
>>>
>>> ----------------------
>>> ral0: link state changed to DOWN
>>> malloc(M_WAITOK) of "32", forcing M_NOWAIT with the following
>>> non-sleepable locks held:
>>> exclusive sleep mutex ral0 (network driver) r = 0 (0xffffffff80c64de8)
>>> locked _at_ /usr/src/sys/dev/ral/if_ral.c:2167
>>> 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
>>> ieee80211_ioctl_setoptie() at ieee80211_ioctl_setoptie+0x4b
>>> ieee80211_ioctl_set80211() at ieee80211_ioctl_set80211+0x64e
>>> ieee80211_ioctl() at ieee80211_ioctl+0x125
>>> ral_ioctl() at ral_ioctl+0xa4
>>> in_control() at in_control+0xc2f
>>> ifioctl() at ifioctl+0x1f6
>>> soo_ioctl() at soo_ioctl+0x38c
>>> ioctl() at ioctl+0x476
>>> syscall() at syscall+0x332
>>> Xfast_syscall() at Xfast_syscall+0xa8
>>> --- syscall (54, FreeBSD ELF64, ioctl), rip = 0x8007c2d4c, rsp =
>>> 0x7fffffffdfd8, rbp = 0x18 ---
>>> ral0: link state changed to UP
>>> ----------------------
>>>
>>> I am indeed curious to understand what exactly is causing these WITNESS
>>> warnings
>>
>>
>> This is a bug in ral(4) as it indiscriminately just holds its mutex 
>> across
>> ieee80211_ioctl() which isn't safe.  The ral(4) maintainer needs to 
>> fix it.
> 
> 
>   The same WITNESS warning also occurs in ath(4) whilst using 
> wpa_supplicant.
> It looks to me that it's probably not individual driver's fault, but a 
> known
> behaviour in net80211.
> 
>   Attached patch should suppress this warning; however, I'm not quite sure
> about whether it's safe to do so or not....
> 
> 
> ------------------------------------------------------------------------
> 
> --- ieee80211_ioctl.c.orig	Sat Jun 18 14:07:01 2005
> +++ ieee80211_ioctl.c	Sat Jun 25 10:04:48 2005
> _at__at_ -1475,7 +1475,7 _at__at_
>  	if (ireq->i_len > IEEE80211_MAX_OPT_IE)
>  		return EINVAL;
>  	/* NB: data.length is validated by the wireless extensions code */
> -	MALLOC(ie, void *, ireq->i_len, M_DEVBUF, M_WAITOK);
> +	MALLOC(ie, void *, ireq->i_len, M_DEVBUF, M_NOWAIT);
>  	if (ie == NULL)
>  		return ENOMEM;
>  	error = copyin(ireq->i_data, ie, ireq->i_len);

This is one instance of a general problem that's existed for over a 
year.  Calling ieee80211_ioctl (typically) requires the driver hold it's 
lock but certain operations in the 802.11 layer may need to block but 
have no way to release the driver lock or otherwise synchronize the 
operations.  I've brought this up several times and suggested that the 
cleanest solution looks to be exposing the driver lock to the 802.11 
layer; possibly by making the driver softc lock part of the ifnet 
structure.  But this approach never went very far so the problems have 
remained.  The key concern with doing the ifnet/softc lock thing is that 
it might force us into a locking model that's too restrictive.

	Sam