Am Montag, 9. Dezember 2002 22:16 schrieb Greg KH:
> On Mon, Dec 09, 2002 at 09:18:40PM +0100, Oliver Neukum wrote:
> > Hi,
> >
> > could anybody tell me how usb_device_read protects itself
> > against devices going away? It takes usb_bus_list_lock, but
> > that seems to help only against busses going away.
>
> It doesn't do a good job of this :)
> I would _really_ like to convert this to the seq_file interface someday,
> but when I initially looked at it, it didn't look like a simple
> change...
>
> Any fixes for this would be gladly accepted.
Well, this isn't easy to do well without deep surgery.
Here's a patch that fixes the problem but isn't pretty.
It's just for review and please note, my kernel still
doesn't compile.
Perhaps we should allocate the buffers before we go down
the tree ? IMHO using BKL is more or less inevitable
in this case in any fix.
People from IBM will soon come down on me for
abuse of BKL ;-)
Regards
Oliver
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-12-09 22:59:41+01:00, [EMAIL PROTECTED]
- fix a race between usbfs and disconnect
devices.c | 86 +++++++++++++++++++++++++++++++++-----------------------------
1 files changed, 47 insertions(+), 39 deletions(-)
diff -Nru a/drivers/usb/core/devices.c b/drivers/usb/core/devices.c
--- a/drivers/usb/core/devices.c Mon Dec 9 23:00:56 2002
+++ b/drivers/usb/core/devices.c Mon Dec 9 23:00:56 2002
@@ -87,7 +87,7 @@
static char *format_bandwidth =
/* B: Alloc=ddd/ddd us (xx%), #Int=ddd, #Iso=ddd */
"B: Alloc=%3d/%3d us (%2d%%), #Int=%3d, #Iso=%3d\n";
-
+
static char *format_device1 =
/* D: Ver=xx.xx Cls=xx(sssss) Sub=xx Prot=xx MxPS=dd #Cfgs=dd */
"D: Ver=%2x.%02x Cls=%02x(%-5s) Sub=%02x Prot=%02x MxPS=%2d #Cfgs=%3d\n";
@@ -99,7 +99,7 @@
static char *format_config =
/* C: #Ifs=dd Cfg#=dd Atr=xx MPwr=dddmA */
"C:%c #Ifs=%2d Cfg#=%2d Atr=%02x MxPwr=%3dmA\n";
-
+
static char *format_iface =
/* I: If#=dd Alt=dd #EPs=dd Cls=xx(sssss) Sub=xx Prot=xx Driver=xxxx*/
"I: If#=%2d Alt=%2d #EPs=%2d Cls=%02x(%-5s) Sub=%02x Prot=%02x Driver=%s\n";
@@ -118,6 +118,9 @@
static DECLARE_WAIT_QUEUE_HEAD(deviceconndiscwq);
static unsigned int conndiscevcnt = 0;
+/* temporary buffer for usb string handling */
+static char temp_string_buffer[128];
+
/* this struct stores the poll state for <mountpoint>/devices pollers */
struct usb_device_status {
unsigned int lastev;
@@ -346,7 +349,7 @@
}
/*
- * Dump the different strings that this device holds.
+ * Dump the different strings that this device holds. Needs BKL.
*/
static char *usb_dump_device_strings (char *start, char *end, struct usb_device *dev)
{
@@ -354,29 +357,25 @@
if (start > end)
return start;
- buf = kmalloc(128, GFP_KERNEL);
- if (!buf)
- return start;
if (dev->descriptor.iManufacturer) {
- if (usb_string(dev, dev->descriptor.iManufacturer, buf, 128) > 0)
- start += sprintf(start, format_string_manufacturer, buf);
- }
+ if (usb_string(dev, dev->descriptor.iManufacturer, temp_string_buffer,
+128) > 0)
+ start += sprintf(start, format_string_manufacturer,
+temp_string_buffer);
+ }
if (start > end)
goto out;
if (dev->descriptor.iProduct) {
- if (usb_string(dev, dev->descriptor.iProduct, buf, 128) > 0)
- start += sprintf(start, format_string_product, buf);
+ if (usb_string(dev, dev->descriptor.iProduct, temp_string_buffer, 128)
+> 0)
+ start += sprintf(start, format_string_product,
+temp_string_buffer);
}
if (start > end)
goto out;
#ifdef ALLOW_SERIAL_NUMBER
if (dev->descriptor.iSerialNumber) {
- if (usb_string(dev, dev->descriptor.iSerialNumber, buf, 128) > 0)
- start += sprintf(start, format_string_serialnumber, buf);
+ if (usb_string(dev, dev->descriptor.iSerialNumber, temp_string_buffer,
+128) > 0)
+ start += sprintf(start, format_string_serialnumber,
+temp_string_buffer);
}
#endif
out:
- kfree(buf);
return start;
}
@@ -386,14 +385,14 @@
if (start > end)
return start;
-
+
start = usb_dump_device_descriptor(start, end, &dev->descriptor);
if (start > end)
return start;
-
+
start = usb_dump_device_strings (start, end, dev);
-
+
for (i = 0; i < dev->descriptor.bNumConfigurations; i++) {
if (start > end)
return start;
@@ -453,17 +452,17 @@
char *pages_start, *data_end, *speed;
unsigned int length;
ssize_t total_written = 0;
-
+
/* don't bother with anything else if we're not writing any data */
if (*nbytes <= 0)
return 0;
-
+
if (level > MAX_TOPO_LEVEL)
return total_written;
/* allocate 2^1 pages = 8K (on i386); should be more than enough for one
device */
- if (!(pages_start = (char*) __get_free_pages(GFP_KERNEL,1)))
+ if (!(pages_start = (char*) __get_free_pages(GFP_ATOMIC,1)))
return -ENOMEM;
-
+
if (usbdev->parent && usbdev->parent->devnum != -1)
parent_devnum = usbdev->parent->devnum;
/*
@@ -511,22 +510,35 @@
/ max,
bus->bandwidth_int_reqs,
bus->bandwidth_isoc_reqs);
-
+
}
data_end = usb_dump_desc(data_end, pages_start + (2 * PAGE_SIZE) - 256,
usbdev);
-
+
if (data_end > (pages_start + (2 * PAGE_SIZE) - 256))
data_end += sprintf(data_end, "(truncated)\n");
-
+
+
+ /* Now look at all of this device's children. */
+ for (chix = 0; chix < usbdev->maxchild; chix++) {
+ if (usbdev->children[chix]) {
+ ret = usb_device_dump(buffer, nbytes, skip_bytes, file_offset,
+usbdev->children[chix],
+ bus, level + 1, chix, ++cnt);
+ if (ret == -EFAULT)
+ goto err_out;
+ total_written += ret;
+ }
+ }
+
+
length = data_end - pages_start;
- /* if we can start copying some data to the user */
+ /* we copy now when a device going away no longer changes the result */
if (length > *skip_bytes) {
length -= *skip_bytes;
if (length > *nbytes)
length = *nbytes;
if (copy_to_user(*buffer, pages_start + *skip_bytes, length)) {
free_pages((unsigned long)pages_start, 1);
-
+
if (total_written == 0)
return -EFAULT;
return total_written;
@@ -538,19 +550,11 @@
*skip_bytes = 0;
} else
*skip_bytes -= length;
-
+
+err_out:
free_pages((unsigned long)pages_start, 1);
-
- /* Now look at all of this device's children. */
- for (chix = 0; chix < usbdev->maxchild; chix++) {
- if (usbdev->children[chix]) {
- ret = usb_device_dump(buffer, nbytes, skip_bytes, file_offset,
usbdev->children[chix],
- bus, level + 1, chix, ++cnt);
- if (ret == -EFAULT)
- return total_written;
- total_written += ret;
- }
- }
+
+
return total_written;
}
@@ -570,6 +574,8 @@
/* enumerate busses */
down (&usb_bus_list_lock);
+ /* prevent devices from going away - no sleeping after this point until we
+copy to user space*/
+ lock_kernel();
for (buslist = usb_bus_list.next; buslist != &usb_bus_list; buslist =
buslist->next) {
/* print devices for this bus */
bus = list_entry(buslist, struct usb_bus, bus_list);
@@ -577,11 +583,13 @@
ret = usb_device_dump(&buf, &nbytes, &skip_bytes, ppos, bus->root_hub,
bus, 0, 0, 0);
if (ret < 0) {
up(&usb_bus_list_lock);
+ unlock_kernel();
return ret;
}
total_written += ret;
}
up (&usb_bus_list_lock);
+ unlock_kernel();
return total_written;
}
@@ -598,7 +606,7 @@
unlock_kernel();
return POLLIN;
}
-
+
/* we may have dropped BKL - need to check for having lost the race */
if (file->private_data) {
kfree(st);
===================================================================
This BitKeeper patch contains the following changesets:
1.1086
## Wrapped with gzip_uu ##
begin 664 bkpatch3625
M'XL(`)@2]3T``ZU668_;.`Q^CG\%%WW89'))\I6DFZ+MI.T6O08]GCJ%H=AR
M8L2Q`DEN.MCTOR\E968ZZ/38;0,C%$6*^D1]I'T'WFFA9AU95Q^%"N[`WU(;
M5$4C&S%:RZVHJZ;]-))JA<;74J)Q;*?'?L5XN1D:)80>KY18L3A`KS-N\C6@
M4<\Z=!1>S9B+G9AU7C]Z\N[Y@]=!,)_#Z9HW*_%&&)C/@^7F?M&*>K11DJ_M
MAH<K\X$10BEE,0GCA,8'-@G#Z$`3LBQP6+!T6BZ3:>`AW;\-_,U8C#(RI9,P
M90S5*$J"!=`1)9,$"!M3-B938&P63V<1[1,Z(P2^$QOZ%(8D>`B_?H+3((<A
ME-4GX*!X+F`IS%Z(!EJ]+#7PIH"BTKEL&I&;X!DP/`0+SJX3&0S_XR\(""?!
MO5NP%\J>6(]QZW$A/E:YT*/\^AP19C`Y,)8R>N"3>,+2.)[$G"PGY?1[V;H1
M-Y=*W`SNKH;1>!J1`TUC$CNB?'O-[<SY!>R!)?)]'RJ7VQ_")2$^<4PFASB.
MH]0QB;&O>$1^AD=1"L-P^AN9U&H!#Y\]'R"=5K5<\AJ6;5D*Y9CTY/%9]N#M
MJQ=/3\'(6TB'$0"4X$75K,"L!1Q/CJZU`%E^24IGN<E-?WNO8*CV[D&NG7WG
M(O\'<Q=3`C1XZO[/@P4ES*I>G..`$0B#\0D8L=U)Q=7%Y>E+J2QXT$;9LV%J
MB]H.3L:!-MQ4.>1KKMRZS/MD?N5[RB8?[MK-PFAJ-_,"3F#1;G<^295U%(TY
M1M<XRPW^5?HR36M9%WH$+X4HM+V>$8:+4\2Z"!.*XFF8A"@ZG:J$+N(\8NCB
M\H&-,;Q7")VK:F>D&E4O>-.6/#>M$FIP"^8!(.@>W`/2PY`=/*`RT)^#WJ&3
M*;MN8F!SLN7F<NGV!T%[=X/.9XLW!6;Q3E#\)-XS)8LV-[\-ZN[;\1#E(DPC
M!S&-?Q[B&Z$J7K]LM\O?F%+M@C;?#.K!6CHMPHDGEQ.6;-/(Z4XX/?%ZXO4H
M=KH75D]<87CA],CK5L#Q9S/Q1W?'5T)G_@1SZ%K>G_0@RU;"9"6^V#/GT+WN
M%0/:Z_5L2+]E<MPRIFX++YR>>CT]ZLQ!<B+"B7-,');F2[F'6LH-8(GPNK9=
MY8M*^5-C(59U@>4TLL79L86+&+%5S8'<!3?ZRY:RN\(M_^3<O:'?[\$_US?N
M/"ZCO;<.'[R]H^SWAPV2^5VS`FNY>WG3S?+""#T`O:EVV7%L&V`FRU(+O.7;
M8P]LY$YGV:)[+3Z*&OI`!P[8`/K]O#&VA#PZ!V`.PT>/'[Q[_M91JK.2V).%
M4IELC7,TTO`ZVZO*&/P>0++A*FOX;`OQW*>8^11;89.[%Y#+W04TF.3]&E?Q
MRPZTDK;=\3VW1KP`?+4HV_-0:M?%E-!M;6S.,>S4ASV2,8ZHTU$PU(\89]80
M`K66.'06;,%QRFS5(9:=PB1@4[QZARBY_1+&T`+1M1`[-U4:!.28L$,?`VUC
MJOKJ0)@:?+,IK#=\75E>U#+?9!NA&E%W,:^XKP6+66N;KTP3R]"O#8N$N'-Y
<<7[](9NO1;[1[7;.18Z?C"$/_@5LGYRJ-0L`````
`
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
