[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAJZ5v0iL9V9SC7fQ6_RGCMfUAZzCkCp0kR6zdZWbWtKZ6dQQ2w@mail.gmail.com>
Date: Mon, 22 Sep 2025 16:16:39 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Takashi Iwai <tiwai@...e.de>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, 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, Sep 22, 2025 at 4:07 PM Takashi Iwai <tiwai@...e.de> wrote:
>
> 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.
Agreed.
I'll send a v3 including these changes.
Powered by blists - more mailing lists