Message ID | 200809271541.15963.rjw@sisk.pl |
---|---|
State | Superseded, archived |
Delegated to: | David Miller |
Headers | show |
Hi Rafael, > > >> Rafael, can you pull from my tree and test the changes: > > >> > > >> git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/ > > >> bluetooth-2.6.git > > >> > > >> It would be interesting if these fixes are enough. > > > > > > They appear to be enough. I haven't had any suspend/resume failures > > > with them > > > applied. > > > > so it works _without_ applying patch-btusb-suspend. > > Well, unfortunately I spoke too soon. > > I'm still seeing post-hibernation crashes triggered by the bluetooth user land > trying to use the device handled by btusb. They happen every second > hibernation, more or less, and apparently they are oopses in various code > paths not directly related to bluetooth, like ext3 (memory corruption or > what?). I pushed two extra patches to my bluetooth-2.6 repository: git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-2.6.git One is fixing a double-free in the error path. This error path can be triggered during suspend/resume if the USB core just disconnects the device. Please check if that fixes it for you. > With patch-btusb-suspend applied I don't see them (actually I have to use > a slightly modified version of the patch which is appended). > > Interestingly enough, suspend to RAM works without any visible problems. As Oliver said, the USB core should do the right thing when no suspend and resume callbacks are provided. I looked through the code so many times now and I am running out of ideas what can happen. Lets try it one last time without the suspend patch, but the double free fix and see if that works. Otherwise I really give up. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday, 30 of September 2008, Marcel Holtmann wrote: > Hi Rafael, > > > > >> Rafael, can you pull from my tree and test the changes: > > > >> > > > >> git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/ > > > >> bluetooth-2.6.git > > > >> > > > >> It would be interesting if these fixes are enough. > > > > > > > > They appear to be enough. I haven't had any suspend/resume failures > > > > with them > > > > applied. > > > > > > so it works _without_ applying patch-btusb-suspend. > > > > Well, unfortunately I spoke too soon. > > > > I'm still seeing post-hibernation crashes triggered by the bluetooth user land > > trying to use the device handled by btusb. They happen every second > > hibernation, more or less, and apparently they are oopses in various code > > paths not directly related to bluetooth, like ext3 (memory corruption or > > what?). > > I pushed two extra patches to my bluetooth-2.6 repository: > > git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-2.6.git > > One is fixing a double-free in the error path. This error path can be > triggered during suspend/resume if the USB core just disconnects the > device. Please check if that fixes it for you. > > > With patch-btusb-suspend applied I don't see them (actually I have to use > > a slightly modified version of the patch which is appended). > > > > Interestingly enough, suspend to RAM works without any visible problems. > > As Oliver said, the USB core should do the right thing when no suspend > and resume callbacks are provided. I looked through the code so many > times now and I am running out of ideas what can happen. > > Lets try it one last time without the suspend patch, but the double free > fix and see if that works. Otherwise I really give up. This time I cannot reproduce the hibernation issue without the suspend patch, so it appears that your double-free fix works for me. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rafael, > > > > >> Rafael, can you pull from my tree and test the changes: > > > > >> > > > > >> git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/ > > > > >> bluetooth-2.6.git > > > > >> > > > > >> It would be interesting if these fixes are enough. > > > > > > > > > > They appear to be enough. I haven't had any suspend/resume failures > > > > > with them > > > > > applied. > > > > > > > > so it works _without_ applying patch-btusb-suspend. > > > > > > Well, unfortunately I spoke too soon. > > > > > > I'm still seeing post-hibernation crashes triggered by the bluetooth user land > > > trying to use the device handled by btusb. They happen every second > > > hibernation, more or less, and apparently they are oopses in various code > > > paths not directly related to bluetooth, like ext3 (memory corruption or > > > what?). > > > > I pushed two extra patches to my bluetooth-2.6 repository: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/holtmann/bluetooth-2.6.git > > > > One is fixing a double-free in the error path. This error path can be > > triggered during suspend/resume if the USB core just disconnects the > > device. Please check if that fixes it for you. > > > > > With patch-btusb-suspend applied I don't see them (actually I have to use > > > a slightly modified version of the patch which is appended). > > > > > > Interestingly enough, suspend to RAM works without any visible problems. > > > > As Oliver said, the USB core should do the right thing when no suspend > > and resume callbacks are provided. I looked through the code so many > > times now and I am running out of ideas what can happen. > > > > Lets try it one last time without the suspend patch, but the double free > > fix and see if that works. Otherwise I really give up. > > This time I cannot reproduce the hibernation issue without the suspend patch, > so it appears that your double-free fix works for me. great. So I push these for 2.6.27 and then for 2.6.28, we can add full suspend and auto-suspend and remote-wakeup support. Thanks for testing. Regards Marcel -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: linux-2.6/drivers/bluetooth/btusb.c =================================================================== --- linux-2.6.orig/drivers/bluetooth/btusb.c +++ linux-2.6/drivers/bluetooth/btusb.c @@ -193,6 +193,7 @@ struct btusb_data { struct usb_endpoint_descriptor *isoc_rx_ep; int isoc_altsetting; + int suspend_count; }; static void btusb_intr_complete(struct urb *urb) @@ -947,10 +948,71 @@ static void btusb_disconnect(struct usb_ hci_free_dev(hdev); } +static int btusb_suspend(struct usb_interface *intf, pm_message_t message) +{ + struct btusb_data *data = usb_get_intfdata(intf); + + BT_DBG("intf %p", intf); + + if (data->suspend_count++) + return 0; + + cancel_work_sync(&data->work); + + usb_kill_anchored_urbs(&data->tx_anchor); + + usb_kill_anchored_urbs(&data->isoc_anchor); + usb_kill_anchored_urbs(&data->bulk_anchor); + usb_kill_anchored_urbs(&data->intr_anchor); + + return 0; +} + +static int btusb_resume(struct usb_interface *intf) +{ + struct btusb_data *data = usb_get_intfdata(intf); + struct hci_dev *hdev = data->hdev; + int err; + + BT_DBG("intf %p", intf); + + if (--data->suspend_count) + return 0; + + if (!test_bit(HCI_RUNNING, &hdev->flags)) + return 0; + + if (test_bit(BTUSB_INTR_RUNNING, &data->flags)) { + err = btusb_submit_intr_urb(hdev); + if (err < 0) { + clear_bit(BTUSB_INTR_RUNNING, &data->flags); + return err; + } + } + + if (test_bit(BTUSB_BULK_RUNNING, &data->flags)) { + if (btusb_submit_bulk_urb(hdev) < 0) + clear_bit(BTUSB_BULK_RUNNING, &data->flags); + else + btusb_submit_bulk_urb(hdev); + } + + if (test_bit(BTUSB_ISOC_RUNNING, &data->flags)) { + if (btusb_submit_isoc_urb(hdev) < 0) + clear_bit(BTUSB_ISOC_RUNNING, &data->flags); + else + btusb_submit_isoc_urb(hdev); + } + + return 0; +} + static struct usb_driver btusb_driver = { .name = "btusb", .probe = btusb_probe, .disconnect = btusb_disconnect, + .suspend = btusb_suspend, + .resume = btusb_resume, .id_table = btusb_table, };