Re: New LOR ?

On 2/12/16 12:50 AM, Poul-Henning Kamp wrote:
> I don't recall seeing this one before:
>
> FreeBSD critter.freebsd.dk 11.0-CURRENT FreeBSD 11.0-CURRENT #31 r293468: Sat Jan  9 11:50:09 UTC 2016     root_at_critter.freebsd.dk:/freebsd/obj/freebsd/svn_src/head/sys/GENERIC  amd64
>
> +taskqueue_drain with the following non-sleepable locks held:
> +exclusive sleep mutex bpf global lock (bpf global lock) r = 0 (0xffffffff81c53
> fe8) locked _at_ /freebsd/svn_src/head/sys/net/bpf.c:772
> +stack backtrace:
> +#0 0xffffffff80a79ee0 at witness_debugger+0x70
> +#1 0xffffffff80a7b1f7 at witness_warn+0x3d7
> +#2 0xffffffff80a6daeb at taskqueue_drain+0x3b
> +#3 0xffffffff80b4bb0b at ieee80211_waitfor_parent+0x3b
> +#4 0xffffffff80b32d57 at ieee80211_ioctl+0x2f7
> +#5 0xffffffff80afe025 at if_setflag+0xd5
> +#6 0xffffffff80afdefc at ifpromisc+0x2c
> +#7 0xffffffff80af41e8 at bpf_detachd_locked+0x1e8
> +#8 0xffffffff80af6cfe at bpf_dtor+0x8e
> +#9 0xffffffff808f7312 at devfs_destroy_cdevpriv+0x82
> +#10 0xffffffff808faa85 at devfs_close_f+0x65
> +#11 0xffffffff809d0e6a at _fdrop+0x1a
> +#12 0xffffffff809d3fb1 at closef+0x1e1
> +#13 0xffffffff809d3afd at fdescfree_fds+0x9d
> +#14 0xffffffff809d361c at fdescfree+0x46c
> +#15 0xffffffff809e1676 at exit1+0x4e6
> +#16 0xffffffff809e118d at sys_sys_exit+0xd
> +#17 0xffffffff80e6b13b at amd64_syscall+0x2db
>
>
>
Yeah, "ifpromisc(ifp, 0);" shouldn't really be called with the global 
bpf lock, it is unneeded and extends the length of the lock held to an 
unacceptable depth.

However it looks like dropping the lock inside of bpf_detachd_locked() 
isn't safe... at least at a glance bpfdetach() relies on BPF_LOCK() 
being held through its entire run, and bpfdetach() calls 
bpf_detachd_locked(), so temporarily unlocking in bpf_detachd_locked() 
would break things.

It may be possible to fix the ieee80211 stack not to call taskqueue 
drain, however in reality the BPF_LOCK is just being held over too much 
code.

Can the call to ifpromisc() be deferred into a taskqueue without causing 
too much grief and/or a race condition?

-Alfred


-Alfred