Received-SPF: pass (google.com: domain of linux-kernel+bounces-56390-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <CGME20240207103144eucas1p16b601a73ff347d2542f8380b25921491@eucas1p1.samsung.com>
 <10423008.nUPlyArG6x@kreacher> <708a65cc-79ec-44a6-8454-a93d0f3114c3@samsung.com>
 <CAJZ5v0hn=KgaWn9pwtLsH2a8n61BNxzb1xrNoxUfEi3o9OAZGw@mail.gmail.com> <4a043533-009f-4db9-b107-c8374be28d2b@samsung.com>
In-Reply-To: <4a043533-009f-4db9-b107-c8374be28d2b@samsung.com>
From: "Rafael J. Wysocki" <rafael@kernel.org>
Date: Wed, 7 Feb 2024 12:25:46 +0100
Message-ID: <CAJZ5v0hDmwaFEtLc8yDc4cXn2wODXAqATe0+_Hpm9QPODUPMQw@mail.gmail.com>
Subject: Re: [PATCH v1] PM: sleep: Restore asynchronous device resume optimization
To: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>, "Rafael J. Wysocki" <rjw@rjwysocki.net>, 
	Linux PM <linux-pm@vger.kernel.org>, Ulf Hansson <ulf.hansson@linaro.org>, 
	Tejun Heo <tj@kernel.org>, Nathan Chancellor <nathan@kernel.org>, LKML <linux-kernel@vger.kernel.org>, 
	Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>, Lai Jiangshan <jiangshanlai@gmail.com>, 
	Naohiro.Aota@wdc.com, kernel-team@meta.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Feb 7, 2024 at 12:16=E2=80=AFPM Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
>
> On 07.02.2024 11:38, Rafael J. Wysocki wrote:
> > On Wed, Feb 7, 2024 at 11:31=E2=80=AFAM Marek Szyprowski
> > <m.szyprowski@samsung.com>  wrote:
> >> On 09.01.2024 17:59, Rafael J. Wysocki wrote:
> >>>    From: Rafael J. Wysocki<rafael.j.wysocki@intel.com>
> >>>
> >>> Before commit 7839d0078e0d ("PM: sleep: Fix possible deadlocks in cor=
e
> >>> system-wide PM code"), the resume of devices that were allowed to res=
ume
> >>> asynchronously was scheduled before starting the resume of the other
> >>> devices, so the former did not have to wait for the latter unless
> >>> functional dependencies were present.
> >>>
> >>> Commit 7839d0078e0d removed that optimization in order to address a
> >>> correctness issue, but it can be restored with the help of a new devi=
ce
> >>> power management flag, so do that now.
> >>>
> >>> Signed-off-by: Rafael J. Wysocki<rafael.j.wysocki@intel.com>
> >>> ---
> >> This patch finally landed in linux-next some time ago as 3e999770ac1c
> >> ("PM: sleep: Restore asynchronous device resume optimization"). Recent=
ly
> >> I found that it causes a non-trivial interaction with commit
> >> 5797b1c18919 ("workqueue: Implement system-wide nr_active enforcement
> >> for unbound workqueues"). Since merge commit 954350a5f8db in linux-nex=
t
> >> system suspend/resume fails (board doesn't wake up) on my old Samsung
> >> Exynos4412-based Odroid-U3 board (ARM 32bit based), which was rock
> >> stable for last years.
> >>
> >> My further investigations confirmed that the mentioned commits are
> >> responsible for this issue. Each of them separately (3e999770ac1c and
> >> 5797b1c18919) doesn't trigger any problems. Reverting any of them on t=
op
> >> of linux-next (with some additional commit due to code dependencies)
> >> also fixes/hides the problem.
> >>
> >> Let me know if You need more information or tests on the hardware. I'm
> >> open to help debugging this issue.
> > If you echo 0 to /sys/power/pm_async before suspending the system,
> > does it still fail?
>
> In such case it works fine.

Thanks for the confirmation.

 It doesn't rely on unbound workqueues then, so that's expected.

Now, I think that there are two possibilities.

One is that commit 3e999770ac1c is generally overoptimistic for your
board and there is a dependency between devices which is not
represented by a device link, and it causes things to go south when
they are not done in a specific order.  If that is the case and commit
5797b1c18919 changes that order, breakage ensues.

The other one is that what happens during async resume does not meet
the assumptions of commit 5797b1c18919 (for example, it can easily
produce a chain of interdependent work items longer than 8) and so it
breaks things.

I would still try to use a non-unbound workqueue for the async thing,
because if it works reliably then, the second possibility will be more
likely.