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: <CAJZ5v0iZJFQeBhA7tM-sWuJDtisvrHGjPPdQHrC-eXXF1xJpbA@mail.gmail.com>
Date: Thu, 16 Oct 2025 22:38:35 +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 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?

> 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.