Re: [PATCH v2] PM: hibernate: Print speed statistics of copy_data_pages()

[Rafael J. Wysocki (Intel)]

On Wed, Jul 15, 2026 at 3:51 PM Nícolas F. R. A. Prado
<nfraprado@xxxxxxxxxxxxx> wrote:
>
> copy_data_pages() can take a long (multi-second) time to finish, and
> currently the only indication of that is the timestamp difference
> between print messages right before and right after. The timestamp is
> also immediately reset afterwards to the time before image creation,
> making it even harder to spot this delay. Furthermore this function runs
> in a critical section with a single CPU online and syscore suspended, so
> it should be kept as quick as possible to keep the system responsive.
>
> Add a call to swsusp_show_speed() to report the amount of data copied,
> time taken, and copy speed of copy_data_pages() to make it easier to
> spot delays and verify performance improvements. The current time is
> obtained through local_clock() instead of ktime_get() since timekeeping
> is suspended in this region.
>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@xxxxxxxxxxxxx>
> ---
> Changes in v2:
> - Patch split from series
> - Use local_clock() instead of sched_clock()
> - Account for total pages copied (nr_pages + nr_highmem) instead of
>   final number of pages in copy (nr_copy_pages)
>
> v1: https://lore.kernel.org/all/20260518-hibernation-decrease-time-in-copy-data-pages-v1-1-3998bdf90ee5@xxxxxxxxxxxxx/
> ---
>  kernel/power/snapshot.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
> index d933b5b2c05d..42c730fadaed 100644
> --- a/kernel/power/snapshot.c
> +++ b/kernel/power/snapshot.c
> @@ -13,6 +13,7 @@
>  #include <linux/version.h>
>  #include <linux/module.h>
>  #include <linux/mm.h>
> +#include <linux/sched/clock.h>
>  #include <linux/suspend.h>
>  #include <linux/delay.h>
>  #include <linux/bitops.h>
> @@ -2109,6 +2110,7 @@ static int swsusp_alloc(struct memory_bitmap *copy_bm,
>  asmlinkage __visible int swsusp_save(void)
>  {
>         unsigned int nr_pages, nr_highmem;
> +       ktime_t start, stop;
>
>         pm_deferred_pr_dbg("Creating image\n");
>
> @@ -2130,7 +2132,14 @@ asmlinkage __visible int swsusp_save(void)
>          * Kill them.
>          */
>         drain_local_pages(NULL);
> +       start = ns_to_ktime(local_clock());
>         nr_copy_pages = copy_data_pages(&copy_bm, &orig_bm, &zero_bm);
> +       stop = ns_to_ktime(local_clock());
> +       /*
> +        * Zero pages are overwritten but still copied, so account for them
> +        * in speed calculation.
> +        */
> +       swsusp_show_speed(start, stop, nr_pages + nr_highmem, "Copied");

Sashiko has concerns about this which look valid to me:

https://sashiko.dev/#/patchset/20260715-copy-data-pages-show-speed-v2-1-6e30f15abcc1%40collabora.com

>
>         /*
>          * End of critical section. From now on, we can write to memory,
>
> ---