linux-kernel - Re: [PATCH v5 1/2] pm: runtime: Add new devm functions

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: <bcf363db-8fff-4fb1-b29e-300f7b8bc090@prolan.hu>
Date: Thu, 27 Mar 2025 17:11:29 +0100
From: Csókás Bence <csokas.bence@...lan.hu>
To: "Rafael J. Wysocki" <rafael@...nel.org>
CC: <linux-pm@...r.kernel.org>, <linux-kernel@...r.kernel.org>, "Varshini
 Rajendran" <varshini.rajendran@...rochip.com>, Tudor Ambarus
	<tudor.ambarus@...aro.org>, Mark Brown <broonie@...nel.org>,
	<linux-spi@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>, "Len
 Brown" <len.brown@...el.com>, Pavel Machek <pavel@....cz>, Greg Kroah-Hartman
	<gregkh@...uxfoundation.org>, Danilo Krummrich <dakr@...nel.org>, "Alexander
 Dahl" <ada@...rsis.com>, Nicolas Ferre <nicolas.ferre@...rochip.com>,
	Alexandre Belloni <alexandre.belloni@...tlin.com>, Claudiu Beznea
	<claudiu.beznea@...on.dev>, Pavel Machek <pavel@...nel.org>
Subject: Re: [PATCH v5 1/2] pm: runtime: Add new devm functions

Hi,

On 2025. 03. 27. 15:14, Rafael J. Wysocki wrote:
> /-- devm_pm_runtime_get_noresume()
> |   /-- devm_{pm_runtime_set_active() + pm_runtime_enable() (in this order)}
> |   |   pm_runtime_use_autosuspend()
> |   |
> |   |   Note that the device cannot be suspended here unless its
> runtime PM usage
> |   |   counter is dropped, in which it would need to be bumped up
> again later to
> |   |   retain the balance.
> |   |
> |   \-> pm_runtime_disable() + pm_runtime_set_suspended() (in this order)
> \-> pm_runtime_put_noidle()

Ah, so basically what I've done originally, just calling 
`devm_pm_runtime_get_noresume()` _first_ instead of _last_, right?

> And pm_runtime_dont_use_autosuspend() is not really necessary after
> disabling runtime PM.

It was done this way in devm_pm_runtime_enable() already, see commit 
b4060db9251f ("PM: runtime: Have devm_pm_runtime_enable() handle 
pm_runtime_dont_use_autosuspend()"). I didn't change anything 
behaviourally there.

> Also, I think that the driver could be fixed without introducing the
> new devm_ stuff which would be way simpler, so why don't you do that
> and then think about devm_?

Sure, I could quick-fix this, go through all the possible error paths 
and whatnot and ref-count in my head, but it doesn't fix the underlying 
problem: in order to properly use PM, you have to do a bunch of calls in 
some set order, then undo them in reverse order on error and remove -- 
exactly the thing devm was designed for, and exactly the thing where 
it's easy for a human to forget one case by accident. Thus I prefer to 
use the *real* solution, devm.

Bence