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: <CAJZ5v0iOgbkJbdRzgrBUaaYL+S_8BZD7XuXdK5vs2gMG3ug1KA@mail.gmail.com>
Date: Thu, 16 Oct 2025 15:46:08 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Jonathan Cameron <jonathan.cameron@...wei.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Linux PM <linux-pm@...r.kernel.org>, 
	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>, 
	Mika Westerberg <mika.westerberg@...ux.intel.com>, Linux ACPI <linux-acpi@...r.kernel.org>, 
	Dan Williams <dan.j.williams@...el.com>, David Lechner <dlechner@...libre.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

On Thu, Oct 16, 2025 at 2:39 PM Jonathan Cameron
<jonathan.cameron@...wei.com> wrote:
>
> On Wed, 15 Oct 2025 16:02:02 +0200
> "Rafael J. Wysocki" <rafael@...nel.org> wrote:
>
> > From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> >
> > There appears to be an emerging pattern in which guard
> > pm_runtime_active_try is used for resuming the given device and
> > incrementing its runtime PM usage counter if the resume has been
> > successful, that is followed by an ACQUIRE_ERR() check on the guard
> > variable and if that triggers, a specific error code is returned, for
> > example:
> >
> >       ACQUIRE(pm_runtime_active_try, pm)(dev);
> >       if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
> >               return -ENXIO
> >
> > Introduce a macro called PM_RUNTIME_ACQUIRE_OR_FAIL() representing the
> > above sequence of statements that can be used to avoid code duplication
> > wherever that sequence would be used.
> >
> > Use this macro right away in the PCI sysfs code where the above pattern
> > is already present.
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > ---
> >
> > Admittedly, the new macro is slightly on the edge, but it really helps
> > reduce code duplication, so here it goes.
>
> Fully agree with the 'on the edge'.
>
> This looks somewhat like the some of the earlier attempts to come up with
> a general solution before ACQUIRE().  Linus was fairly clear on his opinion of
> a proposal that looked a bit similar to this
> cond_guard(mutex_intr, return -EINTR, &mutex);
>
> https://lore.kernel.org/all/CAHk-=win7bwWhPJ=iuW4h-sDTqbX6v9_LJnMaO3KxVfPSs81bQ@mail.gmail.com/
>
> +CC a few people who might have better memories of where things went than I do.
>
> The solution you have here has the benefit of clarity that all it can do is
> return the error code.

Well, I could call the macro PM_RUNTIME_ACQUIRE_OR_RETURN_ERROR(), but
FAIL is just shorter. :-)

Seriously though, the odd syntax bothers me, but it has come from
looking at the multiple pieces of code that otherwise would have
repeated exactly the same code pattern including the guard name in two
places and the pm variable that has no role beyond guarding.

> > ---
> >  drivers/pci/pci-sysfs.c    |    4 +---
> >  include/linux/pm_runtime.h |    5 +++++
> >  2 files changed, 6 insertions(+), 3 deletions(-)
> >
> > --- a/drivers/pci/pci-sysfs.c
> > +++ b/drivers/pci/pci-sysfs.c
> > @@ -1517,9 +1517,7 @@ static ssize_t reset_method_store(struct
> >               return count;
> >       }
> >
> > -     ACQUIRE(pm_runtime_active_try, pm)(dev);
> > -     if (ACQUIRE_ERR(pm_runtime_active_try, &pm))
> > -             return -ENXIO;
> > +     PM_RUNTIME_ACQUIRE_OR_FAIL(dev, -ENXIO);
> >
> >       if (sysfs_streq(buf, "default")) {
> >               pci_init_reset_methods(pdev);
> > --- a/include/linux/pm_runtime.h
> > +++ b/include/linux/pm_runtime.h
> > @@ -637,6 +637,11 @@ DEFINE_GUARD_COND(pm_runtime_active_auto
> >  DEFINE_GUARD_COND(pm_runtime_active_auto, _try_enabled,
> >                 pm_runtime_resume_and_get(_T))
> >
> > +#define PM_RUNTIME_ACQUIRE_OR_FAIL(dev, error)               \
> > +     ACQUIRE(pm_runtime_active_try, __pm)(dev);      \
> > +     if (ACQUIRE_ERR(pm_runtime_active_try, &__pm))  \
> > +             return error
> > +
> >  /**
> >   * pm_runtime_put_sync - Drop device usage counter and run "idle check" if 0.
> >   * @dev: Target device.
> >
> >
> >
>