From patchwork Fri Dec 5 20:03:57 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 5445751 Return-Path: X-Original-To: patchwork-linux-media@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id A28ADBEEA8 for ; Fri, 5 Dec 2014 20:04:14 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 9183B2012E for ; Fri, 5 Dec 2014 20:04:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 59C6620115 for ; Fri, 5 Dec 2014 20:04:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751855AbaLEUEI (ORCPT ); Fri, 5 Dec 2014 15:04:08 -0500 Received: from www.linutronix.de ([62.245.132.108]:37215 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751172AbaLEUEG convert rfc822-to-8bit (ORCPT ); Fri, 5 Dec 2014 15:04:06 -0500 Received: from bigeasy by Galois.linutronix.de with local (Exim 4.80) (envelope-from ) id 1Xwz6b-0000TT-OW; Fri, 05 Dec 2014 21:03:57 +0100 Date: Fri, 5 Dec 2014 21:03:57 +0100 From: Sebastian Andrzej Siewior To: linux-usb@vger.kernel.org, linux-media@vger.kernel.org Cc: Greg Kroah-Hartman , Alan Stern , Felipe Balbi , Sarah Sharp , Laurent Pinchart , Mauro Carvalho Chehab Subject: [PATCH] usb: hcd: get/put device and hcd for hcd_buffers() Message-ID: <20141205200357.GA1586@linutronix.de> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Consider the following scenario: - plugin a webcam - play the stream via gst-launch-0.10 v4l2src device=/dev/video0… - remove the USB-HCD during playback via "rmmod $HCD" and now wait for the crash |musb-hdrc musb-hdrc.2.auto: remove, state 1 |usb usb2: USB disconnect, device number 1 |usb 2-1: USB disconnect, device number 3 |uvcvideo: Failed to resubmit video URB (-19). |musb-hdrc musb-hdrc.2.auto: USB bus 2 deregistered |musb-hdrc musb-hdrc.1.auto: remove, state 4 |usb usb1: USB disconnect, device number 1 |musb-hdrc musb-hdrc.1.auto: USB bus 1 deregistered |Unable to handle kernel paging request at virtual address 6b6b6b6f |pgd = c0004000 |[6b6b6b6f] *pgd=00000000 |Internal error: Oops: 5 [#1] ARM |Modules linked in: uvcvideo] |CPU: 0 PID: 2613 Comm: gst-launch-0.10 Tainted: G W 3.14.25+ #40 |task: ec2b8100 ti: ec38e000 task.ti: ec38e000 |PC is at hcd_buffer_free+0x64/0xc0 |LR is at uvc_free_urb_buffers+0x34/0x50 [uvcvideo] |Process gst-launch-0.10 (pid: 2613, stack limit = 0xec38e240) |[] (hcd_buffer_free) |[] (uvc_free_urb_buffers [uvcvideo]) |[] (uvc_video_enable [uvcvideo]) |[] (uvc_v4l2_release [uvcvideo]) |[] (v4l2_release [videodev]) |[] (__fput) |[] (task_work_run) |[] (do_exit) |[] (do_group_exit) as part of the device-removal the HCD removes its dma-buffers, the HCD structure itself and even the struct device is gone. That means if UVC removes its URBs after its last user (/dev/videoX) is gone and not from the ->disconnect() callback then it is too late because the HCD might gone. First, I fixed by in the UVC driver by ignoring the UVC_SC_VIDEOSTREAMING in its ->disconnect callback and calling uvc_video_enable(, 0) in uvc_unregister_video(). But then I though that it might not be clever to release that memory if there is userspace using it. So instead, I hold the device struct in the HCD and the HCD struct on every USB-buf-alloc. That means after a disconnect we still have a refcount on usb_hcd and device and it will be cleaned "later" once the last USB-buffer is released. Signed-off-by: Sebastian Andrzej Siewior --- With this applied, I only see this three times (which is not new) | ------------[ cut here ]------------ | WARNING: CPU: 0 PID: 1755 at fs/sysfs/group.c:219 sysfs_remove_group+0x88/0x98() | sysfs group c08a70d4 not found for kobject 'event4' | Modules linked in: uvcvideo videobuf2_vmalloc videobuf2_memops videobuf2_core v4l2_common videodev ipv6 musb_hdrc udc_core us] | CPU: 0 PID: 1755 Comm: gst-launch-0.10 Not tainted 3.18.0-rc7-00065-gabcefb342fbf-dirty #1813 | [] (unwind_backtrace) from [] (show_stack+0x10/0x14) | [] (show_stack) from [] (dump_stack+0x80/0x9c) | [] (dump_stack) from [] (warn_slowpath_common+0x68/0x8c) | [] (warn_slowpath_common) from [] (warn_slowpath_fmt+0x30/0x40) | [] (warn_slowpath_fmt) from [] (sysfs_remove_group+0x88/0x98) | [] (sysfs_remove_group) from [] (device_del+0x34/0x198) | [] (device_del) from [] (evdev_disconnect+0x18/0x44) | [] (evdev_disconnect) from [] (__input_unregister_device+0xa4/0x148) | [] (__input_unregister_device) from [] (input_unregister_device+0x40/0x74) | [] (input_unregister_device) from [] (uvc_delete+0x20/0x10c [uvcvideo]) | [] (uvc_delete [uvcvideo]) from [] (v4l2_device_release+0x9c/0xc4 [videodev]) | [] (v4l2_device_release [videodev]) from [] (device_release+0x2c/0x90) | [] (device_release) from [] (kobject_release+0x48/0x7c) | [] (kobject_release) from [] (v4l2_release+0x50/0x78 [videodev]) | [] (v4l2_release [videodev]) from [] (__fput+0x80/0x1c4) | [] (__fput) from [] (task_work_run+0xb4/0xe4) | [] (task_work_run) from [] (do_exit+0x2dc/0x92c) | [] (do_exit) from [] (do_group_exit+0x3c/0xb0) | [] (do_group_exit) from [] (__wake_up_parent+0x0/0x18) | ---[ end trace b54a8f3c8129180e ]--- anyone an idea? drivers/usb/core/buffer.c | 30 +++++++++++++++++++++++++----- drivers/usb/core/hcd.c | 2 ++ 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c index 506b969ea7fd..01e080a61519 100644 --- a/drivers/usb/core/buffer.c +++ b/drivers/usb/core/buffer.c @@ -107,7 +107,7 @@ void hcd_buffer_destroy(struct usb_hcd *hcd) * better sharing and to leverage mm/slab.c intelligence. */ -void *hcd_buffer_alloc( +static void *_hcd_buffer_alloc( struct usb_bus *bus, size_t size, gfp_t mem_flags, @@ -131,7 +131,19 @@ void *hcd_buffer_alloc( return dma_alloc_coherent(hcd->self.controller, size, dma, mem_flags); } -void hcd_buffer_free( +void *hcd_buffer_alloc(struct usb_bus *bus, size_t size, gfp_t mem_flags, + dma_addr_t *dma) +{ + struct usb_hcd *hcd = bus_to_hcd(bus); + void *ret; + + ret = _hcd_buffer_alloc(bus, size, mem_flags, dma); + if (ret) + usb_get_hcd(hcd); + return ret; +} + +static void _hcd_buffer_free( struct usb_bus *bus, size_t size, void *addr, @@ -141,9 +153,6 @@ void hcd_buffer_free( struct usb_hcd *hcd = bus_to_hcd(bus); int i; - if (!addr) - return; - if (!bus->controller->dma_mask && !(hcd->driver->flags & HCD_LOCAL_MEM)) { kfree(addr); @@ -158,3 +167,14 @@ void hcd_buffer_free( } dma_free_coherent(hcd->self.controller, size, addr, dma); } + +void hcd_buffer_free(struct usb_bus *bus, size_t size, void *addr, + dma_addr_t dma) +{ + struct usb_hcd *hcd = bus_to_hcd(bus); + + if (!addr) + return; + _hcd_buffer_free(bus, size, addr, dma); + usb_put_hcd(hcd); +} diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index a6efb4184f2b..536da1639ac5 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -2469,6 +2469,7 @@ struct usb_hcd *usb_create_shared_hcd(const struct hc_driver *driver, kref_init(&hcd->kref); usb_bus_init(&hcd->self); + get_device(dev); hcd->self.controller = dev; hcd->self.bus_name = bus_name; hcd->self.uses_dma = (dev->dma_mask != NULL); @@ -2534,6 +2535,7 @@ static void hcd_release(struct kref *kref) peer->primary_hcd = NULL; } mutex_unlock(&usb_port_peer_mutex); + put_device(hcd->self.controller); kfree(hcd); }