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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Fri, 4 Nov 2022 16:32:16 +0100
From:   Ulf Hansson <ulf.hansson@...aro.org>
To:     Shawn Guo <shawn.guo@...aro.org>
Cc:     "Rafael J . Wysocki" <rafael@...nel.org>,
        Kevin Hilman <khilman@...nel.org>,
        Len Brown <len.brown@...el.com>, Pavel Machek <pavel@....cz>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/4] PM: domains: Drop genpd status manipulation for
 hibernate restore

On Wed, 2 Nov 2022 at 15:21, Shawn Guo <shawn.guo@...aro.org> wrote:
>
> The genpd status manipulation for hibernate restore has really never
> worked as intended.  For example, if the genpd->status was GENPD_STATE_ON,
> the parent domain's `sd_count` must have been increased, so it needs to
> be adjusted too.  So drop this status manipulation.
>
> Suggested-by: Ulf Hansson <ulf.hansson@...aro.org>
> Signed-off-by: Shawn Guo <shawn.guo@...aro.org>

Note that, I was trying to understand a little more about the
background to the below code, although it's not entirely easy to
browse the git history around this.

Unless Rafael thinks there are reasons to keep the code as is, I
wouldn't mind seeing it go away. So:

Reviewed-by: Ulf Hansson <ulf.hansson@...aro.org>

Kind regards
Uffe

> ---
>  drivers/base/power/domain.c | 13 -------------
>  1 file changed, 13 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 6471b559230e..97deae1d4e77 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1374,20 +1374,7 @@ static int genpd_restore_noirq(struct device *dev)
>         if (IS_ERR(genpd))
>                 return -EINVAL;
>
> -       /*
> -        * At this point suspended_count == 0 means we are being run for the
> -        * first time for the given domain in the present cycle.
> -        */
>         genpd_lock(genpd);
> -       if (genpd->suspended_count++ == 0) {
> -               /*
> -                * The boot kernel might put the domain into arbitrary state,
> -                * so make it appear as powered off to genpd_sync_power_on(),
> -                * so that it tries to power it on in case it was really off.
> -                */
> -               genpd->status = GENPD_STATE_OFF;
> -       }
> -
>         genpd_sync_power_on(genpd, true, 0);
>         genpd_unlock(genpd);
>
> --
> 2.25.1
>

Powered by blists - more mailing lists