Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230523074535.249802-1-hch@lst.de> <20230523074535.249802-4-hch@lst.de>
In-Reply-To: <20230523074535.249802-4-hch@lst.de>
From:   "Rafael J. Wysocki" <rafael@kernel.org>
Date:   Tue, 23 May 2023 20:30:13 +0200
Message-ID: <CAJZ5v0i2eXi+UEqm0W2eiadjUw5AHT-c1OHTQkemacfL0-jo2A@mail.gmail.com>
Subject: Re: [PATCH 03/24] PM: hibernate: remove the global snapshot_test variable
To:     Christoph Hellwig <hch@lst.de>
Cc:     Jens Axboe <axboe@kernel.dk>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Mike Snitzer <snitzer@kernel.org>,
        Joern Engel <joern@lazybastard.org>,
        Miquel Raynal <miquel.raynal@bootlin.com>,
        Richard Weinberger <richard@nod.at>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        Pavel Machek <pavel@ucw.cz>, dm-devel@redhat.com,
        linux-kernel@vger.kernel.org, linux-block@vger.kernel.org,
        linux-mtd@lists.infradead.org, linux-pm@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

On Tue, May 23, 2023 at 9:45 AM Christoph Hellwig <hch@lst.de> wrote:
>
> Passing call dependent variable in global variables is a huge
> antipattern.  Fix it up.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: Rafael J. Wysocki <rafael@kernel.org>

> ---
>  kernel/power/hibernate.c | 17 ++++++-----------
>  kernel/power/power.h     |  3 +--
>  kernel/power/swap.c      |  2 +-
>  3 files changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index 07279506366255..78696aa04f5ca3 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -64,7 +64,6 @@ enum {
>  static int hibernation_mode = HIBERNATION_SHUTDOWN;
>
>  bool freezer_test_done;
> -bool snapshot_test;
>
>  static const struct platform_hibernation_ops *hibernation_ops;
>
> @@ -684,7 +683,7 @@ static void power_down(void)
>                 cpu_relax();
>  }
>
> -static int load_image_and_restore(void)
> +static int load_image_and_restore(bool snapshot_test)
>  {
>         int error;
>         unsigned int flags;
> @@ -721,6 +720,7 @@ static int load_image_and_restore(void)
>   */
>  int hibernate(void)
>  {
> +       bool snapshot_test = false;
>         unsigned int sleep_flags;
>         int error;
>
> @@ -748,9 +748,6 @@ int hibernate(void)
>         if (error)
>                 goto Exit;
>
> -       /* protected by system_transition_mutex */
> -       snapshot_test = false;
> -
>         lock_device_hotplug();
>         /* Allocate memory management structures */
>         error = create_basic_memory_bitmaps();
> @@ -792,9 +789,9 @@ int hibernate(void)
>         unlock_device_hotplug();
>         if (snapshot_test) {
>                 pm_pr_dbg("Checking hibernation image\n");
> -               error = swsusp_check();
> +               error = swsusp_check(snapshot_test);
>                 if (!error)
> -                       error = load_image_and_restore();
> +                       error = load_image_and_restore(snapshot_test);
>         }
>         thaw_processes();
>
> @@ -982,8 +979,6 @@ static int software_resume(void)
>          */
>         mutex_lock_nested(&system_transition_mutex, SINGLE_DEPTH_NESTING);
>
> -       snapshot_test = false;
> -
>         if (!swsusp_resume_device) {
>                 error = find_resume_device();
>                 if (error)
> @@ -994,7 +989,7 @@ static int software_resume(void)
>                 MAJOR(swsusp_resume_device), MINOR(swsusp_resume_device));
>
>         pm_pr_dbg("Looking for hibernation image.\n");
> -       error = swsusp_check();
> +       error = swsusp_check(false);
>         if (error)
>                 goto Unlock;
>
> @@ -1022,7 +1017,7 @@ static int software_resume(void)
>                 goto Close_Finish;
>         }
>
> -       error = load_image_and_restore();
> +       error = load_image_and_restore(false);
>         thaw_processes();
>   Finish:
>         pm_notifier_call_chain(PM_POST_RESTORE);
> diff --git a/kernel/power/power.h b/kernel/power/power.h
> index b83c8d5e188dec..978189fcafd124 100644
> --- a/kernel/power/power.h
> +++ b/kernel/power/power.h
> @@ -59,7 +59,6 @@ asmlinkage int swsusp_save(void);
>
>  /* kernel/power/hibernate.c */
>  extern bool freezer_test_done;
> -extern bool snapshot_test;
>
>  extern int hibernation_snapshot(int platform_mode);
>  extern int hibernation_restore(int platform_mode);
> @@ -174,7 +173,7 @@ extern int swsusp_swap_in_use(void);
>  #define SF_HW_SIG              8
>
>  /* kernel/power/hibernate.c */
> -extern int swsusp_check(void);
> +int swsusp_check(bool snapshot_test);
>  extern void swsusp_free(void);
>  extern int swsusp_read(unsigned int *flags_p);
>  extern int swsusp_write(unsigned int flags);
> diff --git a/kernel/power/swap.c b/kernel/power/swap.c
> index 92e41ed292ada8..efed11568bfc72 100644
> --- a/kernel/power/swap.c
> +++ b/kernel/power/swap.c
> @@ -1514,7 +1514,7 @@ int swsusp_read(unsigned int *flags_p)
>   *      swsusp_check - Check for swsusp signature in the resume device
>   */
>
> -int swsusp_check(void)
> +int swsusp_check(bool snapshot_test)
>  {
>         int error;
>         void *holder;
> --
> 2.39.2
>