2018-11-19 10:18:42

by David Hildenbrand

[permalink] [raw]
Subject: [PATCH v1 8/8] PM / Hibernate: exclude all PageOffline() pages

The content of pages that are marked PG_offline is not of interest
(e.g. inflated by a balloon driver), let's skip these pages.

Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Len Brown <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Matthew Wilcox <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: "Michael S. Tsirkin" <[email protected]>
Acked-by: Pavel Machek <[email protected]>
Signed-off-by: David Hildenbrand <[email protected]>
---
kernel/power/snapshot.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index 87e6dd57819f..8d7b4d458842 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -1222,7 +1222,7 @@ static struct page *saveable_highmem_page(struct zone *zone, unsigned long pfn)
BUG_ON(!PageHighMem(page));

if (swsusp_page_is_forbidden(page) || swsusp_page_is_free(page) ||
- PageReserved(page))
+ PageReserved(page) || PageOffline(page))
return NULL;

if (page_is_guard(page))
@@ -1286,6 +1286,9 @@ static struct page *saveable_page(struct zone *zone, unsigned long pfn)
if (swsusp_page_is_forbidden(page) || swsusp_page_is_free(page))
return NULL;

+ if (PageOffline(page))
+ return NULL;
+
if (PageReserved(page)
&& (!kernel_page_present(page) || pfn_is_nosave(pfn)))
return NULL;
--
2.17.2



2018-11-20 22:12:08

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v1 8/8] PM / Hibernate: exclude all PageOffline() pages

On Monday, November 19, 2018 11:16:16 AM CET David Hildenbrand wrote:
> The content of pages that are marked PG_offline is not of interest
> (e.g. inflated by a balloon driver), let's skip these pages.
>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Cc: Pavel Machek <[email protected]>
> Cc: Len Brown <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Matthew Wilcox <[email protected]>
> Cc: Michal Hocko <[email protected]>
> Cc: "Michael S. Tsirkin" <[email protected]>
> Acked-by: Pavel Machek <[email protected]>
> Signed-off-by: David Hildenbrand <[email protected]>

Acked-by: Rafael J. Wysocki <[email protected]>

> ---
> kernel/power/snapshot.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
> index 87e6dd57819f..8d7b4d458842 100644
> --- a/kernel/power/snapshot.c
> +++ b/kernel/power/snapshot.c
> @@ -1222,7 +1222,7 @@ static struct page *saveable_highmem_page(struct zone *zone, unsigned long pfn)
> BUG_ON(!PageHighMem(page));
>
> if (swsusp_page_is_forbidden(page) || swsusp_page_is_free(page) ||
> - PageReserved(page))
> + PageReserved(page) || PageOffline(page))
> return NULL;
>
> if (page_is_guard(page))
> @@ -1286,6 +1286,9 @@ static struct page *saveable_page(struct zone *zone, unsigned long pfn)
> if (swsusp_page_is_forbidden(page) || swsusp_page_is_free(page))
> return NULL;
>
> + if (PageOffline(page))
> + return NULL;
> +
> if (PageReserved(page)
> && (!kernel_page_present(page) || pfn_is_nosave(pfn)))
> return NULL;
>



2018-11-21 12:22:51

by David Hildenbrand

[permalink] [raw]
Subject: Re: [PATCH v1 8/8] PM / Hibernate: exclude all PageOffline() pages

On 21.11.18 12:35, William Kucharski wrote:
> If you are adding PageOffline(page) to the condition list of the already existing if in
> saveable_highmem_page(), why explicitly add it as a separate statement in saveable_page()?
>
> It would seem more consistent to make the second check:
>
> - if (swsusp_page_is_forbidden(page) || swsusp_page_is_free(page))
> + if (swsusp_page_is_forbidden(page) || swsusp_page_is_free(page) ||
> + PageOffline(page))
>
> instead.
>
> It's admittedly a nit but it just seems cleaner to either do that or, if your intention
> was to separate the Page checks from the swsusp checks, to break the calls to
> PageReserved() and PageOffline() into their own check in saveable_highmem_page().

I'll split PageReserved() and PageOffline() off from the swsusp checks,
thanks for your comment!

>
> Thanks!
> -- Bill


--

Thanks,

David / dhildenb

2018-11-21 13:06:58

by William Kucharski

[permalink] [raw]
Subject: Re: [PATCH v1 8/8] PM / Hibernate: exclude all PageOffline() pages

If you are adding PageOffline(page) to the condition list of the already existing if in
saveable_highmem_page(), why explicitly add it as a separate statement in saveable_page()?

It would seem more consistent to make the second check:

- if (swsusp_page_is_forbidden(page) || swsusp_page_is_free(page))
+ if (swsusp_page_is_forbidden(page) || swsusp_page_is_free(page) ||
+ PageOffline(page))

instead.

It's admittedly a nit but it just seems cleaner to either do that or, if your intention
was to separate the Page checks from the swsusp checks, to break the calls to
PageReserved() and PageOffline() into their own check in saveable_highmem_page().

Thanks!
-- Bill


> On Nov 19, 2018, at 3:16 AM, David Hildenbrand <[email protected]> wrote:
>
> --- a/kernel/power/snapshot.c
> +++ b/kernel/power/snapshot.c
> @@ -1222,7 +1222,7 @@ static struct page *saveable_highmem_page(struct zone *zone, unsigned long pfn)
> BUG_ON(!PageHighMem(page));
>
> if (swsusp_page_is_forbidden(page) || swsusp_page_is_free(page) ||
> - PageReserved(page))
> + PageReserved(page) || PageOffline(page))
> return NULL;
>
> if (page_is_guard(page))
> @@ -1286,6 +1286,9 @@ static struct page *saveable_page(struct zone *zone, unsigned long pfn)
> if (swsusp_page_is_forbidden(page) || swsusp_page_is_free(page))
> return NULL;
>
> + if (PageOffline(page))
> + return NULL;
> +
> if (PageReserved(page)
> && (!kernel_page_present(page) || pfn_is_nosave(pfn)))
> return NULL;
> --
> 2.17.2
>