On Wed, 7 Jul 2004, Greg KH wrote: > On Wed, Jul 07, 2004 at 04:52:23PM -0700, Pete Zaitcev wrote: > > On Wed, 7 Jul 2004 22:58:29 +0200 > > Duncan Sands <[EMAIL PROTECTED]> wrote: > > > > > Is there any chance that a writer will wait for ever because readers > > > never stop coming?
In this case, I don't think so. Threads acquire the read lock only when an important USB event occurs: binding change, disconnect, suspend/resume, reset, traversing the device tree, hub port notification. These things aren't all that common. Threads acquire the write lock even less frequently: only when a driver is registered or deregistered (or when usbfs handles the USBDEVFS_CONNECT ioctl, which rescans all the devices on the bus). Such threads can easily afford to wait until the readers have all finished. > > If you want fairness, every kind of rw locks in Linux fails you. > > You must use normal (non-rw) locks in such case. > > > > > Is it important to have several different > > > readers hold the lock at the same time? > > > > Not typically. I argued against gratious use of rwlocks for years. > > See http://www.advogato.org/person/Zaitcev/diary.html?start=153 > > I agree with Pete here. > > Why are we trying to implement a bastard BKL implementation here? > > With this implementation, it sounds like we can starve out legitimate > users. > > Alan, why would we try to take a lock twice for the same codepath? In order, then: This isn't quite the same as BKL but it is vaguely similar. The problem is very simple: When a USB driver is registered or unregistered, the driver-model core will automatically probe/disconnect it from all matching devices without giving usbcore the chance to lock those devices first. The only way to insure proper locking is to effectively lock _all_ devices at these times. (Actually, what I just wrote is an oversimplification. Usbcore does get a chance to lock each device as it is probed, in the usb_probe_interface() function. However that chance occurs too late -- since the driver-model core already owns the bus subsystem rwsem we can't afford to lock the device being probed. To do so would risk deadlock with the other probe path: new device is added and locked, driver-model core is notified, it grabs the USB subsystem rwsem, then probes the existing drivers. The proper order of locking is device first, bus rwsem second. Furthermore, if we did lock the device in usb_probe_interface() we would be trying to lock it twice in the case where a new device is being probed.) To solve this problem, a new strategy is needed: (1) Permit drivers to lock simultaneously any number of different devices, provided register/unregister isn't in progress; (2) Register/unregister must wait for all drivers to unlock their devices and must block new locks. This certainly sounds like a good application for an rw-semaphore, and that's why I did it that way. Drivers trying to lock a device must acquire the rwsem's readlock first. Register/unregister acquire the writelock. This does (almost) exactly what is needed. As for starving legitimate users, see my reply to Duncan's question above. None of these locks have high usage or high contention. Basically, they are only used when something exceptional happens (new connection or disconnection, state change like suspend/resume, etc.) or when some thread needs to traverse the entire device tree. Yes, it's certainly possible that a buggy driver might crash while holding the rwsem, thereby preventing other threads from ever acquiring it -- but that's always true whenever any kind of locking is employed. Lastly, why does a thread need to take the lock twice? Quite simply, it needs to lock two devices. Locking a device means acquiring the rwsem's readlock first, so locking two devices means acquiring the readlock twice. Here are a couple of examples of times when a driver needs to lock two devices (always in top-down order!): Khubd always locks the hub when it processes a port change notification -- after all, we don't want strange things to happen to the hub while we are working on one of its ports! If the port event being handled is a new connection, khubd will create a new usb_device _and lock it_ while enumerating, configuring, and probing it. The code that populates /proc/bus/usb/devices needs to run through the entire device tree, and it needs to guarantee that branches aren't removed while it's working on them. In order to accomplish this it recursively locks each hub it encounters while processing that hub's children. So at any moment it holds the locks for all the hubs on the path leading from the current device back to the root. (It also holds the lock for the device itself, to prevent resets and other strange things from happening while it reports the device's state.) I chose to use an rwsem because it was a good, albeit not perfect, match to the design requirements. If anyone can suggest a superior alternative, I won't hesitate to change the implementation. All the code is localized (not by accident!) in a handful of functions: usb_lock_device(), usb_lock_all_devices(), etc., making such a change very easy to write. Alan Stern ------------------------------------------------------- This SF.Net email sponsored by Black Hat Briefings & Training. Attend Black Hat Briefings & Training, Las Vegas July 24-29 - digital self defense, top technical experts, no vendor pitches, unmatched networking opportunities. Visit www.blackhat.com _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
