wpaul [Sun, 27 Mar 2005 10:14:36 +0000 (10:14 +0000)]
Finally bring an end to the great "make the Atheros NDIS driver
work on SMP" saga. After several weeks and much gnashing of teeth,
I have finally tracked down all the problems, despite their best
efforts to confound and annoy me.
Problem nunmber one: the Atheros windows driver is _NOT_ a de-serialized
miniport! It used to be that NDIS drivers relied on the NDIS library
itself for all their locking and serialization needs. Transmit packet
queues were all handled internally by NDIS, and all calls to
MiniportXXX() routines were guaranteed to be appropriately serialized.
This proved to be a performance problem however, and Microsoft
introduced de-serialized miniports with the NDIS 5.x spec. Microsoft
still supports serialized miniports, but recommends that all new drivers
written for Windows XP and later be deserialized. Apparently Atheros
wasn't listening when they said this.
This means (among other things) that we have to serialize calls to
MiniportSendPackets(). We also have to serialize calls to MiniportTimer()
that are triggered via the NdisMInitializeTimer() routine. It finally
dawned on me why NdisMInitializeTimer() takes a special
NDIS_MINIPORT_TIMER structure and a pointer to the miniport block:
the timer callback must be serialized, and it's only by saving the
miniport block handle that we can get access to the serialization
lock during the timer callback.
Problem number two: haunted hardware. The thing that was _really_
driving me absolutely bonkers for the longest time is that, for some
reason I couldn't understand, my test machine would occasionally freeze
or more frustratingly, reset completely. That's reset and in *pow!*
back to the BIOS startup. No panic, no crashdump, just a reset. This
appeared to happen most often when MiniportReset() was called. (As
to why MiniportReset() was being called, see problem three below.)
I thought maybe I had created some sort of horrible deadlock
condition in the process of adding the serialization, but after three
weeks, at least 6 different locking implementations and heroic efforts
to debug the spinlock code, the machine still kept resetting. Finally,
I started single stepping through the MiniportReset() routine in
the driver using the kernel debugger, and this ultimately led me to
the source of the problem.
One of the last things the Atheros MiniportReset() routine does is
call NdisReadPciSlotInformation() several times to inspect a portion
of the device's PCI config space. It reads the same chunk of config
space repeatedly, in rapid succession. Presumeably, it's polling
the hardware for some sort of event. The reset occurs partway through
this process. I discovered that when I single-stepped through this
portion of the routine, the reset didn't occur. So I inserted a 1
microsecond delay into the read loop in NdisReadPciSlotInformation().
Suddenly, the reset was gone!!
I'm still very puzzled by the whole thing. What I suspect is happening
is that reading the PCI config space so quickly is causing a severe
PCI bus error. My test system is a Sun w2100z dual Opteron system,
and the NIC is a miniPCI card mounted in a miniPCI-to-PCI carrier card,
plugged into a 100Mhz PCI slot. It's possible that this combination of
hardware causes a bus protocol violation in this scenario which leads
to a fatal machine check. This is pure speculation though. Really all I
know for sure is that inserting the delay makes the problem go away.
(To quote Homer Simpson: "I don't know how it works, but fire makes
it good!")
Problem number three: NdisAllocatePacket() needs to make sure to
initialize the npp_validcounts field in the 'private' section of
the NDIS_PACKET structure. The reason if_ndis was calling the
MiniportReset() routine in the first place is that packet transmits
were sometimes hanging. When sending a packet, an NDIS driver will
call NdisQueryPacket() to learn how many physical buffers the packet
resides in. NdisQueryPacket() is actually a macro, which traverses
the NDIS_BUFFER list attached to the NDIS_PACKET and stashes some
of the results in the 'private' section of the NDIS_PACKET. It also
sets the npp_validcounts field to TRUE To indicate that the results are
now valid. The problem is, now that if_ndis creates a pool of transmit
packets via NdisAllocatePacketPool(), it's important that each time
a new packet is allocated via NdisAllocatePacket() that validcounts
be initialized to FALSE. If it isn't, and a previously transmitted
NDIS_PACKET is pulled out of the pool, it may contain stale data
from a previous transmission which won't get updated by NdisQueryPacket().
This would cause the driver to miscompute the number of fragments
for a given packet, and botch the transmission.
Fixing these three problems seems to make the Atheros driver happy
on SMP, which hopefully means other serialized miniports will be
happy too.
And there was much rejoicing.
Other stuff fixed along the way:
- Modified ndis_thsuspend() to take a mutex as an argument. This
allows KeWaitForSingleObject() and KeWaitForMultipleObjects() to
avoid any possible race conditions with other routines that
use the dispatcher lock.
- Fixed KeCancelTimer() so that it returns the correct value for
'pending' according to the Microsoft documentation
- Modfied NdisGetSystemUpTime() to use ticks and hz rather than
calling nanouptime(). Also added comment that this routine wraps
after 49.7 days.
- Added macros for KeAcquireSpinLock()/KeReleaseSpinLock() to hide
all the MSCALL() goop.
- For x86, KeAcquireSpinLockRaiseToDpc() needs to be a separate
function. This is because it's supposed to be _stdcall on the x86
arch, whereas KeAcquireSpinLock() is supposed to be _fastcall.
On amd64, all routines use the same calling convention so we can
just map KeAcquireSpinLockRaiseToDpc() directly to KfAcquireSpinLock()
and it will work. (The _fastcall attribute is a no-op on amd64.)
- Implement and use IoInitializeDpcRequest() and IoRequestDpc() (they're
just macros) and use them for interrupt handling. This allows us to
move the ndis_intrtask() routine from if_ndis.c to kern_ndis.c.
- Fix the MmInitializeMdl() macro so that is uses sizeof(vm_offset_t)
when computing mdl_size instead of uint32_t, so that it matches the
MmSizeOfMdl() routine.
- Change a could of M_WAITOKs to M_NOWAITs in the unicode routines in
subr_ndis.c.
- Use the dispatcher lock a little more consistently in subr_ntoskrnl.c.
- Get rid of the "wait for link event" hack in ndis_init(). Now that
I fixed NdisReadPciSlotInformation(), it seems I don't need it anymore.
This should fix the witness panic a couple of people have reported.
- Use MSCALL1() when calling the MiniportHangCheck() function in
ndis_ticktask(). I accidentally missed this one when adding the
wrapping for amd64.
sam [Sat, 26 Mar 2005 21:49:43 +0000 (21:49 +0000)]
rt_newaddrmsg will blow up if given something other than RTM_ADD
or RTM_DELETE; add an assertion, may want to do something more
heavyhanded in the future
njl [Sat, 26 Mar 2005 17:30:34 +0000 (17:30 +0000)]
Check for invalid frequencies after parsing the package. Keep a running
count of valid frequencies and use that as the final package count, don't
give up when the first invalid state is found. Also, add 0x9999 and expand
our upper check to >= 0xffff Mhz [2].
cperciva [Sat, 26 Mar 2005 04:45:53 +0000 (04:45 +0000)]
When executing mount_foo, pass "mount_foo" as argv[0] instead of "foo".
This unbreaks "/rescue/mount -t foo" -- previously it was necessary to
explicitly call "/rescue/mount_foo".
Hints from: gordon
X-MFC after: 3 days (if approved by re@)
sobomax [Fri, 25 Mar 2005 20:19:18 +0000 (20:19 +0000)]
Comment out rue_miibus_statchg() function. Using trial-and-error approach I
found it guilty in putting the card into unusable state after UP->DOWN->UP
media status change.
Looks like some of register writes in this functions mess up PHY interface.
No visible regressions has been found after commenting this code out -
the card properly handles forceful local mode changes and auto-detects changes
made remotely (tested with Auto, 10HD, 10FD, 100HD, 100FD).
das [Fri, 25 Mar 2005 17:30:31 +0000 (17:30 +0000)]
When the softupdates worklist gets too long, threads that attempt to
add more work are forced to process two worklist items first.
However, processing an item may generate additional work, causing the
unlucky thread to recursively process the worklist. Add a per-thread
flag to detect this situation and avoid the recursion. This should
fix the stack overflows that could occur while removing large
directory trees.
jhb [Fri, 25 Mar 2005 14:18:50 +0000 (14:18 +0000)]
Add code to read the primary PCI bus number out of the Compaq/HP 6010
hotplug Host to PCI bridge. This is only needed for the non-ACPI case
as the BIOS includes a proper _BBN method in ACPI.
imp [Fri, 25 Mar 2005 07:35:59 +0000 (07:35 +0000)]
Restore the ability to read FreeBSD 1 tapes (and I think any net2
based tapes, but I'm not sure where NFS_MAGIC was introduced after
4.3). When support for the pre-4.4 format was removed (the ability to
read 4.2 and 4.3 BSD tapes), the old format inode conversion was
junked as well. However, FreeBSD 1 dump tapes use the NFS_MAGIC
format, but have this inode format. Before, restore would fail
complaining that '.' wasn't found and the root directory wasn't on
this tape. Since the conversion from the not so old format is
relatively trivial, restore the code to make that conversion.
imp [Fri, 25 Mar 2005 06:03:11 +0000 (06:03 +0000)]
c_tapea and c_firstrec are used for TS_TAPE blocks, so convert them
for the old (4.4-lite through FreeBSD 4.x and *BSD) format. It looks
like they aren't used for TS_INODE, but conversion costs so little
there that I've not removed them there (in case my grep was wrong).
This makes at least some of the tapes work for me again. Now, to
regresion test all my dusty tapes...
jeff [Fri, 25 Mar 2005 05:34:39 +0000 (05:34 +0000)]
- Don't recycle vnodes anymore. Free them once they are dead. getnewvnode
now always allocates a new vnode.
- Define a new function, vnlru_free, which frees vnodes from the free list.
It takes as a parameter the number of vnodes to free, which is
wantfreevnodes - freevnodes when called from vnlru_proc or 1 when
called from getnewvnode(). For now, getnewvnode() still tries to reclaim
a free vnode before creating a new one when we are near the limit.
- Define a function, vdestroy, which handles the actual release of memory
and teardown of locks, etc. This could become a uma_dtor() routine.
- Get rid of minvnodes. Now wantfreevnodes is 1/4th the max vnodes. This
keeps more unreferenced vnodes around so that files which have only
been stat'd are less likely to be kicked out of the system before we
have a chance to read them, etc. These vnodes may still be freed via
the normal vnlru_proc() routines which may some day become a real lru.
jhb [Fri, 25 Mar 2005 03:10:51 +0000 (03:10 +0000)]
- Use pci_get_device() and pci_get_vendor() when we only want one part
of the device id.
- Use BAR2 rather than BAR0 for the Rocketport UPCI 8O card. I suspect
that other UPCI cards might need to use BAR2 as well.
marcel [Fri, 25 Mar 2005 01:56:12 +0000 (01:56 +0000)]
Fix inittodr() invocation. Now that devfs is mounted before the
actual root file system is mounted, the first entry on the mountlist
is not the root file system and the timestamp for that entry is
typically 0. Passing that to inittodr() caused annoying errors on
alpha and ia64.
So, call inittodr() for all file systems on mountlist, but only when
the timestamp (mnt_time) is non-zero.
iedowse [Fri, 25 Mar 2005 01:44:38 +0000 (01:44 +0000)]
Use usbd_get_string() instead of calling usbd_get_string_desc()
with the wrong language parameter when retrieving the device serial
number. This invalid request caused some devices not to work at
all.
PR: usb/79190
Submitted by: Hans Petter Selasky <hselasky@c2i.net>
kensmith [Thu, 24 Mar 2005 23:03:39 +0000 (23:03 +0000)]
Update parsing of the ports INDEX file to accomodate some new
fields that got added to it recently-ish. While here document
what each of the fields is based on discussion with portmgr@.
Patch from: murray (slightly adapted)
MFC after: 1 day
jhb [Thu, 24 Mar 2005 21:36:15 +0000 (21:36 +0000)]
Merge from i386:
- Add a i8254_pending variable to save some indirections in clkintr().
- Don't bother setting up an IRQ0 handler if we are using the lapic timer.
jhb [Thu, 24 Mar 2005 21:34:16 +0000 (21:34 +0000)]
- Don't enable periodic interrupts from the RTC by default in rtc_statusb.
Instead, explicitly enable them when we setup the interrupt handler.
Also, move the setting of stathz and profhz down to the same place so
that the code flow is simpler and easier to follow.
- Don't setup an interrupt handler for IRQ0 if we are using the lapic timer
as it doesn't do anything productive in that case.
jhb [Thu, 24 Mar 2005 21:20:25 +0000 (21:20 +0000)]
Don't set ret_namelen and ret_resnamelen in res_find() unless both the
corresponding pointer to the buffer (ret_name and ret_resname) is non-NULL
to avoid possible NULL pointer derefs.
brooks [Thu, 24 Mar 2005 17:51:15 +0000 (17:51 +0000)]
Compile errors are way more useful then panics later.
Replace a KASSERT of LINUX_IFNAMSIZ == IFNAMSIZ with a preprocessor
check and #error message. This will prevent nasty suprises if users
change IFNAMSIZ without updating the linux code appropriatly.
cperciva [Thu, 24 Mar 2005 10:12:29 +0000 (10:12 +0000)]
If "dangerous" environment variables (LD_PRELOAD, LD_LIBMAP,
LD_LIBMAP_DISABLE, LD_LIBRARY_PATH) are used, then make sure the
libraries being loaded aren't on a noexec-mounted filesystem.
This is a compromise position: I'm assuming that nobody will be silly
enough to set the noexec mount flag on part of the default library
path, in order to avoid adding extra overhead into the common case
(where those environment variables aren't used).
jeff [Thu, 24 Mar 2005 09:35:06 +0000 (09:35 +0000)]
- Complete the implementation of td_locks. Track the number of outstanding
lockmgr locks that this thread owns. This is complicated due to
LK_KERNPROC and because lockmgr tolerates unlocking an unlocked lock.
jeff [Thu, 24 Mar 2005 07:39:03 +0000 (07:39 +0000)]
- Update vfs_root implementations to match the new prototype. None of
these filesystems will support shared locks until they are explicitly
modified to do so. Careful review must be done to ensure that this
is safe for each individual filesystem.
jeff [Thu, 24 Mar 2005 07:36:16 +0000 (07:36 +0000)]
- Update vfs_root implementations to match the new prototype. None of
these filesystems will support shared locks until they are explicitly
modified to do so. Careful review must be done to ensure that this
is safe for each individual filesystem.
jeff [Thu, 24 Mar 2005 07:29:23 +0000 (07:29 +0000)]
- Add a 'flags' parameter to VFS_ROOT(). This is intended to allow
lookup to do shared locks on the root. Filesystems are free to ignore
flags and instead acquire an exclusive lock if they do not support
shared locks.
jeff [Thu, 24 Mar 2005 06:08:58 +0000 (06:08 +0000)]
- If vput() is called with a shared lock it must upgrade to an exclusive
before it can call VOP_INACTIVE(). This must use the EXCLUPGRADE path
because we may violate some lock order with another locked vnode if
we drop and reacquire the lock. If EXCLUPGRADE fails, we mark the
vnode with VI_OWEINACT. This case should be very rare.
- Clear VI_OWEINACT in vinactive() and vbusy().
- If VI_OWEINACT is set in vgone() do the VOP_INACTIVE call here as well.
jeff [Thu, 24 Mar 2005 06:04:35 +0000 (06:04 +0000)]
- Remove some long dead LOOKUP_SHARED code that tracked the lock state.
- Always pass LOCKSHARED and rely on namei() to ignore it when
LOOKUP_SHARED is not set.
jeff [Thu, 24 Mar 2005 06:02:37 +0000 (06:02 +0000)]
- Clear LOCKSHARED if LOOKUP_SHARED is not enabled. This is not strictly
necessary since we disable the shared locks in vfs_cache, but it is
prefered that the option not leak out into filesystems when it is
disabled.
jeff [Thu, 24 Mar 2005 06:00:45 +0000 (06:00 +0000)]
- All of the bugs which lead to the complication of the LOOKUP_SHARED
config option have now been fixed. All filesystems are properly locked
and checked via DEBUG_VFS_LOCKS. Remove the workaround code.