linux-kernel - Re: [RESEND PATCH v3 1/2] PCI/PM: refactor pci_pm_suspend

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: <CAJZ5v0hhc+n+WbLCxbJ2VPSWj0RAy4AM+pvqy8eiAJVtBecVWg@mail.gmail.com>
Date:   Tue, 30 Aug 2022 13:44:43 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Rajvi Jingar <rajvi.jingar@...ux.intel.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>
Cc:     Rafael Wysocki <rafael.j.wysocki@...el.com>,
        David Box <david.e.box@...ux.intel.com>,
        Linux PCI <linux-pci@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux PM <linux-pm@...r.kernel.org>
Subject: Re: [RESEND PATCH v3 1/2] PCI/PM: refactor pci_pm_suspend_noirq()

Hi Bjorn,

On Tue, Aug 30, 2022 at 12:49 PM Rajvi Jingar
<rajvi.jingar@...ux.intel.com> wrote:
>
> The state of the device is saved during pci_pm_suspend_noirq(), if it
> has not already been saved, regardless of the skip_bus_pm flag value. So
> skip_bus_pm check is removed before saving the device state.
>
> Signed-off-by: Rajvi Jingar <rajvi.jingar@...ux.intel.com>
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>

I have reviewed this and the [2/2] already and they are clear
improvements to me.

Do you have any concerns regarding any of them?

If not, do you want me to pick them up or do you plan to take care of
them yourself?

> ---
>  v1->v2: no change
>  v2->v3: no change
> ---
>  drivers/pci/pci-driver.c | 18 ++++++------------
>  1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 49238ddd39ee..1f64de3e5280 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -867,20 +867,14 @@ static int pci_pm_suspend_noirq(struct device *dev)
>                 }
>         }
>
> -       if (pci_dev->skip_bus_pm) {
> +       if (!pci_dev->state_saved) {
> +               pci_save_state(pci_dev);
>                 /*
> -                * Either the device is a bridge with a child in D0 below it, or
> -                * the function is running for the second time in a row without
> -                * going through full resume, which is possible only during
> -                * suspend-to-idle in a spurious wakeup case.  The device should
> -                * be in D0 at this point, but if it is a bridge, it may be
> -                * necessary to save its state.
> +                * If the device is a bridge with a child in D0 below it, it needs to
> +                * stay in D0, so check skip_bus_pm to avoid putting it into a
> +                * low-power state in that case.
>                  */
> -               if (!pci_dev->state_saved)
> -                       pci_save_state(pci_dev);
> -       } else if (!pci_dev->state_saved) {
> -               pci_save_state(pci_dev);
> -               if (pci_power_manageable(pci_dev))
> +               if (!pci_dev->skip_bus_pm && pci_power_manageable(pci_dev))
>                         pci_prepare_to_sleep(pci_dev);
>         }
>
> --
> 2.25.1
>