LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Alan Stern <stern@rowland.harvard.edu>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>
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, 10 Apr 2020 22:41:14 -0400 (EDT)
Message-ID: <Pine.LNX.4.44L0.2004102231270.30859-100000@netrider.rowland.org> (raw)
In-Reply-To: <3100919.FSIbSBgRSq@kreacher>

Okay, this is my attempt to summarize what we have been discussing.  
But first: There is a dev_pm_skip_resume() helper routine which
subsystems can call to see whether resume-side _early and _noirq driver
callbacks should be skipped.  But there is no corresponding
dev_pm_skip_suspend() helper routine.  Let's add one, or rename
dev_pm_smart_suspend_and_suspended() to dev_pm_skip_suspend().

Given that, here's my understanding of what should happen.  (I'm
assuming the direct_complete mechanism is not being used.)  This tries
to describe what we _want_ to happen, which is not always the same as
what the current code actually _does_.

	During the suspend side, for each of the
	{suspend,freeze,poweroff}_{late,noirq} phases: If
	dev_pm_skip_suspend() returns true then the subsystem should
	not invoke the driver's callback, and if there is no subsystem
	callback then the core will not invoke the driver's callback.

	During the resume side, for each of the
	{resume,thaw,restore}_{early,noirq} phases: If
	dev_pm_skip_resume() returns true then the subsystem should
	not invoke the driver's callback, and if there is no subsystem
	callback then the core will not invoke the driver's callback.

	dev_pm_skip_suspend() will return "true" if SMART_SUSPEND is
	set and the device's runtime status is "suspended".

	power.must_resume gets set following the suspend-side _noirq
	phase if power.usage_count > 1 (indicating the device was
	in active use before the start of the sleep transition) or
	power.must_resume is set for any of the device's dependents.

	dev_pm_skip_resume() will return "false" if the current
	transition is RESTORE or power.must_resume is set.  Otherwise:
	It will return true if the current transition is THAW,
	SMART_SUSPEND is set, and the device's runtime status is
	"suspended".  It will return "true" if the current transition is
	RESUME, SMART_SUSPEND and MAY_SKIP_RESUME are both set, and
	the device's runtime status is "suspended".  For a RESUME
	transition, it will also return "true" if MAY_SKIP_RESUME and
	power.may_skip_resume are both set, regardless of
	SMART_SUSPEND or the current runtime status.

	At the start of the {resume,thaw,restore}_noirq phase, if
	dev_pm_skip_resume() returns true then the core will set the
	runtime status to "suspended".  Otherwise it will set the
	runtime status to "active".  If this is not what the subsystem
	or driver wants, it must update the runtime status itself.

Comments and differences with respect to the code in your pm-sleep-core
branch:

	I'm not sure whether we should specify other conditions for
	setting power.must_resume.

	dev_pm_skip_resume() doesn't compute the value described
	above.  I'm pretty sure the existing code is wrong.

	device_resume_noirq() checks
	dev_pm_smart_suspend_and_suspended() before setting the
	runtime PM status to "active", contrary to the text above.
	The difference shows up in the case where SMART_SUSPEND is
	clear but the runtime PM status is "suspended".  Don't we want
	to set the status to "active" in this case?  Or is there some 
	need to accomodate legacy drivers here?  In any case, wouldn't
	it be good enough to check just the SMART_SUSPEND flag?

	__device_suspend_late() sets power.may_skip_resume, contrary
	to the comment in include/linux/pm.h: That flag is supposed to
	be set by subsystems, not the core.

	I'm not at all sure that this algorithm won't run into trouble
	at some point when it tries to set a device's runtime status
	to "active" but the parent's status is set to "suspended".
	And I'm not sure this problem can be fixed by adjusting
	power.must_resume.

Alan Stern


  reply index

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-23 14:38 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
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 [this message]
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=Pine.LNX.4.44L0.2004102231270.30859-100000@netrider.rowland.org \
    --to=stern@rowland.harvard.edu \
    --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=rjw@rjwysocki.net \
    /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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git