On 2012年05月17日 22:31, Alan Stern wrote:
On Thu, 17 May 2012, Lan Tianyu wrote:
Also, you should check to see whether the usb_lock_device() call is
really needed. If the caller already owns the lock for the parent hub
then you don't need to lock the child -- you don't even need the
usb_{get,put}_dev() calls.
How about this?
#define usb_get_hub_each_child(hdev, port1, child) \
for (port1 = 1, child = usb_get_hub_child(hdev, port1); \
port1<= hdev->maxchild; child = \
usb_get_hub_child(hdev, ++port1))
usb_get_hub_child(hdev, port1)
{
struct usb_hub *hub = hdev_to_hub(hdev);
return usb_get_dev(hub->port_data[port1 - 1].child);
}
For more general, usb_get_hub_child() increases child refcount
to prevent child from being removed during processing and the caller
should invoke usb_put_dev() when child will never be used.
Have you looked through the callers to see whether the child's refcount
really needs to be incremented? I think it probably doesn't. (Hint:
If the current code doesn't increment the refcount, that's a good sign
that the new code doesn't need to increment it either.)
In the r8a66597-hcd.c
static void collect_usb_address_map(struct usb_device *udev, unsigned long *map)
{
int chix;
if (udev->state == USB_STATE_CONFIGURED &&
udev->parent && udev->parent->devnum > 1 &&
udev->parent->descriptor.bDeviceClass == USB_CLASS_HUB)
map[udev->devnum/32] |= (1 << (udev->devnum % 32));
for (chix = 0; chix < udev->maxchild; chix++) {
struct usb_device *childdev =
usb_get_hub_child_device(udev, chix + 1);
if (childdev)
collect_usb_address_map(childdev, map);
}
}
The function will be invoked in the rh_timer_func().
The code path is usb_hcd_poll_rh_status() -> hcd->driver->hub_status_data()
-> r8a66597_hub_status_data() -> r8a66597_check_detect_child() ->
collect_usb_address_map().
The function is to traverse all the device tree of the hcd. There maybe
a case that after child pointer being passing to collect_usb_address_map()
as param, the child was removed and pointer became invaild. This will cause
oops and maybe small probability event. Does this make sense?
hi alan:
Is this ok for you?
Alan Stern
>
> Alan Stern
>
--
Best Regards
Tianyu Lan
linux kernel enabling team
--
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
