From owner-freebsd-hackers@FreeBSD.ORG Wed Aug 9 06:08:38 2006 Return-Path: X-Original-To: freebsd-hackers@freebsd.org Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 447B316A4E0 for ; Wed, 9 Aug 2006 06:08:38 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from harmony.bsdimp.com (vc4-2-0-87.dsl.netrack.net [199.45.160.85]) by mx1.FreeBSD.org (Postfix) with ESMTP id C899643D4C for ; Wed, 9 Aug 2006 06:08:37 +0000 (GMT) (envelope-from imp@bsdimp.com) Received: from localhost (localhost.village.org [127.0.0.1] (may be forged)) by harmony.bsdimp.com (8.13.4/8.13.4) with ESMTP id k7967FUW021445; Wed, 9 Aug 2006 00:07:15 -0600 (MDT) (envelope-from imp@bsdimp.com) Date: Wed, 09 Aug 2006 00:07:19 -0600 (MDT) Message-Id: <20060809.000719.-432838874.imp@bsdimp.com> To: hselasky@c2i.net From: "M. Warner Losh" In-Reply-To: <200608021437.55500.hselasky@c2i.net> References: <200608021437.55500.hselasky@c2i.net> X-Mailer: Mew version 4.2 on Emacs 21.3 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-2.0 (harmony.bsdimp.com [127.0.0.1]); Wed, 09 Aug 2006 00:07:15 -0600 (MDT) Cc: freebsd-hackers@freebsd.org Subject: Re: miibus + USB = problem X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 Aug 2006 06:08:38 -0000 In message: <200608021437.55500.hselasky@c2i.net> Hans Petter Selasky writes: : I am currently in the progress of porting "if_aue.c" to my new USB API, and : have hit a problem that needs to be solved. The problem is that the USB : system sleeps under "miibus_xxx". Questions are: : : Is this allowed? Yes. : What locks are held when these functions are called ? Whatever locks are held when they are called :-). miibus is called in the following instances: (1) during probe/attach of children (bus_generic_probe is the typical cause) (2) During the 'tick' routine. The tick routine is called from a timeout typically. In the aue driver, for example, this is done with timeout/untimeout. There's been rumblings for a genric mii /dev entry that can be used for things like kicking off the autonegotiation, etc, but that's not in the tree now, nor is it likely to be soon. The aue driver takes out the AUE_LOCK in these routines, and in detach, it unregisters the timeout. Alas, it is stupid, and does this with the lock held, thus ensuring deadlock if the timeout fires after the lock is acquired, but before the untimeout can complete (meaning that the timeout routine would sleep forever waiting for the lock, oops). This is why you can't run the detach routine locked in most cases. You have to make sure that all other threads have exited, and that can be tricky. : The problem with USB devices, is that the read-register process is very slow. : It can take up to several milliseconds. And if the device is suddenly : detached one has to think about adding exit code everywhere. Yes. One does. There's no such thing as a free lunch in the kernel, alas. : The solution I see with USB devices is something like this: : : if (sc->device_gone) { : : exit mutexes ; : : kthread_exit(0); : } : : Of course I cannot "kthread_exit()" when the call comes from read/write/ioctl, : because there is a stack, that expects a returning call. If the kernel code : was objective C, then maybe one could throw an exception or do something : alike so that the processor gets out of the USB-read procedure. Yes. You can't longjmp your way out of this problem. :-) There's no thread to kill here... : Solutions: : : 1) use USB hardware polling, not releasing any mutexes, simply using DELAY(), : until read/writes complete. That's insanely ugly. : 2) pre-read all read registers regularly. How can I do this with "miibus"? You can't do that either. You have to use the aue 'registers' to read the mii bus management registers. The only way to deal with this is to have all the places that read the registers deal with getting an error. If the device really is dying, you can set a flag so that further reads also return an error to catch places where the code is sloppy and doesn't check the return value. In fact, the aue_csr_read_* routines do exactly this already, and much of the code is written to check for the dying. Apart from the deadlock in the locking unwiding (I'm sure there are others, because we also call if_detach with this lock held...) Warner