linux-kernel - Re: [PATCH] rpm: pm: enable PM_RPM

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: <CAOuDEK3x95u6+68e=fkejnjWhRrA5Yt1qTaAG3Prje8C-+6dmw@mail.gmail.com>
Date:   Wed, 8 Nov 2023 16:44:24 +0800
From:   Guan-Yu Lin <guanyulin@...gle.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     rafael@...nel.org, len.brown@...el.com, pavel@....cz,
        stern@...land.harvard.edu, heikki.krogerus@...ux.intel.com,
        mkl@...gutronix.de, hadess@...ess.net, mailhol.vincent@...adoo.fr,
        ivan.orlov0322@...il.com, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
        pumahsu@...gle.com, raychi@...gle.com, albertccwang@...gle.com
Subject: Re: [PATCH] rpm: pm: enable PM_RPM_EXCEPTION config flag

On Tue, Oct 31, 2023 at 5:48 PM Greg KH <gregkh@...uxfoundation.org> wrote:
>
> On Tue, Oct 31, 2023 at 05:38:55PM +0800, Guan-Yu Lin wrote:
> > Introducing PM_RPM_EXCEPTION config flag, which may alter the priority
> > between system power management and runtime power management. In
> > suspend-to-idle flow, PM core will suspend all devices to avoid device
> > interact with the system. However, chances are devices might be used by
> > other systems rather than a single system. In this case, PM core shouldn't
> > suspend the devices. One may use PM_RPM_EXCEPTION config flag to mark
> > such exception, and determine the power state of a device with runtime
> > power management rather than system power management.
> >
> > Signed-off-by: Guan-Yu Lin <guanyulin@...gle.com>
> > ---
> >  drivers/usb/core/generic.c |  6 ++++++
> >  drivers/usb/core/usb.h     | 16 ++++++++++++++++
> >  kernel/power/Kconfig       |  8 ++++++++
> >  3 files changed, 30 insertions(+)
> >
> > diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c
> > index 740342a2812a..bb0dfcfc9764 100644
> > --- a/drivers/usb/core/generic.c
> > +++ b/drivers/usb/core/generic.c
> > @@ -266,6 +266,9 @@ int usb_generic_driver_suspend(struct usb_device *udev, pm_message_t msg)
> >  {
> >       int rc;
> >
> > +     if (usb_runtime_pm_exception(udev))
> > +             return 0;
> > +
> >       /* Normal USB devices suspend through their upstream port.
> >        * Root hubs don't have upstream ports to suspend,
> >        * so we have to shut down their downstream HC-to-USB
> > @@ -294,6 +297,9 @@ int usb_generic_driver_resume(struct usb_device *udev, pm_message_t msg)
> >  {
> >       int rc;
> >
> > +     if (usb_runtime_pm_exception(udev))
> > +             return 0;
> > +
> >       /* Normal USB devices resume/reset through their upstream port.
> >        * Root hubs don't have upstream ports to resume or reset,
> >        * so we have to start up their downstream HC-to-USB
> > diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
> > index 60363153fc3f..14a054f814a2 100644
> > --- a/drivers/usb/core/usb.h
> > +++ b/drivers/usb/core/usb.h
> > @@ -90,6 +90,22 @@ extern void usb_major_cleanup(void);
> >  extern int usb_device_supports_lpm(struct usb_device *udev);
> >  extern int usb_port_disable(struct usb_device *udev);
> >
> > +#ifdef       CONFIG_PM_RPM_EXCEPTION
> > +
> > +static inline int usb_runtime_pm_exception(struct usb_device *udev)
> > +{
> > +     return atomic_read(&udev->dev.power.usage_count);
> > +}
> > +
> > +#else
> > +
> > +static inline int usb_runtime_pm_exception(struct usb_device *udev)
> > +{
> > +     return 0;
> > +}
> > +
> > +#endif
> > +
> >  #ifdef       CONFIG_PM
> >
> >  extern int usb_suspend(struct device *dev, pm_message_t msg);
> > diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> > index 4b31629c5be4..beba7a0f3947 100644
> > --- a/kernel/power/Kconfig
> > +++ b/kernel/power/Kconfig
> > @@ -193,6 +193,14 @@ config PM
> >         responsible for the actual handling of device suspend requests and
> >         wake-up events.
> >
> > +config PM_RPM_EXCEPTION
> > +     bool "Prioritize Runtime Power Management more than Power Management"
> > +     default n
>
> The default is always 'n' so no need to specify it.
>

Thanks, I will include this in the next version.

> > +     help
> > +     Provides a way to prioritize Runtime Power Management more than Power
> > +     Management. This way system can suspnd with maintaining specific
> > +     components in operation.
>
> This really doesn't give me a good description of why someone would ever
> want to enable this at all.
>
> And why does this have to be a build option?  That feels very heavy, why
> not make it changable at runtime?
>
> If this is a build option, how are you going to get all the distros and
> all of the Android/ChromeOS systems in the world to enable it?
>
> thanks,
>
> greg k-h

Let's reach a consensus on what this patch should do first. I'll then
change the description and implementation accordingly if needed. Please
see the next reply for an explanation of my idea.

Thanks,
Guan-Yu