public inbox for linux-pm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] PM: runtime: Auto-cleanup macros for runtime PM
@ 2025-09-20 10:49 Rafael J. Wysocki
  2025-09-20 10:54 ` [PATCH v2 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations Rafael J. Wysocki
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2025-09-20 10:49 UTC (permalink / raw)
  To: Linux PM, Takashi Iwai
  Cc: LKML, Linux PCI, Alex Williamson, Bjorn Helgaas, Zhang Qilong,
	Ulf Hansson

Hi All,

This supersedes both

https://lore.kernel.org/linux-pm/5049058.31r3eYUQgx@rafael.j.wysocki/

and

https://lore.kernel.org/linux-pm/20250919163147.4743-1-tiwai@suse.de/

that were sent simultaneously by mistake and both made a mistake of
forgetting that __pm_runtime_suspend() returns an error code for devices
with runtime PM disabled.

The first patch in this series has been modified to provide additional
two macros for the cases in which runtime PM is expected to work
transparently if runtime PM has been disabled for the given device.
Also the names of the new classes defined in it and the new helper
static inline function have been changed.

The second patch has been updated to reflect the changes in the first patch.

The last patch is basically the same as before.

Thanks!




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations
  2025-09-20 10:49 [PATCH v2 0/3] PM: runtime: Auto-cleanup macros for runtime PM Rafael J. Wysocki
@ 2025-09-20 10:54 ` Rafael J. Wysocki
  2025-09-22  8:38   ` Takashi Iwai
  2025-09-20 10:56 ` [PATCH v2 2/3] PCI/sysfs: Use runtime PM class macro for auto-cleanup Rafael J. Wysocki
  2025-09-20 10:56 ` [PATCH v2 3/3] PM: runtime: Drop DEFINE_FREE() for pm_runtime_put Rafael J. Wysocki
  2 siblings, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2025-09-20 10:54 UTC (permalink / raw)
  To: Linux PM, Takashi Iwai
  Cc: LKML, Linux PCI, Alex Williamson, Bjorn Helgaas, Zhang Qilong,
	Ulf Hansson

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

In all of the cases in which runtime PM is known to be enabled for the
given device or it can be regarded as operational (and so it can be
accessed) with runtime PM disabled, a piece of code like:

	ret = pm_runtime_resume_and_get(dev);
	if (ret < 0)
		return ret;
	.....
	pm_runtime_put(dev);
	return 0;

can be simplified with CLASS() like:

	CLASS(pm_runtime_get_active, pm)(dev);
	if (IS_ERR(pm))
		return PTR_ERR(pm);
	.....
	return 0;

(see pm_runtime_put() call is gone).

However, if the device cannot be accessed unless runtime PM has been
enabled for it, the CLASS(pm_runtime_get_active_enabled) variant
needs to be used, that is (in the context of the example above):

	CLASS(pm_runtime_get_active_enabled, pm)(dev);
	if (IS_ERR(pm))
		return PTR_ERR(pm);
	.....
	return 0;

When the original code calls pm_runtime_put_autosuspend(), use
CLASS(pm_runtime_get_active_enabled_auto) variant, instead.
For example, a piece of code like:

	ret = pm_runtime_resume_and_get(dev);
	if (ret < 0)
		return ret;
	.....
	pm_runtime_put_autosuspend(dev);
	return 0;

will become:

	CLASS(pm_runtime_get_active_enabled_auto, pm)(dev);
	if (IS_ERR(pm))
		return PTR_ERR(pm);
	.....
	return 0;

Note that the cases in which the return value of pm_runtime_get_sync()
is checked can also be handled with the help of the new class macros.
For example, a piece of code like:

	ret = pm_runtime_get_sync(dev);
	if (ret < 0) {
		pm_runtime_put(dev);
		return ret;
	}
	.....
	pm_runtime_put(dev);
	return 0;

can be rewritten as:

	CLASS(pm_runtime_get_active_enabled, pm)(dev);
	if (IS_ERR(pm))
		return PTR_ERR(pm);
	.....
	return 0;

Link: https://lore.kernel.org/linux-pm/878qimv24u.wl-tiwai@suse.de/ [1]
Co-developed-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v1 -> v2:
   * Rename the new classes and the new static inline helper.
   * Add two classes for handling disabled runtime PM.
   * Expand the changelog.
   * Adjust the subject.

---
 drivers/base/power/runtime.c |    2 +
 include/linux/pm_runtime.h   |   70 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+)

--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -796,6 +796,8 @@ static int rpm_resume(struct device *dev
 		if (dev->power.runtime_status == RPM_ACTIVE &&
 		    dev->power.last_status == RPM_ACTIVE)
 			retval = 1;
+		else if (rpmflags & RPM_TRANSPARENT)
+			goto out;
 		else
 			retval = -EACCES;
 	}
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -21,6 +21,7 @@
 #define RPM_GET_PUT		0x04	/* Increment/decrement the
 					    usage_count */
 #define RPM_AUTO		0x08	/* Use autosuspend_delay */
+#define RPM_TRANSPARENT	0x10	/* Succeed if runtime PM is disabled */
 
 /*
  * Use this for defining a set of PM operations to be used in all situations
@@ -533,6 +534,32 @@ static inline int pm_runtime_resume_and_
 }
 
 /**
+ * pm_runtime_get_active_dev - Resume a device and bump up its usage counter.
+ * @dev: Target device.
+ * @rpmflags: Additional runtime PM flags to combine with RPM_GET_PUT.
+ *
+ * Resume @dev synchronously and if that is successful, increment its runtime
+ * PM usage counter.
+ *
+ * Return:
+ * * 0 if the runtime PM usage counter of @dev has been incremented.
+ * * Negative error code otherwise.
+ */
+static inline struct device *pm_runtime_get_active_dev(struct device *dev,
+						       int rpmflags)
+{
+	int ret;
+
+	ret = __pm_runtime_resume(dev, RPM_GET_PUT | rpmflags);
+	if (ret < 0) {
+		pm_runtime_put_noidle(dev);
+		return ERR_PTR(ret);
+	}
+
+	return dev;
+}
+
+/**
  * pm_runtime_put - Drop device usage counter and queue up "idle check" if 0.
  * @dev: Target device.
  *
@@ -606,6 +633,49 @@ static inline int pm_runtime_put_autosus
 	return __pm_runtime_put_autosuspend(dev);
 }
 
+/*
+ * The way to use the classes defined below is to define a class variable and
+ * use it going forward for representing the target device until it goes out of
+ * the scope.  For example:
+ *
+ * CLASS(pm_runtime_get_active, active_dev)(dev);
+ * if (IS_ERR(active_dev))
+ *         return PTR_ERR(active_dev);
+ *
+ * ... do something with active_dev (which is guaranteed to never suspend) ...
+ */
+DEFINE_CLASS(pm_runtime_get_active, struct device *,
+	     if (!IS_ERR_OR_NULL(_T)) pm_runtime_put(_T),
+	     pm_runtime_get_active_dev(dev, RPM_TRANSPARENT), struct device *dev)
+
+DEFINE_CLASS(pm_runtime_get_active_auto, struct device *,
+	     if (!IS_ERR_OR_NULL(_T)) pm_runtime_put_autosuspend(_T),
+	     pm_runtime_get_active_dev(dev, RPM_TRANSPARENT), struct device *dev)
+
+/*
+ * The following two classes are analogous to the two classes defined above,
+ * respectively, but they produce an error pointer if runtime PM has been
+ * disabled for the given device.
+ *
+ * They should be used in the cases when a device with runtime PM disabled is
+ * not regarded as operational and so it cannot be accessed.  The classes
+ * defined above should be used in all of the other cases.
+ */
+DEFINE_CLASS(pm_runtime_get_active_enabled, struct device *,
+	     if (!IS_ERR_OR_NULL(_T)) pm_runtime_put(_T),
+	     pm_runtime_get_active_dev(dev, 0), struct device *dev)
+
+DEFINE_CLASS(pm_runtime_get_active_enabled_auto, struct device *,
+	     if (!IS_ERR_OR_NULL(_T)) pm_runtime_put_autosuspend(_T),
+	     pm_runtime_get_active_dev(dev, 0), struct device *dev)
+
+/*
+ * The following two classes are analogous to the two classes defined above,
+ * respectively, but they transparently handle devices with runtime PM disabled.
+ *
+ * They can be used when it is known that runtime PM has been enabled for the
+ * given device.
+ */
 /**
  * pm_runtime_put_sync - Drop device usage counter and run "idle check" if 0.
  * @dev: Target device.




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 2/3] PCI/sysfs: Use runtime PM class macro for auto-cleanup
  2025-09-20 10:49 [PATCH v2 0/3] PM: runtime: Auto-cleanup macros for runtime PM Rafael J. Wysocki
  2025-09-20 10:54 ` [PATCH v2 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations Rafael J. Wysocki
@ 2025-09-20 10:56 ` Rafael J. Wysocki
  2025-09-20 10:56 ` [PATCH v2 3/3] PM: runtime: Drop DEFINE_FREE() for pm_runtime_put Rafael J. Wysocki
  2 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2025-09-20 10:56 UTC (permalink / raw)
  To: Linux PM, Takashi Iwai
  Cc: LKML, Linux PCI, Alex Williamson, Bjorn Helgaas, Zhang Qilong,
	Ulf Hansson

From: Takashi Iwai <tiwai@suse.de>

Use the newly introduced class macro to simplify the code.

Also, add the proper error handling for the PM runtime get errors.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20250919163147.4743-3-tiwai@suse.de
[ rjw: Adjusted the subject and the name of the class ]
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v1 -> v2:
   * Adjust the name of the class to handle the disabled runtime PM case
     transparently (like the original code).

---
 drivers/pci/pci-sysfs.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1475,8 +1475,9 @@ static ssize_t reset_method_store(struct
 		return count;
 	}
 
-	pm_runtime_get_sync(dev);
-	struct device *pmdev __free(pm_runtime_put) = dev;
+	CLASS(pm_runtime_get_active, pmdev)(dev);
+	if (IS_ERR(pmdev))
+		return -ENXIO;
 
 	if (sysfs_streq(buf, "default")) {
 		pci_init_reset_methods(pdev);




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 3/3] PM: runtime: Drop DEFINE_FREE() for pm_runtime_put
  2025-09-20 10:49 [PATCH v2 0/3] PM: runtime: Auto-cleanup macros for runtime PM Rafael J. Wysocki
  2025-09-20 10:54 ` [PATCH v2 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations Rafael J. Wysocki
  2025-09-20 10:56 ` [PATCH v2 2/3] PCI/sysfs: Use runtime PM class macro for auto-cleanup Rafael J. Wysocki
@ 2025-09-20 10:56 ` Rafael J. Wysocki
  2 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2025-09-20 10:56 UTC (permalink / raw)
  To: Linux PM, Takashi Iwai
  Cc: LKML, Linux PCI, Alex Williamson, Bjorn Helgaas, Zhang Qilong,
	Ulf Hansson

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The DEFINE_FREE() for pm_runtime_put has been superseded by recently
introduced runtime PM auto-cleanup macros and its only user has been
converted to using one of the new macros, so drop it.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

v1 -> v2: No changes.

---
 include/linux/pm_runtime.h |    2 --
 1 file changed, 2 deletions(-)

--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -582,8 +582,6 @@ static inline int pm_runtime_put(struct
 	return __pm_runtime_idle(dev, RPM_GET_PUT | RPM_ASYNC);
 }
 
-DEFINE_FREE(pm_runtime_put, struct device *, if (_T) pm_runtime_put(_T))
-
 /**
  * __pm_runtime_put_autosuspend - Drop device usage counter and queue autosuspend if 0.
  * @dev: Target device.




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations
  2025-09-20 10:54 ` [PATCH v2 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations Rafael J. Wysocki
@ 2025-09-22  8:38   ` Takashi Iwai
  2025-09-22 12:50     ` Rafael J. Wysocki
  0 siblings, 1 reply; 10+ messages in thread
From: Takashi Iwai @ 2025-09-22  8:38 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM, Takashi Iwai, LKML, Linux PCI, Alex Williamson,
	Bjorn Helgaas, Zhang Qilong, Ulf Hansson

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?


thanks,

Takashi

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations
  2025-09-22  8:38   ` Takashi Iwai
@ 2025-09-22 12:50     ` Rafael J. Wysocki
  2025-09-22 13:32       ` Takashi Iwai
  0 siblings, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2025-09-22 12:50 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Rafael J. Wysocki, Linux PM, LKML, Linux PCI, Alex Williamson,
	Bjorn Helgaas, Zhang Qilong, Ulf Hansson

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations
  2025-09-22 12:50     ` Rafael J. Wysocki
@ 2025-09-22 13:32       ` Takashi Iwai
  2025-09-22 13:44         ` Rafael J. Wysocki
  0 siblings, 1 reply; 10+ messages in thread
From: Takashi Iwai @ 2025-09-22 13:32 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Takashi Iwai, Linux PM, LKML, Linux PCI, Alex Williamson,
	Bjorn Helgaas, Zhang Qilong, Ulf Hansson

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.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations
  2025-09-22 13:32       ` Takashi Iwai
@ 2025-09-22 13:44         ` Rafael J. Wysocki
  2025-09-22 14:07           ` Takashi Iwai
  0 siblings, 1 reply; 10+ messages in thread
From: Rafael J. Wysocki @ 2025-09-22 13:44 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Rafael J. Wysocki, Linux PM, LKML, Linux PCI, Alex Williamson,
	Bjorn Helgaas, Zhang Qilong, Ulf Hansson

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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations
  2025-09-22 13:44         ` Rafael J. Wysocki
@ 2025-09-22 14:07           ` Takashi Iwai
  2025-09-22 14:16             ` Rafael J. Wysocki
  0 siblings, 1 reply; 10+ messages in thread
From: Takashi Iwai @ 2025-09-22 14:07 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Takashi Iwai, Linux PM, LKML, Linux PCI, Alex Williamson,
	Bjorn Helgaas, Zhang Qilong, Ulf Hansson

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations
  2025-09-22 14:07           ` Takashi Iwai
@ 2025-09-22 14:16             ` Rafael J. Wysocki
  0 siblings, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2025-09-22 14:16 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Rafael J. Wysocki, Linux PM, LKML, Linux PCI, Alex Williamson,
	Bjorn Helgaas, Zhang Qilong, Ulf Hansson

On Mon, Sep 22, 2025 at 4:07 PM Takashi Iwai <tiwai@suse.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@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.

Agreed.

I'll send a v3 including these changes.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-09-22 14:16 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-20 10:49 [PATCH v2 0/3] PM: runtime: Auto-cleanup macros for runtime PM Rafael J. Wysocki
2025-09-20 10:54 ` [PATCH v2 1/3] PM: runtime: Add auto-cleanup macros for "resume and get" operations Rafael J. Wysocki
2025-09-22  8:38   ` Takashi Iwai
2025-09-22 12:50     ` Rafael J. Wysocki
2025-09-22 13:32       ` Takashi Iwai
2025-09-22 13:44         ` Rafael J. Wysocki
2025-09-22 14:07           ` Takashi Iwai
2025-09-22 14:16             ` Rafael J. Wysocki
2025-09-20 10:56 ` [PATCH v2 2/3] PCI/sysfs: Use runtime PM class macro for auto-cleanup Rafael J. Wysocki
2025-09-20 10:56 ` [PATCH v2 3/3] PM: runtime: Drop DEFINE_FREE() for pm_runtime_put Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox