Am Sonntag, 13. September 2009 01:35:01 schrieb Dave Taht:
> > static void usb_alphatrack_abort_transfers(struct usb_alphatrack *dev)
> > {
> > /* shutdown transfer */
> > if (dev->interrupt_in_running) {
> > dev->interrupt_in_running = 0;
> > if (dev->intf)
> > usb_kill_urb(dev->interrupt_in_urb);
> > }
> > if (dev->interrupt_out_busy)
> > if (dev->intf)
> > usb_kill_urb(dev->interrupt_out_urb);
> > }
> >
> > Don't do this conditionally. There's a small race where you perform
> > a wakeup on a freed queue.
>
> Can you suggest a usb driver to look at that is doing this right? (this
> same question applies to most other comments in your review). I had
> based this off of code that were the current examples of exemplary
> behavior back in ancient days.
Just about all drivers do this correctly. Simply and unconditionally kill
all urbs.
> > static int usb_alphatrack_open(struct inode *inode, struct file *file)
> > {
> > struct usb_alphatrack *dev;
> > int subminor;
> > int retval = 0;
> > struct usb_interface *interface;
> >
> > nonseekable_open(inode, file);
> > subminor = iminor(inode);
> >
> > mutex_lock(&disconnect_mutex);
> >
> > usbcore does this for you
>
> When did it start doing that?
When Alan added minor_rwsem. A few versions ago.
> > /**
> > * usb_alphatrack_poll
> > */
> > static unsigned int usb_alphatrack_poll(struct file *file, poll_table *
> > wait)
> >
> > should indicate an error condition in case of disconnection
This one is kind of important
> >
> > /* FIXME - there are more usb_alloc routines for dma correctness.
> > Needed? */
> > dev->ring_buffer =
> > kmalloc((true_size * sizeof(struct alphatrack_icmd)), GFP_KERNEL);
> >
> > questionable on cache coherency grounds
>
> When I wrote this I assumed that the only users would be x86 and x86_64,
> and maybe ppc. I guess now people are pushing x86 past 4GB, and this is
> also a problem here?
No, x86 is safe.
> Again, helps to see a driver that is doing it right. Suggestion?
Look at the high performance serial stuff, option and sierra.
Regards
Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
