linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
	Qais Yousef <qais.yousef@arm.com>,
	USB list <linux-usb@vger.kernel.org>,
	Linux-pm mailing list <linux-pm@vger.kernel.org>,
	Kernel development list <linux-kernel@vger.kernel.org>
Subject: Re: lockdep warning in urb.c:363 usb_submit_urb
Date: Fri, 03 Apr 2020 18:13:33 +0200	[thread overview]
Message-ID: <2885568.bLUgZ6Vdpo@kreacher> (raw)
In-Reply-To: <2274735.ifPVKiii8o@kreacher>

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 <stern@rowland.harvard.edu> 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!




  reply	other threads:[~2020-04-03 16:13 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-23 14:38 lockdep warning in urb.c:363 usb_submit_urb Qais Yousef
2020-03-23 15:36 ` Oliver Neukum
2020-03-23 15:54   ` Alan Stern
2020-03-23 16:02     ` Qais Yousef
2020-03-23 16:18     ` Oliver Neukum
2020-03-23 15:57   ` Qais Yousef
2020-03-23 16:20     ` Oliver Neukum
2020-03-23 17:29   ` Qais Yousef
2020-03-24  9:08     ` Oliver Neukum
2020-03-24 10:46       ` Qais Yousef
2020-03-24 13:20         ` Oliver Neukum
2020-03-24 13:43           ` Qais Yousef
2020-03-24 13:52             ` Alan Stern
2020-03-24 14:06               ` Qais Yousef
2020-03-24 15:56                 ` Alan Stern
2020-03-24 17:22                   ` Qais Yousef
2020-03-24 19:22                     ` Alan Stern
2020-03-25 15:00                       ` Qais Yousef
2020-03-25 20:49                         ` Alan Stern
2020-03-25 21:28                           ` Rafael J. Wysocki
2020-03-26 12:27                             ` Qais Yousef
2020-03-27 20:45                               ` Alan Stern
2020-03-28 14:15                                 ` Rafael J. Wysocki
2020-03-28 19:58                                   ` Alan Stern
2020-03-29  9:16                                     ` Rafael J. Wysocki
2020-03-29 13:56                                       ` Rafael J. Wysocki
2020-03-29 16:27                                       ` Alan Stern
2020-04-03 15:04                                         ` Rafael J. Wysocki
2020-04-03 16:13                                           ` Rafael J. Wysocki [this message]
2020-04-03 16:41                                           ` Alan Stern
2020-04-03 18:32                                             ` Rafael J. Wysocki
2020-04-03 20:15                                               ` Alan Stern
2020-04-06 16:45                                                 ` Rafael J. Wysocki
2020-04-06 20:25                                                   ` Alan Stern
2020-04-09 18:45                                                     ` Rafael J. Wysocki
2020-04-11  2:41                                                       ` Alan Stern
2020-04-13 21:32                                                         ` Rafael J. Wysocki
2020-04-14 13:43                                                           ` Rafael J. Wysocki
2020-04-14 17:47                                                             ` Alan Stern
2020-04-15 22:20                                                               ` Rafael J. Wysocki
2020-04-16 15:18                                                                 ` Alan Stern
2020-04-17  9:57                                                                   ` Rafael J. Wysocki
2020-04-17 16:10                                                                     ` Alan Stern
2020-04-17 21:54                                                                       ` Rafael J. Wysocki
2020-04-17 23:37                                                                         ` Alan Stern
2020-04-18  9:40                                                                           ` Rafael J. Wysocki
2020-04-03 17:08                                         ` Rafael J. Wysocki
2020-04-20 20:26                           ` Alan Stern
2020-04-21 11:15                             ` Qais Yousef
2020-03-24 13:47         ` Alan Stern

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2885568.bLUgZ6Vdpo@kreacher \
    --to=rjw@rjwysocki.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=qais.yousef@arm.com \
    --cc=rafael@kernel.org \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).