linux-kernel - Re: [PATCH v2 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations

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: <87zfammvgs.wl-tiwai@suse.de>
Date: Mon, 22 Sep 2025 16:07:47 +0200
From: Takashi Iwai <tiwai@...e.de>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: Takashi Iwai <tiwai@...e.de>,
	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>
Subject: Re: [PATCH v2 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations

On Mon, 22 Sep 2025 15:44:51 +0200,
Rafael J. Wysocki wrote:
> 
> On Mon, Sep 22, 2025 at 3:32 PM Takashi Iwai <tiwai@...e.de> wrote:
> >
> > On Mon, 22 Sep 2025 14:50:32 +0200,
> > Rafael J. Wysocki wrote:
> > >
> > > On Mon, Sep 22, 2025 at 10:38 AM Takashi Iwai <tiwai@...e.de> wrote:
> > > >
> > > > On Sat, 20 Sep 2025 12:54:58 +0200,
> > > > Rafael J. Wysocki wrote:
> > > > >
> > > > > From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > > > >
> > > > > It is generally useful to be able to automatically drop a device's
> > > > > runtime PM usage counter incremented by runtime PM operations that
> > > > > resume a device and bump up its usage counter [1].
> > > > >
> > > > > To that end, add DEFINE_CLASS() macros allowing pm_runtime_put()
> > > > > and pm_runtime_put_autosuspend() to be used for the auto-cleanup in
> > > > > those cases.
> > > > >
> > > > > Simply put, a piece of code like below:
> > > > >
> > > > >       pm_runtime_get_sync(dev);
> > > > >       .....
> > > > >       pm_runtime_put(dev);
> > > > >       return 0;
> > > > >
> > > > > can be transformed with CLASS() like:
> > > > >
> > > > >       CLASS(pm_runtime_get_active, pm)(dev);
> > > > >       if (IS_ERR(pm))
> > > > >               return PTR_ERR(pm);
> > > > >       .....
> > > > >       return 0;
> > > > >
> > > > > (note the new resume error handling).
> > > >
> > > > Do we still allow the code without the error check even using CLASS()?
> > > > Although the error check should be handled, it's not mandatory for
> > > > now.  That said, the above example could be still in a form like:
> > > >
> > > >         CLASS(pm_runtime_get_active, pm)(dev);
> > > >         .....
> > > >         return 0;
> > > >
> > > > while adding the proper error check is recommended?
> > >
> > > I'd rather not encourage doing this.
> > >
> > > While it may still produce working code in some cases, one needs to
> > > remember that in case of a runtime resume error it will be running
> > > without a runtime PM reference it has attempted to acquire.
> >
> > Fair enough.  Then it'd be also good to mention that in the
> > description, too.
> 
> I can also add classes for the cases in which resume errors can be
> neglected, like these:
> 
> DEFINE_CLASS(pm_runtime_get_sync, struct device *,
>          if (_T) pm_runtime_put(_T),
>          ({ pm_runtime_get_sync(dev); dev; }), struct device *dev)
> 
> DEFINE_CLASS(pm_runtime_get_sync_auto, struct device *,
>          if (_T) pm_runtime_put_autosuspend(_T),
>          ({ pm_runtime_get_sync(dev); dev; }), struct device *dev)
> 
> with a comment explaining what they are for.

It might be helpful, indeed, since the error handling isn't always
straightforward, and this still allows us to convert to the
auto-cleanup safely.  It's still worth to mention that those aren't
recommended options, though.


thanks,

Takashi