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] [day] [month] [year] [list]

Message-ID: <CAJZ5v0gKWSdVeT-eKobSe+BedPKZq25adZKdxOgV0z+iOCAyNw@mail.gmail.com>
Date: Fri, 17 Oct 2025 11:43:15 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: dan.j.williams@...el.com
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Takashi Iwai <tiwai@...e.de>, 
	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>, 
	"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

On Thu, Oct 16, 2025 at 10:59 PM <dan.j.williams@...el.com> wrote:
>
> Rafael J. Wysocki wrote:
> > On Thu, Oct 16, 2025 at 9:45 PM <dan.j.williams@...el.com> wrote:
> > >
> > > 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.
> >
> > No, it doesn't.  PM_RUNTIME_ACQUIRE_ACTIVE_ERR is that error code.  Or
> > did I misunderstand what you said?
>
> Oh, what I am saying is that pm_runtime_get_active() returns a distinct
> error code like -EACCES or -EINPROGRESS etc. The
> PM_RUNTIME_ACQUIRE_ACTIVE_ERR proposal ignores that value and open codes
> returning -ENXIO.

No, it doesn't.

You can still do

ret = PM_RUNTIME_ACQUIRE_ACTIVE_ERR;
if (ret)
        return ret;

if the caller needs to know the original resume error code.

The code being updated in the example patch returns -ENXIO, but it
does so before the change either.