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

[Takashi Iwai <tiwai@suse.de>]

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@suse.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@suse.de> wrote:
> > > >
> > > > On Sat, 20 Sep 2025 12:54:58 +0200,
> > > > Rafael J. Wysocki wrote:
> > > > >
> > > > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.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