Re: [PATCH 3/3] PM: hibernate: Allow disabling zero page check in copy_data_pages()

[F. R. A. Prado]

On Tue, 2026-06-02 at 10:04 +0200, Rafael J. Wysocki wrote:
> On Tue, Jun 2, 2026 at 12:52 AM Nícolas F. R. A. Prado
> <nfraprado@xxxxxxxxxxxxx> wrote:
> > 
> > On Tue, 2026-05-26 at 13:53 +0200, Rafael J. Wysocki wrote:
> > > On Mon, May 18, 2026 at 11:34 PM Nícolas F. R. A. Prado
> > > <nfraprado@xxxxxxxxxxxxx> wrote:
> > > > 
> > > > The current implementation of copy_data_pages() has at its
> > > > center
> > > > do_copy_page(), which copies data of all saveable pages in a
> > > > loop
> > > > one
> > > > long at a time, while checking whether the page is fully zero
> > > > to
> > > > allow
> > > > it to not be saved in the hibernation image.
> > > > 
> > > > As the comment above do_copy_page() mentions, this loop was
> > > > used
> > > > instead
> > > > of the more optimized copy_page() and memcpy() due to them
> > > > depending on
> > > > fpu_begin()/fpu_end() (see [1] and commit 95018f7c94cb
> > > > ("[PATCH]
> > > > swsusp:
> > > > do not use memcpy for snapshotting memory")). However, since
> > > > commit
> > > > c6dbd3e5e69c ("x86/mmx_32: Remove X86_USE_3DNOW"), this
> > > > limitation
> > > > no
> > > > longer holds, and it only affected x86-32 in the first place.
> > > > 
> > > > From testing it is clear that removing the zero page check and
> > > > using
> > > > copy_page() makes it almost 3 times quicker:
> > > > - Current:
> > > >     PM: hibernation: Copied 4940240 kbytes in 11.33 seconds
> > > > (436.03
> > > > MB/s)
> > > > - With just the zero check removed:
> > > >     PM: hibernation: Copied 4974664 kbytes in 9.03 seconds
> > > > (550.90
> > > > MB/s)
> > > > - With the zero check removed and using copy_page() instead:
> > > >     PM: hibernation: Copied 6275216 kbytes in 3.96 seconds
> > > > (1584.65
> > > > MB/s)
> > > > 
> > > > Given that copy_data_pages() runs inside a critical section
> > > > where
> > > > only
> > > > a single CPU is online and syscore is suspended, it should be
> > > > kept
> > > > as
> > > > short as possible to keep the system responsive.
> > > 
> > > I'm not sure if being responsive during hibernation is all that
> > > important.
> > > 
> > > Also, the change is about using copy_page() to reduce the time
> > > spent
> > > in syscore_suspend(), so the subject is a bit confusing.
> > > 
> > > > While switching from the loop to copy_page() brings big speed
> > > > improvements, it also makes the zero page check much costlier
> > > > since
> > > > it
> > > > can no longer be done in the middle of the copy. In fact upon
> > > > testing
> > > > adding the zero check alongside copy_page() made it slower than
> > > > the
> > > > current code.
> > > 
> > > Guess what, CPUs have caches.
> > > 
> > > > The following shows a rough comparison of a few more metrics
> > > > between the
> > > > current code and when both copy_page() is used and the zero
> > > > page
> > > > check
> > > > is disabled:
> > > > - Total time to hibernate:
> > > >   - before: 13.77s
> > > >   - after: 14.13s
> > > 
> > > Score for the current code.
> > > 
> > > > - Total time to resume:
> > > >   - before: 5.85s
> > > >   - after: 5.85s
> > > > - Total time in syscore_suspend():
> > > >   - before: 11.3s
> > > >   - after: 4.08s
> > > 
> > > Why does this matter?
> > 
> > Reducing the time spent here means reduced time between
> > pm_wakeup_pending() checks, which ultimately means the kernel is
> > quicker to abort hibernation in reaction to a pending wakeup.
> 
> That could be spelled out more directly in the patch changelog.
> 
> I'm still unsure why it matters though.  Hibernation is slow pretty
> much by definition, so why would anyone expect a fast response to a
> wakeup event during hibernation?  If they have latency constraints,
> shouldn't they use suspend instead?

We can't just rely on suspend because battery life also matters.

Indeed hibernation is slow, but if it can be made faster, or at least
quicker to respond to wakeup events, then why not?

-- 
Thanks,

Nícolas