linux-kernel - Re: [PATCH v4 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations

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: <68d6edf1cab3b_10520100a5@dwillia2-mobl4.notmuch>
Date: Fri, 26 Sep 2025 12:48:01 -0700
From: <dan.j.williams@...el.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>, Linux PM
	<linux-pm@...r.kernel.org>, Jonathan Cameron <jonathan.cameron@...wei.com>
CC: Takashi Iwai <tiwai@...e.de>, LKML <linux-kernel@...r.kernel.org>, "Linux
 PCI" <linux-pci@...r.kernel.org>, Alex Williamson
	<alex.williamson@...hat.com>, Bjorn Helgaas <helgaas@...nel.org>, "Zhang
 Qilong" <zhangqilong3@...wei.com>, Ulf Hansson <ulf.hansson@...aro.org>,
	"Frank Li" <Frank.Li@....com>, Dhruva Gole <d-gole@...com>
Subject: Re: [PATCH v4 1/3] PM: runtime: Add auto-cleanup macros for "resume
 and get" operations

Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> 
> It is generally useful to be able to automatically drop a device's
> runtime PM usage counter incremented by runtime PM operations that
> resume a device and bump up its usage counter [1].
> 
> To that end, add guard definition macros allowing pm_runtime_put()
> and pm_runtime_put_autosuspend() to be used for the auto-cleanup in
> those cases.
> 
> Simply put, a piece of code like below:
> 
> 	pm_runtime_get_sync(dev);
> 	.....
> 	pm_runtime_put(dev);
> 	return 0;
> 
> can be transformed with guard() like:
> 
> 	guard(pm_runtime_active)(dev);
> 	.....
> 	return 0;
> 
> (see the pm_runtime_put() call is gone).
> 
> However, it is better to do proper error handling in the majority of
> cases, so doing something like this instead of the above is recommended:
> 
> 	ACQUIRE(pm_runtime_active_try, pm)(dev);
> 	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
> 		return -ENXIO;
> 	.....
> 	return 0;
> 
> In all of the cases in which runtime PM is known to be enabled for the
> given device or the device can be regarded as operational (and so it can
> be accessed) with runtime PM disabled, a piece of code like:
> 
> 	ret = pm_runtime_resume_and_get(dev);
> 	if (ret < 0)
> 		return ret;
> 	.....
> 	pm_runtime_put(dev);
> 	return 0;
> 
> can be changed as follows:
> 
> 	ACQUIRE(pm_runtime_active_try, pm)(dev);
> 	ret = ACQUIRE_ERR(pm_runtime_active_try, &pm);
> 	if (ret < 0)
> 		return ret;
> 	.....
> 	return 0;
> 
> (again, see the pm_runtime_put() call is gone).
> 
> Still, if the device cannot be accessed unless runtime PM has been
> enabled for it, the CLASS(pm_runtime_get_active_enabled) variant

Leftover from CLASS() approach?

s/CLASS(pm_runtime_get_active_enabled)/ACQUIRE(pm_runtime_active_try_enabled)/

> needs to be used, that is (in the context of the example above):
> 
> 	ACQUIRE(pm_runtime_active_try_enabled, pm)(dev);
> 	ret = ACQUIRE_ERR(pm_runtime_active_try_enabled, &pm);
> 	if (ret < 0)
> 		return ret;
> 	.....
> 	return 0;
> 
> When the original code calls pm_runtime_put_autosuspend(), use one
> of the "auto" guard variants, pm_runtime_active_auto/_try/_enabled,
> so for example, a piece of code like:
> 
> 	ret = pm_runtime_resume_and_get(dev);
> 	if (ret < 0)
> 		return ret;
> 	.....
> 	pm_runtime_put_autosuspend(dev);
> 	return 0;
> 
> will become:
> 
> 	ACQUIRE(pm_runtime_active_auto_try_enabled, pm)(dev);
> 	ret = ACQUIRE_ERR(pm_runtime_active_auto_try_enabled, &pm);
> 	if (ret < 0)
> 		return ret;
> 	.....
> 	return 0;
> 
> Note that the cases in which the return value of pm_runtime_get_sync()
> is checked can also be handled with the help of the new class macros.

s/class/guard/

> For example, a piece of code like:
> 
> 	ret = pm_runtime_get_sync(dev);
> 	if (ret < 0) {
> 		pm_runtime_put(dev);
> 		return ret;
> 	}
> 	.....
> 	pm_runtime_put(dev);
> 	return 0;
> 
> can be rewritten as:
> 
> 	ACQUIRE(pm_runtime_active_auto_try_enabled, pm)(dev);
> 	ret = ACQUIRE_ERR(pm_runtime_active_auto_try_enabled, &pm);
> 	if (ret < 0)
> 		return ret;
> 	.....
> 	return 0;

I like that this appears to unify the pm_runtime_resume_and_get() and
pm_runtime_get_sync() usages into common pattern.

> or pm_runtime_get_active_try can be used if transparent handling of
> disabled runtime PM is desirable.

Do you think the above should go in Documentation too?

Either way, for the usage of ACQUIRE(), looks good to me.

Acked-by: Dan Williams <dan.j.williams@...el.com>