On Fri, Apr 16, 2010 at 9:26 AM, Jeremy Ramer <jdramer@xxxxxxxxx> wrote:
> Hi,
> I am seeing a very strange kernel crashing issue with my linux driver.
> The strange thing is most of the time the driver works fine. I can
> read and write my USB device with no issues. Then usually one day a
> week, seeming to be on a rotating schedule of every 8 days or so,
> every time I try to read from the device with usb_bulk_msg() the
> kernel crashes hard. There is no response to anything. Removing the
> USB device does nothing. Magic SysRq commands do nothing. The only way
> to recover is turning off the power. This will happen for the full
> day. The next day reads work fine.
>
> I have seen this on 3 different computers. Two laptops running Kubuntu
> 9.10, kernel version 2.6.31-20 and a desktop running OpenSuse 11.0,
> kernel version 2.6.25.16. The HCD is EHCI. The host vendors are Intel
> 82801I (ICH9) and Intel ICH10. These are all 64-bit SMP systems.
>
> I have traced the position of the crash to be somewhere inside
> usb_bulk_msg(). I am working on tracing through that code but as the
> crash comes and goes I have not had the chance yet. One thing about my
> usage of usb_bulk_msg(). The read size is larger than the maximum
> packet size. This is to increase the efficiency of the read which is
> usually MBs in size. My review of source code shows that this is not
> usually done, but I have not seen any documentation that specifically
> forbids it. I could submit the URB myself but I would be just
> duplicating the code inside usb_bulk_msg(). Is there any reason the
> max packet size can not be exceeded with this function?
>
> Here's the code for the read operation.
>
> #define MYDEV_TIMEOUT 250
> #define BUF_SIZE_IN 3110400+10
> static ssize_t mydev_read (struct file *file, char __user *buf, size_t
> count, loff_t *ppos)
> {
> struct mydev *dev = (struct mydev *)file->private_data;
> ssize_t retval = 0;
> int actual;
>
> if (dev == NULL)
> return -ENODEV;
>
> if (count > BUF_SIZE_IN) {
> printk(KERN_INFO "count %zd too large, trimmed to %d\n",
> count, BUF_SIZE_IN);
> count = BUF_SIZE_IN;
> }
>
> mutex_lock(&dev->io_mutex);
> retval = usb_bulk_msg(dev->udev,
> usb_rcvbulkpipe(dev->udev, dev->bulk_in),
> dev->inbuf, count+2, &actual, MYDEV_TIMEOUT);
> mutex_unlock(&dev->io_mutex);
>
> if (retval < 0)
> return retval;
>
> if (actual > 0) {
> int framing = dev->inbuf[actual-1];
>
> if (framing > actual)
> return -EIO;
>
> actual -= framing;
> }
>
> if (copy_to_user(buf, dev->inbuf, actual))
> return -EFAULT;
>
> return actual;
> }
>
> Thanks!
> Jeremy
>
I have traced this hang as far as the function map_urb_for_dma()
called in usb_hcd_submit_urb(). Since it seems DMA-related I have
made the following patch to my driver that seems to have fixed the
hang. Are there any concerns with allocating the DMA coherent memory
in the probe? Is it better to do this allocation on every transfer?
diff --git a/kernel-module/mydev.c b/kernel-module/mydev.c
index c29e253..8d8080a 100644
--- a/kernel-module/mydev.c
+++ b/kernel-module/mydev.c
@@ -76,6 +76,8 @@ struct mydev {
char *msgdata; /* control message data buffer */
unsigned char * inbuf; /* buffer to hold incoming packet */
unsigned char * outbuf; /* buffer to hold incoming packet */
+ dma_addr_t in_dma; /* dma addr for in buffer */
+ dma_addr_t out_dma; /* dma addr for out buffer */
int full; /* is the full interface in use? */
unsigned int bulk_in; /* address of bulk in endpoint */
unsigned int bulk_out; /* address of bulk out endpoint */
@@ -87,10 +89,10 @@ struct mydev {
static void mydev_delete (struct kref *kref)
{
struct mydev *dev = to_mydev(kref);
+ usb_buffer_free(dev->udev, BUF_SIZE_IN, dev->inbuf, dev->in_dma);
+ usb_buffer_free(dev->udev, BUF_SIZE_OUT, dev->outbuf, dev->out_dma);
usb_put_dev(dev->udev);
kfree(dev->msgdata);
- kfree(dev->inbuf);
- kfree(dev->outbuf);
kfree(dev);
}
@@ -593,8 +595,8 @@ static int mydev_probe(struct usb_interface
*interface, const struct usb_device_
/* Allocate buffers */
dev->msgdata = kmalloc(16, GFP_KERNEL);
- dev->inbuf = kmalloc(BUF_SIZE_IN, GFP_KERNEL);
- dev->outbuf = kmalloc(BUF_SIZE_OUT, GFP_KERNEL);
+ dev->inbuf = usb_buffer_alloc(dev->udev, BUF_SIZE_IN, GFP_KERNEL,
&dev->in_dma);
+ dev->outbuf = usb_buffer_alloc(dev->udev, BUF_SIZE_OUT, GFP_KERNEL,
&dev->out_dma);
if (!(dev->msgdata && dev->inbuf && dev->outbuf)) {
retval = -ENOMEM;
dev_err(&interface->dev,
Thanks!
Jeremy
--
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
