Re: LoR aironet/VM subsystem

On Thu, Aug 12, 2004 at 10:27:41AM -0700, Nate Lawson wrote:
> Alan Cox wrote:
> >On Wed, Aug 11, 2004 at 10:37:42PM -0700, Nate Lawson wrote:
> >
> >>an0: <Cisco Systems 340 Series Wireless LAN Adapter> at port 
> >>0x2000-0x203f irq 11 function 0 config 5 on pccard1
> >>an0: got RSSI <-> dBM map
> >>an0: supported rates: 1Mbps 2Mbps 5.5Mbps 11Mbps
> >>an0: Ethernet address: 00:40:96:42:91:c5
> >>an0: [GIANT-LOCKED]
> >>lock order reversal
> >>1st 0xc163339c an0 (network driver) _at_ dev/an/if_an.c:1931
> >>2nd 0xc134b164 user map (user map) _at_ vm/vm_map.c:2902
> >>KDB: stack backtrace:
> >>kdb_backtrace(0,ffffffff,c0696d88,c0697a08,c066630c) at kdb_backtrace+0x29
> >>witness_checkorder(c134b164,9,c06469a0,b56) at witness_checkorder+0x544
> >>_sx_xlock(c134b164,c0646997,b56) at _sx_xlock+0x50
> >>_vm_map_lock_read(c134b128,c0646997,b56,1b69978,c1402e4c) at 
> >>_vm_map_lock_read+0x33
> >>vm_map_lookup(d0b69a10,bfbfd000,1,d0b69a14,d0b69a04) at vm_map_lookup+0x28
> >>vm_fault(c134b128,bfbfd000,1,0,c1371840) at vm_fault+0x66
> >>trap_pfault(d0b69ad8,0,bfbfd000) at trap_pfault+0xd2
> >>trap(c04f0018,c06b0010,c1630010,c1633df4,bfbfd000) at trap+0x311
> >>calltrap() at calltrap+0x5
> >>--- trap 0xc, eip = 0xc060661e, esp = 0xd0b69b18, ebp = 0xd0b69ba4 ---
> >>generic_copyin(c1632000,c020693a,d0b69c60,c04d4b78,c06bb8e0) at 
> >>generic_copyin+0x32
> >>ifhwioctl(c020693a,c1632000,d0b69c60,c1371840,c0698048) at ifhwioctl+0x854
> >>ifioctl(c16219e0,c020693a,d0b69c60,c1371840,0) at ifioctl+0xbd
> >>soo_ioctl(c158f440,c020693a,d0b69c60,c133c480,c1371840) at soo_ioctl+0x2b1
> >>ioctl(c1371840,d0b69d14,3,2,292) at ioctl+0x3e0
> >>syscall(2f,2f,2f,3,bfbfef64) at syscall+0x217
> >>Xint0x80_syscall() at Xint0x80_syscall+0x1f
> >>--- syscall (54, FreeBSD ELF32, ioctl), eip = 0x280d0083, esp = 
> >>0xbfbfad7c, ebp = 0xbfbfadb8 ---
> >>
> >
> >This looks like a programming error in the driver.  Specifically,
> >the driver is calling copyin() while holding a mutex.  That's not
> >allowed because of the potential for a page fault that sleeps.
> 
> It is.  AN_LOCK is held over the entire contents of an_ioctl().  This 
> function is so huge, it's unclear how to reduce the scope of this lock.

It should be held over almost none of it, especially not calls to
sub-ioctl() routines and ieee80211_*() ones and ones which perform
no action on the an device itself (which is a lot of them).  It's
never going to be needed before a copyin() or copyout().

-- 
Brian Fundakowski Feldman                           \'[ FreeBSD ]''''''''''\
  <> green_at_FreeBSD.org                               \  The Power to Serve! \
 Opinions expressed are my own.                       \,,,,,,,,,,,,,,,,,,,,,,\