linux-kernel - Re: [PATCH v1 1/3] PM: runtime: Introduce PM_RUNTIME_ACQUIRE_OR

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: <68f14b5b6a92_2a2b10018@dwillia2-mobl4.notmuch>
Date: Thu, 16 Oct 2025 12:45:31 -0700
From: <dan.j.williams@...el.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>, Takashi Iwai <tiwai@...e.de>
CC: David Lechner <dlechner@...libre.com>, Jonathan Cameron
	<jonathan.cameron@...wei.com>, Linux PM <linux-pm@...r.kernel.org>, 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>, Mika Westerberg
	<mika.westerberg@...ux.intel.com>, Linux ACPI <linux-acpi@...r.kernel.org>,
	Dan Williams <dan.j.williams@...el.com>, "Fabio M. De Francesco"
	<fabio.maria.de.francesco@...ux.intel.com>
Subject: Re: [PATCH v1 1/3] PM: runtime: Introduce
 PM_RUNTIME_ACQUIRE_OR_FAIL() macro

Rafael J. Wysocki wrote:
[..]
> > > [1]: https://lore.kernel.org/all/CAHk-=whn07tnDosPfn+UcAtWHBcLg=KqA16SHVv0GV4t8P1fHw@mail.gmail.com/
> > 
> > Yeah, I myself also find it suboptimal, hence it wasn't really
> > proposed...  It's a limit of macro, unfortunately.
> 
> The macro from the $subject patch can be split along the lines of the appended
> patch to avoid the "disgusting syntax" issue, although it then becomes less
> attractive as far as I'm concerned.  It still allows the details unrelated to
> the rest of the code to be hidden though.
> 
> ---
>  drivers/acpi/acpi_tad.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> --- a/drivers/acpi/acpi_tad.c
> +++ b/drivers/acpi/acpi_tad.c
> @@ -31,6 +31,12 @@ MODULE_DESCRIPTION("ACPI Time and Alarm
>  MODULE_LICENSE("GPL v2");
>  MODULE_AUTHOR("Rafael J. Wysocki");
>  
> +#define PM_RUNTIME_ACQUIRE_ACTIVE(dev)	\
> +	ACQUIRE(pm_runtime_active_try, pm_runtime_active_guard_var)(dev)
> +
> +#define PM_RUNTIME_ACQUIRE_ACTIVE_ERR	\
> +	ACQUIRE_ERR(pm_runtime_active_try, &pm_runtime_active_guard_var)
> +
>  /* ACPI TAD capability flags (ACPI 6.2, Section 9.18.2) */
>  #define ACPI_TAD_AC_WAKE	BIT(0)
>  #define ACPI_TAD_DC_WAKE	BIT(1)
> @@ -264,8 +270,8 @@ static int acpi_tad_wake_set(struct devi
>  	args[0].integer.value = timer_id;
>  	args[1].integer.value = value;
>  
> -	ACQUIRE(pm_runtime_active_try, pm)(dev);
> -	if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
> +	PM_RUNTIME_ACQUIRE_ACTIVE(dev);
> +	if (PM_RUNTIME_ACQUIRE_ACTIVE_ERR)
>  		return -ENXIO;

This defeats one of the other motivations for ACQUIRE() vs
scoped_cond_guard() in that it drops the error code from
pm_runtime_active_try. Maybe it is the case that failure is always
-ENXIO, but from a future code evolution standpoint do you want to
commit to always translating _try errors to a local error code?

Btw, was acpi_tad_wake_set() buggy previously for ignoring
pm_runtime_get_sync() errors, or is it a regression risk now for
honoring errors?