From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 89C80C43331 for ; Fri, 3 Apr 2020 16:13:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 664DF206E9 for ; Fri, 3 Apr 2020 16:13:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2391070AbgDCQNg (ORCPT ); Fri, 3 Apr 2020 12:13:36 -0400 Received: from cloudserver094114.home.pl ([79.96.170.134]:59716 "EHLO cloudserver094114.home.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2390975AbgDCQNg (ORCPT ); Fri, 3 Apr 2020 12:13:36 -0400 Received: from 185.80.35.16 (185.80.35.16) (HELO kreacher.localnet) by serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer 0.83.341) id ce824f3db010ba3f; Fri, 3 Apr 2020 18:13:33 +0200 From: "Rafael J. Wysocki" To: Alan Stern Cc: "Rafael J. Wysocki" , Qais Yousef , USB list , Linux-pm mailing list , Kernel development list Subject: Re: lockdep warning in urb.c:363 usb_submit_urb Date: Fri, 03 Apr 2020 18:13:33 +0200 Message-ID: <2885568.bLUgZ6Vdpo@kreacher> In-Reply-To: <2274735.ifPVKiii8o@kreacher> References: <2274735.ifPVKiii8o@kreacher> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Friday, April 3, 2020 5:04:16 PM CEST Rafael J. Wysocki wrote: > On Sunday, March 29, 2020 6:27:38 PM CEST Alan Stern wrote: > > On Sun, 29 Mar 2020, Rafael J. Wysocki wrote: > > > > > On Sat, Mar 28, 2020 at 8:58 PM Alan Stern wrote: > > [cut] > > > > > Can you give a similarly > > > > succinct outline for how SMART_SUSPEND and LEAVE_SUSPENDED should work? > > > > And also describe how they differ from direct_complete and how they > > > > interact with it? (For example, how does setting both flags differ > > > > from returning a positive value from ->prepare?) > > > > > > I will, but I need some time to do that. Stay tuned. > > > > You bet! > > Sorry for the delay, too much distraction nowadays. > > I'll address the other points in your message separately. > > The rules for SMART_SUSPEND are as follows: > > (a) If SMART_SUSPEND is set and the device is runtime-suspended during system > suspend, it is not expected to be resumed by the core or the middle layer > (subsystem) code unless the latter has a specific reason to do that (e.g. > it knows that the device needs to be reconfigured which cannot be done > without resuming it). > > The device can still be resumed when it is needed to suspend a dependent > device, but that cannot happen before the "late suspend" phase. s/cannot/must/ > (b) Drivers that set SMART_SUSPEND are allowed to reuse their PM-runtime > callbacks for system-wide suspend and resume. > > That is, they can point either the ->suspend_late or the ->suspend_noirq > callback pointer to the same function as ->runtime_suspend and they can > point either the ->resume_noirq or ->the resume_early callback to the' > same function as ->runtime_resume. > > (c) Drivers that set SMART_SUSPEND are alwo allowed to provide special s/alwo/also/ > simplified callbacks for the "freeze" and "thaw" transitions during > hibernation (and restore) and (if they do so) special callbacks for the > "restore" phase. > > [OK, I realize that (b) and (c) are not documented, see the notes below.] > > Because of (a), if the device with SMART_SUSPEND set is still runtime-suspended > during the "late" phase of suspend, the core will not invoke the driver's > "late" and "noirq" suspend callbacks directly (*). Middle layer (subsystem) > code is expected to behave accordingly. > > Because of (b), if the "late" and "noirq" driver callbacks were skipped during > the "freeze" transition, the core will also avoid invoking the "noirq" and > "early" callbacks provided by the driver during the "thaw" transition and > the callbacks during the "restore" transition will be executed unconditionally > (**). Middle layer code is expected to behave accordingly. > > Notes: > > 1. I have considered splitting SMART_SUSPEND into two or even three flags > so that (a), (b) and (c) are each associated with a separate flag, but > then I would expect the majority of users to use all of them anyway. > > 2. LEAVE_SUSPENDED (which may be better renamed to SKIP_RESUME) is kind of > expected to be used along with SMART_SUSPEND unless there is a good enough > reason to avoid using it. I admit that this isn't really straightforward, > maybe the default behavior should be to skip the resume and there should be > FORCE_RESUME instead of LEAVE_SUSPENDED. > > 3. (*) Under the assumption that either ->suspend_late or ->suspend_noirq > points to the same routine as ->runtime_suspend (and the other is NULL), > invokig that callback for a runtime-suspended device is technically invalid. > In turn, under the assumption that either ->resume_early or ->resume_noirq > points to the same routine as ->runtime_resume (and the other is NULL), it is > valid to invoke that callback if the late/noirq suspend was skipped. > > 4. (**) If the "freeze" and "thaw" callbacks are simplified, they cannot be > run back-to-back with ->runtime_resume and ->runtime_suspend, respectively. That is, ->freeze -> ->runtime_resume would be invalid and ->runtime_suspend -> ->thaw would be invalid. > Thus if "freeze" is skippend, "thaw" must be skipped too. However, > "restore" needs to be prepared to be invoked after "freeze" or > ->runtime_suspend (and the state of the device may not match the > callback that ran previously), so it must be special. > > 5. I agree that skipping the driver level of callbacks depending on what is > provided by the middle layer is inconsistent, but I wanted to take the > users of pm_runtime_force_suspend/resume() into account by letting those > things run. > > It would be more consistent to expect middle layer code (bus types, PM > domains) to provide either all of the noirq/early/late callbacks, or none > of them and make SMART_SUSPEND and pm_runtime_force_suspend/resume() > mutually exclusive. > > Cheers!