On Tue, 23 Mar 2004 09:12:00 +0100 Oliver Neukum <[EMAIL PROTECTED]> wrote:
> Am Tuesday 23 March 2004 01:28 schrieb Pete Zaitcev: > > On Mon, 22 Mar 2004 22:21:14 +0100 > > Oliver Neukum <[EMAIL PROTECTED]> wrote: > > > > > you are using an irqsave spinlock in a completion handler. Not actually > > > wrong but it tends to confuse people about the context the function is > > > executed under. > > > > > priv = usb_get_serial_port_data(port); > > > - spin_lock_irqsave(&priv->lock, flags); > > > + spin_lock(&priv->lock); > > > priv->last_msr = data[BELKIN_SA_MSR_INDEX]; > > > > I strongly disagree with this point of view. Keeping track of a > > context in which a function can be called places an unreasonable > > burden on a programmer and introduces extremely annoying code rot > > whenever code migrates contexts. > > You cannot efficiently write code that runs under several contexts. > Too many things mostly related to the ability to sleep depend on the > context. We continously get buggy drivers which sleep in interrupt > because people are not aware. What you say is true for any non-trivial code, especially if it does any memory allocations. We learned this at a high price with Youngdale's SCSI which basically made programmers to assume an interrupt context throughout the stack and thus precluded GFP_KERNEL completely. However, a small spinlock bracket in a device driver is not the place to apply this wisdom, primarily because a well used paradigm there looks like this: foo_submit(dev) spin_lock_irqsave(dev->lock, flags) poke_registers(dev->regs, ...) spin_unlock_irqrestore(dev->lock, flags) foo_write(f, x) dev = f_to_dev(f) foo_submit(dev, x) foo_intr(dev_id) dev = id_to_dev(dev_id) foo_submit(dev, next_x(dev)) Another place where an attempt to mark interrupt, process, softirq code with spinlocks blows up is when you have to block several levels at once, that is any place where timers get into the picture. This is particularly critical for network drivers. Please consider that sleeping in an interrupt is catcheable by BUG(), but not closing interrupts when needed is not catcheable. Thus, I will happily trade you all such bugs for those where the code races undetected. To summarize, your phylosophy is generally right, but not in this particular case. -- Pete P.S. I personally maintain a driver which does spin_lock() in interrupt, that is drivers/sound/ymfpci.c, but the reason is that it imported that code from Alan Cox's cs46xx.c, not because I wrote it that way. My other phylosophy is not to change the code which works. ------------------------------------------------------- This SF.Net email is sponsored by: IBM Linux Tutorials Free Linux tutorial presented by Daniel Robbins, President and CEO of GenToo technologies. Learn everything from fundamentals to system administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
