On Sat, 3 Feb 2007, Prakash Punnoor wrote:
> Ok, here you go. I hope I did everything correctly. I applied the patch.
> recompiled and installed the modules unloaded present modules and put the
> newly compiled in. Attached the usbmon log and dmesg. calling
> sane-find-scanner 2x success, 2x failure, 1x success
This is great! I have found the problem... and it is indeed a programming
bug.
Oliver, here's what's happening. When the autoresume takes place, we
first resume the parent hub and then move on to resume the device itself.
Resuming the device involves reading the port status on the parent hub to
see whether the device has done a remote wakeup or has disconnected. (It
also involves reading the port status after the resume, to make sure the
resume succeeded -- but that doesn't matter here.)
At the same time khubd responds to the awakening of the hub, and it starts
reading various port statuses to see if anything has changed. So now we
have two different threads, the user app and khubd, both reading port
statuses on the same hub at the same time.
That's not a problem as far as the hardware is concerned. But both
threads call hub_port_status(), which reads the status into a dedicated
buffer at hub->status->port. Even though the URBs are serialized, on
non-cache-coherent architectures we could have problems with one thread
accessing the buffer while the other thread is doing DMA on it.
On x86_64 that's not a problem either. What _is_ a problem is that one
thread is trying to read the status of port 1 while the other thread is
reading the status of port 2. Port 2 isn't connected, and the other
thread sees that status in the buffer and hence thinks that port 1 isn't
connected either. So the resume fails and things start to go wrong.
Obviously we can't afford to use a single dedicated buffer like this.
Each thread reading a hub's port status will have to provide its own
buffer, or more simply, we will have to allocate a buffer dynamically for
every read instead of using a single static buffer. I don't like the
overhead of all those kmalloc and kfree calls for a 4-byte buffer, but the
simplicity is very attractive.
What do you think is the best way to solve this?
Prakash, the patch below implements the simple approach. I haven't
tested it myself, but you can go ahead and try it. It ought to eliminate
the problems you've been seeing.
Alan Stern
Index: usb-2.6/drivers/usb/core/hub.c
===================================================================
--- usb-2.6.orig/drivers/usb/core/hub.c
+++ usb-2.6/drivers/usb/core/hub.c
@@ -1422,18 +1422,24 @@ static int hub_port_status(struct usb_hu
u16 *status, u16 *change)
{
int ret;
+ struct usb_port_status *buffer;
- ret = get_port_status(hub->hdev, port1, &hub->status->port);
+ buffer = kmalloc(sizeof(*buffer), GFP_NOIO);
+ if (!buffer)
+ return -ENOMEM;
+
+ ret = get_port_status(hub->hdev, port1, buffer);
if (ret < 4) {
dev_err (hub->intfdev,
"%s failed (err = %d)\n", __FUNCTION__, ret);
if (ret >= 0)
ret = -EIO;
} else {
- *status = le16_to_cpu(hub->status->port.wPortStatus);
- *change = le16_to_cpu(hub->status->port.wPortChange);
+ *status = le16_to_cpu(buffer->wPortStatus);
+ *change = le16_to_cpu(buffer->wPortChange);
ret = 0;
}
+ kfree(buffer);
return ret;
}
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
