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

Reply via email to