[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [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 <david@xxxxxxxxxx> 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
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.