Hi,
this is the first version of the patch to fix module unload races.
It should fix all bugs David identified and a few more.
To do so it has to get the BKL at a few places more, but it holds
it for shorter amounts of time.
If you want to tell me that I did something terribly wrong,
please do so now, before I waste much time testing this version.
Regards
Oliver
PS: I have to know. Is there a keyboard somewhere at IBM
which you cut a notch into every time you kill a use of the BKL ?
You can import this changeset into BK by piping this whole message to:
'| bk receive [path to repository]' or apply the patch as usual.
===================================================================
[EMAIL PROTECTED], 2002-07-02 13:52:29+02:00, [EMAIL PROTECTED]
- sane BKL handling
diff -Nru a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
--- a/drivers/usb/core/devio.c Tue Jul 2 13:52:59 2002
+++ b/drivers/usb/core/devio.c Tue Jul 2 13:52:59 2002
@@ -722,14 +722,11 @@
if (test_bit(i, &ps->ifclaimed))
continue;
+ lock_kernel();
if (intf->driver) {
- const struct usb_device_id *id;
- down(&intf->driver->serialize);
- intf->driver->disconnect(ps->dev, intf->private_data);
- id = usb_match_id(ps->dev,intf,intf->driver->id_table);
- intf->driver->probe(ps->dev, i, id);
- up(&intf->driver->serialize);
+ usb_bind_driver(intf->driver,ps->dev, i);
}
+ unlock_kernel();
}
return 0;
@@ -1092,16 +1089,17 @@
/* disconnect kernel driver from interface, leaving it unbound. */
case USBDEVFS_DISCONNECT:
+ /* this function is voodoo. without locking it is a maybe thing */
+ lock_kernel();
driver = ifp->driver;
if (driver) {
- down (&driver->serialize);
dbg ("disconnect '%s' from dev %d interface %d",
driver->name, ps->dev->devnum, ctrl.ifno);
- driver->disconnect (ps->dev, ifp->private_data);
+ usb_unbind_driver(driver, ps->dev, ifp->private_data);
usb_driver_release_interface (driver, ifp);
- up (&driver->serialize);
} else
retval = -EINVAL;
+ unlock_kernel();
break;
/* let kernel drivers try to (re)bind to the interface */
@@ -1111,18 +1109,28 @@
/* talk directly to the interface's driver */
default:
+ lock_kernel(); /* against module unload */
driver = ifp->driver;
- if (driver == 0 || driver->ioctl == 0)
- retval = -ENOSYS;
- else {
- if (ifp->driver->owner)
+ if (driver == 0 || driver->ioctl == 0) {
+ unlock_kernel();
+ retval = -ENOSYS;
+ } else {
+ if (ifp->driver->owner) {
__MOD_INC_USE_COUNT(ifp->driver->owner);
+ unlock_kernel();
+ }
/* ifno might usefully be passed ... */
retval = driver->ioctl (ps->dev, ctrl.ioctl_code, buf);
/* size = min_t(int, size, retval)? */
- if (ifp->driver->owner)
+ if (ifp->driver->owner) {
__MOD_DEC_USE_COUNT(ifp->driver->owner);
+ } else {
+ unlock_kernel();
+ }
}
+
+ if (retval == -ENOIOCTLCMD)
+ retval = -ENOTTY;
}
/* cleanup and return */
@@ -1139,7 +1147,7 @@
static int usbdev_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
unsigned long arg)
{
struct dev_state *ps = (struct dev_state *)file->private_data;
- int ret = -ENOIOCTLCMD;
+ int ret = -ENOTTY;
if (!(file->f_mode & FMODE_WRITE))
return -EPERM;
diff -Nru a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
--- a/drivers/usb/core/hub.c Tue Jul 2 13:52:59 2002
+++ b/drivers/usb/core/hub.c Tue Jul 2 13:52:59 2002
@@ -1050,8 +1050,6 @@
static int usb_hub_thread(void *__hub)
{
- lock_kernel();
-
/*
* This thread doesn't need any user-level access,
* so get rid of all our resources
@@ -1071,8 +1069,6 @@
} while (!signal_pending(current));
dbg("usb_hub_thread exiting");
-
- unlock_kernel();
complete_and_exit(&khubd_exited, 0);
}
diff -Nru a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
--- a/drivers/usb/core/usb.c Tue Jul 2 13:52:59 2002
+++ b/drivers/usb/core/usb.c Tue Jul 2 13:52:59 2002
@@ -33,6 +33,7 @@
#include <linux/devfs_fs_kernel.h>
#include <linux/spinlock.h>
#include <linux/errno.h>
+#include <linux/smp_lock.h>
#ifdef CONFIG_USB_DEBUG
#define DEBUG
@@ -254,6 +255,86 @@
up (&usb_bus_list_lock);
}
+/**
+ * usb_unbind_driver - disconnects a driver from a device
+ * @driver: Driver to be disconnected
+ * @device: usb device to be disconnected
+ * @priv: private pointer passed to device driver
+ * Context: BKL held
+ *
+ * Handles module usage count correctly
+ */
+
+void usb_unbind_driver(struct usb_driver *driver, struct usb_device *device, void
+*priv)
+{
+ /* as soon as we increase the module use count we drop the BKL
+ before that we must not sleep */
+ if (driver->owner) {
+ __MOD_INC_USE_COUNT(driver->owner);
+ unlock_kernel();
+ }
+ down(&driver->serialize); /* if we sleep here on an umanaged driver
+ the holder of the lock guards against
+ module unload */
+
+ driver->disconnect(device, priv);
+
+ up(&driver->serialize);
+ if (driver->owner) {
+ lock_kernel();
+ __MOD_DEC_USE_COUNT(driver->owner);
+ }
+}
+
+/**
+ * usb_bind_driver - connect a driver to a device's interface
+ */
+
+void *usb_bind_driver(struct usb_driver *driver, struct usb_device *dev, unsigned int
+ifnum)
+{
+ int i;
+ void *private;
+ const struct usb_device_id *id;
+ struct usb_interface *interface;
+
+ if (driver->owner) {
+ __MOD_INC_USE_COUNT(driver->owner);
+ unlock_kernel();
+ }
+
+ interface = &dev->actconfig->interface[ifnum];
+
+ id = driver->id_table;
+ /* new style driver? */
+ if (id) {
+ for (i = 0; i < interface->num_altsetting; i++) {
+ interface->act_altsetting = i;
+ id = usb_match_id(dev, interface, id);
+ if (id) {
+ down(&driver->serialize);
+ private = driver->probe(dev,ifnum,id);
+ up(&driver->serialize);
+ if (private != NULL)
+ break;
+ }
+ }
+
+ /* if driver not bound, leave defaults unchanged */
+ if (private == NULL)
+ interface->act_altsetting = 0;
+ } else { /* "old style" driver */
+ down(&driver->serialize);
+ private = driver->probe(dev, ifnum, NULL);
+ up(&driver->serialize);
+ }
+ if (driver->owner) {
+ lock_kernel();
+ __MOD_DEC_USE_COUNT(driver->owner);
+ }
+
+ return private;
+}
+
/*
* This function is part of a depth-first search down the device tree,
* removing any instances of a device driver.
@@ -278,13 +359,7 @@
struct usb_interface *interface = &dev->actconfig->interface[i];
if (interface->driver == driver) {
- if (driver->owner)
- __MOD_INC_USE_COUNT(driver->owner);
- down(&driver->serialize);
- driver->disconnect(dev, interface->private_data);
- up(&driver->serialize);
- if (driver->owner)
- __MOD_DEC_USE_COUNT(driver->owner);
+ usb_unbind_driver(driver, dev, interface->private_data);
/* if driver->disconnect didn't release the interface */
if (interface->driver)
usb_driver_release_interface(driver, interface);
@@ -300,7 +375,7 @@
/**
* usb_deregister - unregister a USB driver
* @driver: USB operations of the driver to unregister
- * Context: !in_interrupt ()
+ * Context: !in_interrupt (), must be called with BKL held
*
* Unlinks the specified driver from the internal USB driver list.
*/
@@ -666,9 +741,7 @@
struct list_head *tmp;
struct usb_interface *interface;
void *private;
- const struct usb_device_id *id;
struct usb_driver *driver;
- int i;
if ((!dev) || (ifnum >= dev->actconfig->bNumInterfaces)) {
err("bad find_interface_driver params");
@@ -683,37 +756,12 @@
goto out_err;
private = NULL;
+ lock_kernel();
for (tmp = usb_driver_list.next; tmp != &usb_driver_list;) {
driver = list_entry(tmp, struct usb_driver, driver_list);
tmp = tmp->next;
- if (driver->owner)
- __MOD_INC_USE_COUNT(driver->owner);
- id = driver->id_table;
- /* new style driver? */
- if (id) {
- for (i = 0; i < interface->num_altsetting; i++) {
- interface->act_altsetting = i;
- id = usb_match_id(dev, interface, id);
- if (id) {
- down(&driver->serialize);
- private = driver->probe(dev,ifnum,id);
- up(&driver->serialize);
- if (private != NULL)
- break;
- }
- }
-
- /* if driver not bound, leave defaults unchanged */
- if (private == NULL)
- interface->act_altsetting = 0;
- } else { /* "old style" driver */
- down(&driver->serialize);
- private = driver->probe(dev, ifnum, NULL);
- up(&driver->serialize);
- }
- if (driver->owner)
- __MOD_DEC_USE_COUNT(driver->owner);
+ private = usb_bind_driver(driver, dev, ifnum);
/* probe() may have changed the config on us */
interface = dev->actconfig->interface + ifnum;
@@ -721,9 +769,11 @@
if (private) {
usb_driver_claim_interface(driver, interface, private);
up(&dev->serialize);
+ unlock_kernel();
return 0;
}
}
+ unlock_kernel();
out_err:
up(&dev->serialize);
@@ -1135,27 +1185,22 @@
info("USB disconnect on device %d", dev->devnum);
+ lock_kernel();
if (dev->actconfig) {
for (i = 0; i < dev->actconfig->bNumInterfaces; i++) {
struct usb_interface *interface =
&dev->actconfig->interface[i];
struct usb_driver *driver = interface->driver;
if (driver) {
- if (driver->owner)
- __MOD_INC_USE_COUNT(driver->owner);
- down(&driver->serialize);
- driver->disconnect(dev, interface->private_data);
- up(&driver->serialize);
+ usb_unbind_driver(driver, dev,
+interface->private_data);
/* if driver->disconnect didn't release the interface
*/
if (interface->driver)
usb_driver_release_interface(driver,
interface);
- /* we don't need the driver any longer */
- if (driver->owner)
- __MOD_DEC_USE_COUNT(driver->owner);
}
/* remove our device node for this interface */
put_device(&interface->dev);
}
}
+ unlock_kernel();
/* Free up all the children.. */
for (i = 0; i < USB_MAXCHILDREN; i++) {
@@ -1484,6 +1529,8 @@
EXPORT_SYMBOL(usb_reset_device);
EXPORT_SYMBOL(usb_connect);
EXPORT_SYMBOL(usb_disconnect);
+EXPORT_SYMBOL(usb_bind_driver);
+EXPORT_SYMBOL(usb_unbind_driver);
EXPORT_SYMBOL(__usb_get_extra_descriptor);
diff -Nru a/include/linux/usb.h b/include/linux/usb.h
--- a/include/linux/usb.h Tue Jul 2 13:52:59 2002
+++ b/include/linux/usb.h Tue Jul 2 13:52:59 2002
@@ -430,6 +430,10 @@
/* for when layers above USB add new non-USB drivers */
extern void usb_scan_devices(void);
+/* for probe/disconnect with correct module usage counting */
+void *usb_bind_driver(struct usb_driver *driver, struct usb_device *dev, unsigned int
+ifnum);
+void usb_unbind_driver(struct usb_driver *driver, struct usb_device *device, void
+*priv);
+
/* mostly for devices emulating SCSI over USB */
extern int usb_reset_device(struct usb_device *dev);
===================================================================
This BitKeeper patch contains the following changesets:
1.646
## Wrapped with gzip_uu ##
begin 664 bkpatch13844
M'XL(`!N4(3T``[59_6_;-A/^V?HK;BNP-W'CF*0^[<Q9WR;!NV!I4S0)L&(;
M#%FB;<&R:.@C:=\Y__N.I&0KMIPL66:TU0=YQ^>.=_<<U3=PD_&TWQ)Q=,M3
MXPW\++(<'WDB$GXX%7,>1TGQ]5"D$QS\+`0.=N7KKI;HCF:=/.4\ZXZBR6@6
M&SCKDY\'4\#!K-^BA^;J3?YMP?NMSV?_N[GX[V?#&`[EMAIL PROTECTED]$W[%<Q@,C-'L
M75CP^'"6"G\J%URNAI>,$$HILXEI.]1>,L\TK25UR"C$VY"YO?'(Z1F3E$_>
M:?%`S!^*,^(PAUJFP^RES6S3-DZ!'CJ6`X1UB=LE#*C9MUF?]=X2UB<$M('O
MFEP!;RWH$.,]_'/,)T8`'<C\A,/[7RX`I4-<9F+\`K9'/-/XM':2T7GFSS"(
M3XSC!I11$L1%R+O*HFZ1C0ZG:[P6L2VR)(YEN<N0,3/T0ML>.^@[^IA3=BEE
MQ"6,4IO:O:5E]WIN(Z(PE8HS*:9$@SJ>'G66S+1L:QDXC'/;8]0-1KY)[$<!
MU74&(N5KQ75,Q":>]R2F:=&`R:06,Y<!]9VQ[9J6%8YLVWTFII7B.B:*UK(G
M,87\-A*;J-PE]2S7PR`?4SXV&1TQ3D;4?!ZJFNHZ+M-UB*E2=Y=$<R:_&/5F
M3C\.U*$NL:E))%!"')7BIOTPPVG?_EL9SDSH4/O5<APPR^?^C`,B'V?X+X<)
M3W@:!;!(Q0@-B;)`)`D/<ICR>(%&EE+CZ"O,15C$*)O$P@\A]0.>P5V43R$2
M01ZOJ\9:0@_P-!4II#POTB3#FJ+W[Q(ZZ9WZ@S7BT\ZM?$&].7>9!=1HM6(1
MS(8SGB8\WML_,DY=YH"#PR95PRU<:SB*DG"H%]^+DGS<.=8/!XL,;_GM`40H
MBC),R4CC'R@]IZ1G`3-`_UK=-N33*(-QD01Y)!+`^ULA0B$.E;-$D8-4@8Z"
M*)>C/F[)MQ&78OBNW6T`CFNXN/PII1@P%-?4UU:K7%4:4B1U4THC8&W%>-$Y
M7N!;/^?#T,]]I9?*>%3ZK%WF4<S:;6<"VNE/_"C)\HVX0`-0+[7!4L(>V)5O
MJE\TAA(?YBD06"[+W>\<ZX"1;_?A3[5%FX#P'4;2K8^SH'/V\?+JRY5\>0\\
MQFA6,E*_,K?2*NXPQJ5"":B'F]7:H?E>0F>F=@G3=C^N#B/-5)*U]7>HEK-M
M-=O02BL[M"'GER?7%R<?3O>W3+R^_J+VRBKW2EU;&*PRI^IS&DNB*NU/%\1G
M4,N3Y;#&)C;KT1[K$7M);<9ZNABZ+RN&!#K6*[8[<W&KVYT0-_3.3T-9FS3G
M/56;E($OJ$R8R+:)\8=75Q:-Q@U3_<'3&_:,_N3)#:NU)%B:*3:DA"TM![-7
M;9BL-B_:L1[#+7M=^O+#D(<KUI(D-,;.'^8<2VN(91>99I/*9!LKNZNG=E5Y
MX25\@^Q.C3=EYPD_ZM8SFR^&L@P<3H^-<V8[X!&CVVX;T&YME6NT:PU7,D+Y
M>IR*N7Q"*@RXE'RG!_IPJB?D`I`YUK(\U+.40%_R0BF\:Z9DA#Z4O``+@54%
MU2[\+$,OHTPIK9>5$B<"9WS-^_J@P&.I1K[_6;(_-@05&63^A$,@"JQ2Z-P4
M5XR_&9(<?C=N110V4%:6IP7V'7*@-+]=L5A]2`-JZ^L!*&5M:<"^@=57TE(&
MF4#BQ>L=!]R5E/N9I%>^!E=!NY.FB84:1(,,2:@CCD$DY_MJ?%X@Q24BARSF
M?*'X>4U@-3)HM8;##Y>GP_./)\.;J[/AR>7-Q^N-:4=-%(O4T)(E:.^':C(>
MAB,_CO[/D6>E1;@>`M'K3SEBD]8E4,S]!+T<5KLCJ4<:((V9BCA$#XJQ>I(K
MPJ20-:YB[=7L+?;^'>&40-;ALE?Y6WGZ2$XJ%HV(=[IGBQ.UOT[/'O77O7&/
MB]42YV':5-WJ*F4P9JN$^4\&*IS'ODJ>K@$Z]-J;?=^S(^\`W95%DP1]+VDX
M&B?%7(6?>D+4ZZC$M,)GA(E1M*5L*&=%(4ZH#:TPXU!UJQS^^F&G`)>+#>`'
M!-4Y]H,<T8ZC"79CU>!ORL(_-(H0!NM^+1SF_BB6)F*<)OP.;?P65_7BIU6V
M1*'&*LOS7H0*R!%$\.-Z?SK'N,#0C_.,YSGVP3C\]JV6P::Z-@WAU::AIDBU
M5PJ5]-Y<?N5!O^[IKK<2Q-M0-V)U.*W=B:=&J[JX-EAQB]*M7')0:=V=#>62
ME:KO!O#QYN)B7^??"&O3K.P/Y5_T;YGP92#*PC/"4A4>0,Q];%A"/O8+M!\C
M,%`4&NHC0WV-06V-QUQ'9!24C:MLZ+_'HJ$W\/MJ?:7[,2<]YB*=&`<:C`K"
M74ZZ?_6J@;&M3YRPRD+IWE/F47"1D#UW?09L/CH]#*#MPY-)U$%!7^K,^%V4
MZ"1.BT4.>_L'FD.0?@,_CG'#U*%YQ9^GCM.3ASO'E<?2<\>3W<36$=#I$6`(
MW*6.`K[V^V8U>XA?E29Y@*V.,]M'/'DLQJ&FLY_I-F'!(XB)QSIY%"F]^`_<
M*#_JX*D(E=G>#AB6YV"S?/;KI\O/U\.K+Q_>7U[L;5B-\[;''P#:U^>CAN^#
MS;WV2[].;C;;.S](KCMM/!C9KOX4[+VLT7[5+\&RS0YY$/NI+[]@Z*9ZU6DW
M=-?Z@^I&>]U@]TMZ:\MD8"'_-W;V.I/*_K*A]2P_J/R;K'_TK[6SDFU7_VT1
73'DPRXKY@-#0]3S'-?X"D>"L3C49````
`
end
-------------------------------------------------------
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
