[PATCH v2 0/2] PM: hibernate: Drain trailing zero pages on userspace restore

[PATCH v2 0/2] PM: hibernate: Drain trailing zero pages on userspace restore

[Alberto Garcia]

Hi,

Here's v2 of the patch.

Changes from v1:

- The extra snapshot_write_next() call happens now at the beginning of
  snapshot_write_finalize(), but only when handle->cur is in the data
  page region.

- We return -ENODATA if snapshot_write_next() returns > 0 (indicating
  that the kernel was expecting another page).

- A second patch changes the userspace restore code to return -ENODATA
  when snapshot_image_loaded() fails.

Regards,

Berto

Alberto Garcia (2):
  PM: hibernate: Drain trailing zero pages on userspace restore
  PM: hibernate: return -ENODATA if the snapshot image is not loaded

 kernel/power/snapshot.c | 11 +++++++++++
 kernel/power/user.c     |  7 +++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

-- 
2.47.3

[PATCH v2 1/2] PM: hibernate: Drain trailing zero pages on userspace restore

[Alberto Garcia]

Commit 005e8dddd497 ("PM: hibernate: don't store zero pages in the
image file") added an optimization to skip zero-filled pages in the
hibernation image. On restore, zero pages are handled internally by
snapshot_write_next() in a loop that processes them without returning
to the caller.

With the userspace restore interface, writing the last non-zero page
to /dev/snapshot is followed by the SNAPSHOT_ATOMIC_RESTORE ioctl. At
this point there are no more calls to snapshot_write_next() so any
trailing zero pages are not processed, snapshot_image_loaded() fails
because handle->cur is smaller than expected, the ioctl returns -EPERM
and the image is not restored.

The in-kernel restore path is not affected by this because the loop in
load_image() in swap.c calls snapshot_write_next() until it returns 0.
It is this final call that drains any trailing zero pages.

Fixed by calling snapshot_write_next() in snapshot_write_finalize(),
giving the kernel the chance to drain any trailing zero pages.

Fixes: 005e8dddd497 ("PM: hibernate: don't store zero pages in the image file")
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 kernel/power/snapshot.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index 6e1321837c66..a564650734dc 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -2855,6 +2855,17 @@ int snapshot_write_finalize(struct snapshot_handle *handle)
 {
 	int error;
 
+	/*
+	 * Call snapshot_write_next() to drain any trailing zero pages,
+	 * but make sure we're in the data page region first.
+	 * This function can return PAGE_SIZE if the kernel was expecting
+	 * another copy page. Return -ENODATA in that situation.
+	 */
+	if (handle->cur > nr_meta_pages + 1) {
+		error = snapshot_write_next(handle);
+		if (error)
+			return error > 0 ? -ENODATA : error;
+	}
 	copy_last_highmem_page();
 	error = hibernate_restore_protect_page(handle->buffer);
 	/* Do that only if we have loaded the image entirely */
-- 
2.47.3

[PATCH v2 2/2] PM: hibernate: return -ENODATA if the snapshot image is not loaded

[Alberto Garcia]

snapshot_image_loaded() is used in both the in-kernel and the
userspace restore path to ensure that the snapshot image has been
completely loaded. However the latter path returns -EPERM in such
situations, which is meant for cases where the operation is neither
write-only nor ready.

This patch updates the check so the returned error code is -ENODATA in
both cases.

Suggested-by: Brian Geffon <bgeffon@google.com>
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 kernel/power/user.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/power/user.c b/kernel/power/user.c
index 4401cfe26e5c..be77f3556bd7 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -322,11 +322,14 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
 		error = snapshot_write_finalize(&data->handle);
 		if (error)
 			break;
-		if (data->mode != O_WRONLY || !data->frozen ||
-		    !snapshot_image_loaded(&data->handle)) {
+		if (data->mode != O_WRONLY || !data->frozen) {
 			error = -EPERM;
 			break;
 		}
+		if (!snapshot_image_loaded(&data->handle)) {
+			error = -ENODATA;
+			break;
+		}
 		error = hibernation_restore(data->platform_support);
 		break;
 
-- 
2.47.3

Re: [PATCH v2 1/2] PM: hibernate: Drain trailing zero pages on userspace restore

[Brian Geffon]

On Mon, Mar 9, 2026 at 1:40 PM Alberto Garcia <berto@igalia.com> wrote:
>
> Commit 005e8dddd497 ("PM: hibernate: don't store zero pages in the
> image file") added an optimization to skip zero-filled pages in the
> hibernation image. On restore, zero pages are handled internally by
> snapshot_write_next() in a loop that processes them without returning
> to the caller.
>
> With the userspace restore interface, writing the last non-zero page
> to /dev/snapshot is followed by the SNAPSHOT_ATOMIC_RESTORE ioctl. At
> this point there are no more calls to snapshot_write_next() so any
> trailing zero pages are not processed, snapshot_image_loaded() fails
> because handle->cur is smaller than expected, the ioctl returns -EPERM
> and the image is not restored.
>
> The in-kernel restore path is not affected by this because the loop in
> load_image() in swap.c calls snapshot_write_next() until it returns 0.
> It is this final call that drains any trailing zero pages.
>
> Fixed by calling snapshot_write_next() in snapshot_write_finalize(),
> giving the kernel the chance to drain any trailing zero pages.
>
> Fixes: 005e8dddd497 ("PM: hibernate: don't store zero pages in the image file")
> Signed-off-by: Alberto Garcia <berto@igalia.com>

Acked-by: Brian Geffon <bgeffon@google.com>

> ---
>  kernel/power/snapshot.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
> index 6e1321837c66..a564650734dc 100644
> --- a/kernel/power/snapshot.c
> +++ b/kernel/power/snapshot.c
> @@ -2855,6 +2855,17 @@ int snapshot_write_finalize(struct snapshot_handle *handle)
>  {
>         int error;
>
> +       /*
> +        * Call snapshot_write_next() to drain any trailing zero pages,
> +        * but make sure we're in the data page region first.
> +        * This function can return PAGE_SIZE if the kernel was expecting
> +        * another copy page. Return -ENODATA in that situation.
> +        */
> +       if (handle->cur > nr_meta_pages + 1) {
> +               error = snapshot_write_next(handle);
> +               if (error)
> +                       return error > 0 ? -ENODATA : error;
> +       }
>         copy_last_highmem_page();
>         error = hibernate_restore_protect_page(handle->buffer);
>         /* Do that only if we have loaded the image entirely */
> --
> 2.47.3
>

Re: [PATCH v2 2/2] PM: hibernate: return -ENODATA if the snapshot image is not loaded

[Brian Geffon]

On Mon, Mar 9, 2026 at 1:40 PM Alberto Garcia <berto@igalia.com> wrote:
>
> snapshot_image_loaded() is used in both the in-kernel and the
> userspace restore path to ensure that the snapshot image has been
> completely loaded. However the latter path returns -EPERM in such
> situations, which is meant for cases where the operation is neither
> write-only nor ready.
>
> This patch updates the check so the returned error code is -ENODATA in
> both cases.
>
> Suggested-by: Brian Geffon <bgeffon@google.com>
> Signed-off-by: Alberto Garcia <berto@igalia.com>

Acked-by: Brian Geffon <bgeffon@google.com>

> ---
>  kernel/power/user.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/power/user.c b/kernel/power/user.c
> index 4401cfe26e5c..be77f3556bd7 100644
> --- a/kernel/power/user.c
> +++ b/kernel/power/user.c
> @@ -322,11 +322,14 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
>                 error = snapshot_write_finalize(&data->handle);
>                 if (error)
>                         break;
> -               if (data->mode != O_WRONLY || !data->frozen ||
> -                   !snapshot_image_loaded(&data->handle)) {
> +               if (data->mode != O_WRONLY || !data->frozen) {
>                         error = -EPERM;
>                         break;
>                 }
> +               if (!snapshot_image_loaded(&data->handle)) {
> +                       error = -ENODATA;
> +                       break;
> +               }
>                 error = hibernation_restore(data->platform_support);
>                 break;
>
> --
> 2.47.3
>

Re: [PATCH v2 1/2] PM: hibernate: Drain trailing zero pages on userspace restore

[Rafael J. Wysocki]

On Mon, Mar 9, 2026 at 6:45 PM Brian Geffon <bgeffon@google.com> wrote:
>
> On Mon, Mar 9, 2026 at 1:40 PM Alberto Garcia <berto@igalia.com> wrote:
> >
> > Commit 005e8dddd497 ("PM: hibernate: don't store zero pages in the
> > image file") added an optimization to skip zero-filled pages in the
> > hibernation image. On restore, zero pages are handled internally by
> > snapshot_write_next() in a loop that processes them without returning
> > to the caller.
> >
> > With the userspace restore interface, writing the last non-zero page
> > to /dev/snapshot is followed by the SNAPSHOT_ATOMIC_RESTORE ioctl. At
> > this point there are no more calls to snapshot_write_next() so any
> > trailing zero pages are not processed, snapshot_image_loaded() fails
> > because handle->cur is smaller than expected, the ioctl returns -EPERM
> > and the image is not restored.
> >
> > The in-kernel restore path is not affected by this because the loop in
> > load_image() in swap.c calls snapshot_write_next() until it returns 0.
> > It is this final call that drains any trailing zero pages.
> >
> > Fixed by calling snapshot_write_next() in snapshot_write_finalize(),
> > giving the kernel the chance to drain any trailing zero pages.
> >
> > Fixes: 005e8dddd497 ("PM: hibernate: don't store zero pages in the image file")
> > Signed-off-by: Alberto Garcia <berto@igalia.com>
>
> Acked-by: Brian Geffon <bgeffon@google.com>

Applied as 7.0-rc material, thanks!

> > ---
> >  kernel/power/snapshot.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
> > index 6e1321837c66..a564650734dc 100644
> > --- a/kernel/power/snapshot.c
> > +++ b/kernel/power/snapshot.c
> > @@ -2855,6 +2855,17 @@ int snapshot_write_finalize(struct snapshot_handle *handle)
> >  {
> >         int error;
> >
> > +       /*
> > +        * Call snapshot_write_next() to drain any trailing zero pages,
> > +        * but make sure we're in the data page region first.
> > +        * This function can return PAGE_SIZE if the kernel was expecting
> > +        * another copy page. Return -ENODATA in that situation.
> > +        */
> > +       if (handle->cur > nr_meta_pages + 1) {
> > +               error = snapshot_write_next(handle);
> > +               if (error)
> > +                       return error > 0 ? -ENODATA : error;
> > +       }
> >         copy_last_highmem_page();
> >         error = hibernate_restore_protect_page(handle->buffer);
> >         /* Do that only if we have loaded the image entirely */
> > --
> > 2.47.3
> >

Re: [PATCH v2 2/2] PM: hibernate: return -ENODATA if the snapshot image is not loaded

[Rafael J. Wysocki]

On Mon, Mar 9, 2026 at 6:45 PM Brian Geffon <bgeffon@google.com> wrote:
>
> On Mon, Mar 9, 2026 at 1:40 PM Alberto Garcia <berto@igalia.com> wrote:
> >
> > snapshot_image_loaded() is used in both the in-kernel and the
> > userspace restore path to ensure that the snapshot image has been
> > completely loaded. However the latter path returns -EPERM in such
> > situations, which is meant for cases where the operation is neither
> > write-only nor ready.
> >
> > This patch updates the check so the returned error code is -ENODATA in
> > both cases.
> >
> > Suggested-by: Brian Geffon <bgeffon@google.com>
> > Signed-off-by: Alberto Garcia <berto@igalia.com>
>
> Acked-by: Brian Geffon <bgeffon@google.com>

Applied as 7.1 material, thanks!

> > ---
> >  kernel/power/user.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/power/user.c b/kernel/power/user.c
> > index 4401cfe26e5c..be77f3556bd7 100644
> > --- a/kernel/power/user.c
> > +++ b/kernel/power/user.c
> > @@ -322,11 +322,14 @@ static long snapshot_ioctl(struct file *filp, unsigned int cmd,
> >                 error = snapshot_write_finalize(&data->handle);
> >                 if (error)
> >                         break;
> > -               if (data->mode != O_WRONLY || !data->frozen ||
> > -                   !snapshot_image_loaded(&data->handle)) {
> > +               if (data->mode != O_WRONLY || !data->frozen) {
> >                         error = -EPERM;
> >                         break;
> >                 }
> > +               if (!snapshot_image_loaded(&data->handle)) {
> > +                       error = -ENODATA;
> > +                       break;
> > +               }
> >                 error = hibernation_restore(data->platform_support);
> >                 break;
> >
> > --
> > 2.47.3
> >