Am Freitag, 23. September 2005 00:49 schrieb David Kubicek: > On Fri, Sep 23, 2005 at 12:07:23AM +0200, Oliver Neukum wrote: > > Am Mittwoch, 21. September 2005 20:04 schrieb David Kubicek: > > > On Wed, Sep 21, 2005 at 06:32:05PM +0200, Oliver Neukum wrote: > > > > Am Mittwoch, 21. September 2005 14:51 schrieb David Kubicek: > > > > > > This is in interrupt. Please use the simple form of the locks. > > > I'd like to find out the difference between and in which context to > > > use the following locking -- could anyone help, please (even URL)? > > > 1) spin_lock_irqsave > > > 2) spin_lock_irq > > > 3) spin_lock > > > > > > > Please rename the structure so it is not mistaken for the actual buffer. > > > I have renamed some other structures and variables as well to blend them > > > in with existing code. > > > > spin_unlock(&acm->throttle_lock); > > > > - urb->actual_length = 0; > > - urb->dev = acm->dev; > > - > > - i = usb_submit_urb(urb, GFP_ATOMIC); > > - if (i) > > - dev_dbg(&acm->data->dev, "bulk rx resubmit %d\n", i); > > + spin_lock_irqsave(&acm->read_lock, flags); > > + list_add(&buf->list, &acm->spare_read_bufs); > > + spin_unlock_irqrestore(&acm->read_lock, flags); > > + goto next_buffer; > > + > > +urbs: > > + while (!list_empty(&acm->spare_read_bufs)) { > > + spin_lock_irq(&acm->read_lock); > > > > If you have code that mixes irqsave and irq spinlocks something is wrong. > > Interrupt handling code such as this can take the simplest form: > > spin_lock(&lock); > Ok, so what does that mean for me now? Should I change those to normal > spin_locks too? Locks I used are not there "by design" it's just the way > a similar piece of some other driver used them -- I needed to use locks, > but wasn't sure about how it's done in kernel programming, so I studied > other kernel code, how to use different types of locks. In the patch is > the result of my findings. I do not insist on them if they are combined > in a wrong manner. All locks in the patch are for simple internal data > structures, it should be very easy to use correct ones.
It is OK now. I'll send the patch onwards. A driver using spin_lock_irq() in a tasklet is buggy. You can always use irqsave. It will just be slower. But in a tasklet or interrupt handler spin_lock() will do. Basically these locks differ in whether they shut down irq processing on the local CPU. In interrupt it is down and must not be reenabled. Therefore spin_lock_irq() is deadly. Thank you Oliver ------------------------------------------------------- SF.Net email is sponsored by: Tame your development challenges with Apache's Geronimo App Server. Download it for free - -and be entered to win a 42" plasma tv or your very own Sony(tm)PSP. Click here to play: http://sourceforge.net/geronimo.php _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel