Hi,
[about the oops in ohci when a scanner is disconnected]
On Fri, Feb 28, 2003 at 09:07:17AM -0800, Greg KH wrote:
> On Thu, Feb 27, 2003 at 11:15:34AM +0100, Henning Meier-Geinitz wrote:
> > Hi,
> >
> > On Wed, Feb 26, 2003 at 03:12:03PM -0800, Greg KH wrote:
> > > On Wed, Feb 26, 2003 at 10:37:30PM +0100, Henning Meier-Geinitz wrote:
> > > > Unable to handle kernel NULL pointer dereference at virtual address 00000000
> > > > printing eip:
> > > > c01e1dd7
> > > > *pde = 00000000
> > > > Oops: 0000
> > > > CPU: 0
> > > > EIP: 0060:[<c01e1dd7>] Not tainted
> > > > EFLAGS: 00010082
> > > > EIP is at dl_reverse_done_list+0x163/0x194
> > > > eax: 00000000 ebx: dfd72080 ecx: dfd7b0c0 edx: 00000014
> > > > esi: dfd7b0e0 edi: df73f2bc ebp: c02b1efc esp: c02b1eb8
> > > > ds: 007b es: 007b ss: 0068
> > > > Process swapper (pid: 0, threadinfo=c02b0000 task=c027bde0)
> > > > Stack: dfbbb15e df73f2bc c1524eec dfbbb004 00000003 c0262c60 00000005 ffffff92
> > > > e0800010 dfe0e000 00000002 00000014 00000000 00000000 00000000 00000005
> > > > 00000246 c02b1f2c c01e2c10 dfe0e000 c02b1f90 e0800014 c026324c 0000023a
> > > > Call Trace:
> > > > [<c01e2c10>] ohci_irq+0x130/0x1d8
> > >
> > > David, any clues?
> >
> > The crash doesn't happen if I move back the irq unlink from
> > destroy_scanner to disconnect_scanner:
>
> Which points at the ohci driver not allowing usb_unlink_urb() to be
> called after the device is disconnected. David, is this true?
Ok, what's the result of the discussion? Should I wait for changes in
ohci/usbcore or just send the patch for the scanner driver that moves
the irq urb unlink to the disconnect function?
Anyway, I'll just append it (against 2.5.64) :-)
--------
Used kobject reference counting to free the scn struct when the device
is closed and disconnected. Avoids crashes when writing to a
disconnected device. (Thanks to Greg KH).
I've also changed irq_scanner to avoid submitting new URBs when the
old one returned with an error. Without this change irq_scanner gets
called ever and ever again after a disconnect while open.
Bye,
Henning
diff -u linux-2.5.64/drivers/usb/image/scanner.c
linux-2.5.64-nohang2/drivers/usb/image/scanner.c
--- linux-2.5.64/drivers/usb/image/scanner.c 2003-03-11 11:19:47.000000000 +0100
+++ linux-2.5.64-nohang2/drivers/usb/image/scanner.c 2003-03-11 13:18:54.000000000
+0100
@@ -1,7 +1,7 @@
/* -*- linux-c -*- */
/*
- * Driver for USB Scanners (linux-2.5.64)
+ * Driver for USB Scanners (linux-2.5)
*
* Copyright (C) 1999, 2000, 2001, 2002 David E. Nelson
* Copyright (C) 2002, 2003 Henning Meier-Geinitz
@@ -350,6 +350,9 @@
* - Added vendor/product ids for Artec, Avision, Brother, Medion, Primax,
* Prolink, Fujitsu, Plustek, and SYSCAN scanners.
* - Fixed generation of devfs names if dynamic minors are disabled.
+ * - Used kobject reference counting to free the scn struct when the device
+ * is closed and disconnected. Avoids crashes when writing to a
+ * disconnected device. (Thanks to Greg KH).
*
* TODO
* - Performance
@@ -427,6 +430,7 @@
return;
default:
dbg("%s - nonzero urb status received: %d", __FUNCTION__, urb->status);
+ return;
}
dbg("irq_scanner(%d): data:%x", scn->scn_minor, *data);
@@ -461,6 +465,7 @@
return -ENODEV;
}
scn = usb_get_intfdata(intf);
+ kobject_get(&scn->kobj);
dev = scn->scn_dev;
@@ -521,6 +526,8 @@
up(&scn_mutex);
up(&(scn->sem));
+ kobject_put(&scn->kobj);
+
return 0;
}
@@ -813,6 +820,37 @@
return retval;
}
+static void destroy_scanner (struct kobject *kobj)
+{
+ struct scn_usb_data *scn;
+
+ dbg ("%s", __FUNCTION__);
+
+ scn = to_scanner(kobj);
+
+ down (&scn_mutex);
+ down (&(scn->sem));
+
+ usb_driver_release_interface(&scanner_driver,
+ &scn->scn_dev->actconfig->interface[scn->ifnum]);
+
+ kfree(scn->ibuf);
+ kfree(scn->obuf);
+
+ dbg("%s: De-allocating minor:%d", __FUNCTION__, scn->scn_minor);
+ devfs_unregister(scn->devfs);
+ usb_deregister_dev(1, scn->scn_minor);
+ usb_free_urb(scn->scn_irq);
+ usb_put_dev(scn->scn_dev);
+ up (&(scn->sem));
+ kfree (scn);
+ up (&scn_mutex);
+}
+
+static struct kobj_type scanner_kobj_type = {
+ .release = destroy_scanner,
+};
+
static struct
file_operations usb_scanner_fops = {
.owner = THIS_MODULE,
@@ -982,6 +1020,8 @@
return -ENOMEM;
}
memset (scn, 0, sizeof(struct scn_usb_data));
+ kobject_init(&scn->kobj);
+ scn->kobj.ktype = &scanner_kobj_type;
scn->scn_irq = usb_alloc_urb(0, GFP_KERNEL);
if (!scn->scn_irq) {
@@ -1049,6 +1089,7 @@
}
+ usb_get_dev(dev);
scn->bulk_in_ep = have_bulk_in;
scn->bulk_out_ep = have_bulk_out;
scn->intr_ep = have_intr;
@@ -1089,28 +1130,13 @@
intf->kdev = NODEV;
usb_set_intfdata(intf, NULL);
- if (scn) {
- down (&scn_mutex);
- down (&(scn->sem));
-
- if(scn->intr_ep) {
- dbg("disconnect_scanner(%d): Unlinking IRQ URB",
scn->scn_minor);
- usb_unlink_urb(scn->scn_irq);
- }
- usb_driver_release_interface(&scanner_driver,
- &scn->scn_dev->actconfig->interface[scn->ifnum]);
-
- kfree(scn->ibuf);
- kfree(scn->obuf);
-
- dbg("disconnect_scanner: De-allocating minor:%d", scn->scn_minor);
- devfs_unregister(scn->devfs);
- usb_deregister_dev(1, scn->scn_minor);
- usb_free_urb(scn->scn_irq);
- up (&(scn->sem));
- kfree (scn);
- up (&scn_mutex);
+ if(scn->intr_ep) {
+ dbg("%s(%d): Unlinking IRQ URB", __FUNCTION__, scn->scn_minor);
+ usb_unlink_urb(scn->scn_irq);
}
+
+ if (scn)
+ kobject_put(&scn->kobj);
}
/* we want to look at all devices, as the vendor/product id can change
diff -u linux-2.5.64/drivers/usb/image/scanner.h
linux-2.5.64-nohang2/drivers/usb/image/scanner.h
--- linux-2.5.64/drivers/usb/image/scanner.h 2003-03-11 11:19:47.000000000 +0100
+++ linux-2.5.64-nohang2/drivers/usb/image/scanner.h 2003-03-11 13:33:00.000000000
+0100
@@ -1,5 +1,5 @@
/*
- * Driver for USB Scanners (linux-2.5.64)
+ * Driver for USB Scanners (linux-2.5)
*
* Copyright (C) 1999, 2000, 2001, 2002 David E. Nelson
* Previously maintained by Brian Beattie
@@ -335,7 +335,9 @@
wait_queue_head_t rd_wait_q; /* read timeouts */
struct semaphore sem; /* lock to prevent concurrent reads or writes */
unsigned int rd_nak_timeout; /* Seconds to wait before read() timeout. */
+ struct kobject kobj; /* Handles our reference counting */
};
+#define to_scanner(d) container_of(d, struct scn_usb_data, kobj)
extern devfs_handle_t usb_devfs_handle;
-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
