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: <68f15c85b1781_2a2010086@dwillia2-mobl4.notmuch>
Date: Thu, 16 Oct 2025 13:58:45 -0700
From: <dan.j.williams@...el.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>, <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

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.

> > 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?
> 
> No, I don't.
> 
> > 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?
> 
> You may call it buggy strictly speaking, but it just assumed that if
> the runtime resume failed, the subsequent operation would just fail
> either, so -EIO would be returned to the caller.
> 
> This change allows distinguishing resume errors from I/O errors.

Ah, ok, makes sense.