From patchwork Tue Jun 18 15:03:49 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ming Lei X-Patchwork-Id: 2742851 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 5C2669F8E1 for ; Tue, 18 Jun 2013 15:07:59 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 92EB22049C for ; Tue, 18 Jun 2013 15:07:54 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 206B12049B for ; Tue, 18 Jun 2013 15:07:53 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1UoxTk-0005SA-2l; Tue, 18 Jun 2013 15:05:54 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1UoxT6-0002gl-Ob; Tue, 18 Jun 2013 15:05:12 +0000 Received: from mail-pd0-f176.google.com ([209.85.192.176]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1UoxSt-0002dl-6h for linux-arm-kernel@lists.infradead.org; Tue, 18 Jun 2013 15:05:01 +0000 Received: by mail-pd0-f176.google.com with SMTP id t12so3980259pdi.21 for ; Tue, 18 Jun 2013 08:04:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references; bh=PMQ2g+wZaVwkvasTThHjzKbc8Je961jtDExaeY/rT+o=; b=iNEMpxPZRyiiKgPj5XkOVrkj60i+WeML+wjSd14aJyiq59C2hTpMOx+nWSEArt2d4P /apEoub83bVHXd8cLrigtmPVzjgwWgWcO1IQe1rC85V0oGsDRPCzb+c0yqYKyB1vuNKb HUALGdO4pBmCMP4X6LWqNpwT3LZkzgmdYPQBMyk9v3SKBGTfsgOlplouv5sxQhRbGgsD nSQb4EsbgrPv8hNUEw6GX6eheT+qoII9xS+JcM3uFy35CUP94fmqiI0T94iq7P0sk269 1H3Jdj2+EJgU9zPbp/ICL48718FnthvQm3lXboGSWe1piXI7CINz6Xn+cYu5Yhq/7PHf TDjA== X-Received: by 10.68.20.33 with SMTP id k1mr351452pbe.168.1371567877578; Tue, 18 Jun 2013 08:04:37 -0700 (PDT) Received: from localhost ([183.37.201.115]) by mx.google.com with ESMTPSA id pe9sm18623190pbc.35.2013.06.18.08.04.32 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Tue, 18 Jun 2013 08:04:36 -0700 (PDT) From: Ming Lei To: Greg Kroah-Hartman Subject: [RFC PATCH v1 3/6] USB: URB documentation: claim complete() may be run with IRQs enabled Date: Tue, 18 Jun 2013 23:03:49 +0800 Message-Id: <1371567833-9077-4-git-send-email-ming.lei@canonical.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1371567833-9077-1-git-send-email-ming.lei@canonical.com> References: <1371567833-9077-1-git-send-email-ming.lei@canonical.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130618_110459_402839_624D4821 X-CRM114-Status: GOOD ( 14.86 ) X-Spam-Score: -1.9 (-) Cc: Oliver Neukum , Ming Lei , linux-usb@vger.kernel.org, Steven Rostedt , Alan Stern , Thomas Gleixner , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-5.4 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, 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 There is no good reason to run complete() in hard interrupt disabled context. After running complete() in tasklet, we will enable local IRQs when calling complete() since we can do it now. Even though we still disable IRQs now when calling complete() in tasklet, the URB documentation is updated to claim complete() may be run in tasklet context and local IRQs may be enabled, so that USB drivers can know the change and avoid one deadlock caused by: assume IRQs disabled in complete() and call spin_lock() to hold lock which might be acquired in interrupt context. Current spin_lock() usages in drivers' complete() will be cleaned up at the same time, and when the cleanup is finished, local IRQs will be enabled when calling complete() in tasklet. Also fix description about type of usb_complete_t. Cc: Oliver Neukum Cc: Alan Stern Signed-off-by: Ming Lei --- Documentation/usb/URB.txt | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/Documentation/usb/URB.txt b/Documentation/usb/URB.txt index 00d2c64..454db87 100644 --- a/Documentation/usb/URB.txt +++ b/Documentation/usb/URB.txt @@ -195,13 +195,12 @@ by the completion handler. The handler is of the following type: - typedef void (*usb_complete_t)(struct urb *, struct pt_regs *) + typedef void (*usb_complete_t)(struct urb *) -I.e., it gets the URB that caused the completion call, plus the -register values at the time of the corresponding interrupt (if any). -In the completion handler, you should have a look at urb->status to -detect any USB errors. Since the context parameter is included in the URB, -you can pass information to the completion handler. +I.e., it gets the URB that caused the completion call. In the completion +handler, you should have a look at urb->status to detect any USB errors. +Since the context parameter is included in the URB, you can pass +information to the completion handler. Note that even when an error (or unlink) is reported, data may have been transferred. That's because USB transfers are packetized; it might take @@ -210,12 +209,19 @@ have transferred successfully before the completion was called. NOTE: ***** WARNING ***** -NEVER SLEEP IN A COMPLETION HANDLER. These are normally called -during hardware interrupt processing. If you can, defer substantial -work to a tasklet (bottom half) to keep system latencies low. You'll -probably need to use spinlocks to protect data structures you manipulate -in completion handlers. - +NEVER SLEEP IN A COMPLETION HANDLER. These are called during hardware +interrupt processing if HCD_BH isn't set in hcd->driver->flags, otherwise +called in tasklet context. If you can, defer substantial work to a tasklet +(bottom half) to keep system latencies low. You'll probably need to use +spinlocks to protect data structures you manipulate in completion handlers. + +Driver can't assume that local IRQs are disabled any more when running +complete(), for example, if drivers want to hold a lock which might be +acquired in hard interrupt handler, they have to use spin_lock_irqsave() +instead of spin_lock() to hold the lock. + +In the future, all HCD might set HCD_BH to run complete() in tasklet so +that system latencies can be kept low. 1.8. How to do isochronous (ISO) transfers?