linux-kernel - Re: [PATCH] drvier: usb: dwc3: Fix runtime PM trying to activate child device xxx.dwc3 but parent is not active

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZ5v0i6aFarDU8OTZ_3VS9dp4SaqKJ0SuiN4gFSxrRoAAV5CQ@mail.gmail.com>
Date: Thu, 4 Sep 2025 16:08:47 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Alan Stern <stern@...land.harvard.edu>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Thinh Nguyen <Thinh.Nguyen@...opsys.com>, 
	ryan zhou <ryanzhou54@...il.com>, Roy Luo <royluo@...gle.com>, 
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>, 
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>
Subject: Re: [PATCH] drvier: usb: dwc3: Fix runtime PM trying to activate
 child device xxx.dwc3 but parent is not active

On Wed, Sep 3, 2025 at 11:54 PM Alan Stern <stern@...land.harvard.edu> wrote:
>
> On Wed, Sep 03, 2025 at 09:30:47PM +0200, Rafael J. Wysocki wrote:
> > I personally think that it would be reasonable to simply preserve
> > device states in error paths unless they have been changed already
> > before the error (or suspend abort due to a wakeup signal).
>
> The problem is complicated by the interaction between runtime-PM states
> and system-sleep states.  In the case, we've been considering, B changes
> from runtime-suspended to runtime-suspended + system-suspended.
> Therefore the error path is allowed to modify B's state.

Yes, it is, but retaining the B's state in an error path is also fine
so long as no changes have been made to it so far.

If B was runtime-suspended to start with and none of the suspend
callbacks invoked for it so far has done anything to it, then it is de
facto still runtime-suspended and its state need not be changed in an
error path.

> > By this rule, B would be left in runtime suspend if it were still in
> > runtime suspend when the error (or suspend abort in general) occurred
> > and then it doesn't matter what happens to A.
>
> More fully, B would be changed from runtime-suspended + system-suspended
> back to simply runtime-suspended.  Unfortunately, none of the PM
> callbacks in the kernel are defined to make this change -- at least, not
> without some cooperation from the driver.

Except when runtime-suspended + system-suspended is effectively the
same as runtime-suspended.

Say this is not the case and say that the device is runtime-suspended
to start with.  Then the "suspend" callback has two choices: either
(1) it can runtime-resume the device before doing anything to it,
which will also cause the device's parent and suppliers to
runtime-resume, or (2) it can update the device's state without
resuming it.

If it chooses (1), then "resume" is straightforward.  If it chooses
(2), "resume" may just reverse the changes made by "suspend" and
declare that the device is runtime-suspended.  And if it really really
wants to resume the device then, why not call runtime_resume() on it?

> > The PM core can do something like that for the drivers opting in for
> > runtime PM integration assistance, so to speak.  That is, drivers that
> > point their ->suspend() and ->resume() callbacks to
> > pm_runtime_force_suspend() and pm_runtime_force_resume(),
> > respectively, or set DPM_FLAG_SMART_SUSPEND (or both at the same time
> > which is now feasible).  Otherwise, it is hard to say what the
> > expectations of the driver are and some code between the driver and
> > the PM core may be involved (say, the PCI bus type).
>
> Setting DPM_FLAG_SMART_SUSPEND really does sound like the best answer.
>
> But there still should be some way the PM core can make resumes easier
> for drivers that don't set the flag.  Something like: If the device is
> in runtime suspend with SMART_SUSPEND clear, perform a runtime resume on
> the device's parent (and anything else the device depends on) before
> invoking ->resume.

Say that ->resume() does nothing to the device (because it is
runtime-suspended and there's no need to resume it).  Why would the
core resume the parent etc then?