linux-kernel - Re: [PATCH v3 1/5] PM: sleep: Resume children after resuming the parent

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: <CAJZ5v0jVOG_u=F36aOVh=qu4Ef-5QFAmC+5-fmF_mU8NSr_LnA@mail.gmail.com>
Date: Wed, 7 May 2025 16:56:23 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Jon Hunter <jonathanh@...dia.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, "Rafael J. Wysocki" <rjw@...ysocki.net>, 
	Linux PM <linux-pm@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org>, 
	Alan Stern <stern@...land.harvard.edu>, Ulf Hansson <ulf.hansson@...aro.org>, 
	Johan Hovold <johan@...nel.org>, Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>, 
	Saravana Kannan <saravanak@...gle.com>, 
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>
Subject: Re: [PATCH v3 1/5] PM: sleep: Resume children after resuming the parent

On Wed, May 7, 2025 at 4:39 PM Jon Hunter <jonathanh@...dia.com> wrote:
>
>
> On 07/05/2025 15:25, Rafael J. Wysocki wrote:
>
> ...
>
> >>>> So are the devices in question "async"?  To check this, please see the
> >>>> "async" attribute in the "power" subdirectory of the sysfs device
> >>>> directory for each of them.
> >>>
> >>> I checked for both the I2C controller and ina3221 and don't see any
> >>> 'async' files ...
> >>>
> >>> $ ls /sys/class/i2c-dev/i2c-2/device/2-0040/power/
> >>> autosuspend_delay_ms  runtime_active_time  runtime_suspended_time
> >>> control               runtime_status
> >>> $ ls /sys/class/i2c-dev/i2c-2/device/2-0041/power/
> >>> autosuspend_delay_ms  runtime_active_time  runtime_suspended_time
> >>> control               runtime_status
> >>> $ ls /sys/class/i2c-dev/i2c-2/power/
> >>> autosuspend_delay_ms  runtime_active_time  runtime_suspended_time
> >>> control               runtime_status
> >>
> >> You need to set CONFIG_PM_ADVANCED_DEBUG to see those (and other debug
> >> attributes).
> >>
> >>>> If they are "async", you can write "disable" to this attribute to turn
> >>>> them into "sync" devices.  I'd do this and see what happens.
> >>
> >> You may also turn off async suspend altogether:
> >>
> >> # echo 0 > /sys/power/pm_async
> >>
> >> and see if this helps.
>
> This does indeed help!
>
> >>>> Overall, it looks like some dependencies aren't properly represented
> >>>> by device links on this platform.
> >>>
> >>> Yes that would appear to be the case, but at the moment, I don't see
> >>> what it is. The ina3221 devices appear to suspend fine AFAICT, but hangs
> >>> when suspending I2C controller. Exactly where is still a mystery.
> >
> > I checked in the meantime and found that the i2c subsystem enables
> > async suspend/resume for all devices, clients and controllers, so the
> > devices in question are "async" AFAICS.
>
> So that would make sense given that the above works.
>
> When it fails it appears to hang in dpm_wait_for_subordinate() when
> calling dpm_wait_for_children() from what I can tell.

So apparently one of the children has not been suspended yet when this
happens.  That's fine because it should be suspended at one point and
the parent suspend should be unblocked, so it looks like the child
suspend doesn't complete for some reason.

> I will enable the PM_ADVANCED_DEBUG and confirm that making the I2C
> itself non-async works.

What probably happens is that after the "PM: sleep: Suspend async
parents after suspending children" , the i2c clients are suspended
upfront (because they have no children) and when one of them has
suspended, it triggers a parent suspend.  The parent suspend then
waits for the other client to complete suspending, but that cannot
make progress for some reason.

Before that patch, the i2c clients would have suspended only after all
of the "sync" devices following them in dpm_list had been suspended
(the list is processed in the reverse order during suspend), so it
looks like there is a hidden dependency between one of the i2c clients
and a "sync" device.

If the above supposition is right, flagging the i2c client as "sync"
will make the problem go away.