[PATCH v1 1/8] mm: balloon: update comment about isolation/migration/compaction

[PATCH v1 1/8] mm: balloon: update comment about isolation/migration/compaction

[David Hildenbrand]

Commit b1123ea6d3b3 ("mm: balloon: use general non-lru movable page
feature") reworked balloon handling to make use of the general
non-lru movable page feature. The big comment block in
balloon_compaction.h contains quite some outdated information. Let's fix
this.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 include/linux/balloon_compaction.h | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/include/linux/balloon_compaction.h b/include/linux/balloon_compaction.h
index 53051f3d8f25..cbe50da5a59d 100644
--- a/include/linux/balloon_compaction.h
+++ b/include/linux/balloon_compaction.h
@@ -4,15 +4,18 @@
  *
  * Common interface definitions for making balloon pages movable by compaction.
  *
- * Despite being perfectly possible to perform ballooned pages migration, they
- * make a special corner case to compaction scans because balloon pages are not
- * enlisted at any LRU list like the other pages we do compact / migrate.
+ * Balloon page migration makes use of the general non-lru movable page
+ * feature.
+ *
+ * page->private is used to reference the responsible balloon device.
+ * page->mapping is used in context of non-lru page migration to reference
+ * the address space operations for page isolation/migration/compaction.
  *
  * As the page isolation scanning step a compaction thread does is a lockless
  * procedure (from a page standpoint), it might bring some racy situations while
  * performing balloon page compaction. In order to sort out these racy scenarios
  * and safely perform balloon's page compaction and migration we must, always,
- * ensure following these three simple rules:
+ * ensure following these simple rules:
  *
  *   i. when updating a balloon's page ->mapping element, strictly do it under
  *      the following lock order, independently of the far superior
@@ -21,19 +24,8 @@
  *	      +--spin_lock_irq(&b_dev_info->pages_lock);
  *	            ... page->mapping updates here ...
  *
- *  ii. before isolating or dequeueing a balloon page from the balloon device
- *      pages list, the page reference counter must be raised by one and the
- *      extra refcount must be dropped when the page is enqueued back into
- *      the balloon device page list, thus a balloon page keeps its reference
- *      counter raised only while it is under our special handling;
- *
- * iii. after the lockless scan step have selected a potential balloon page for
- *      isolation, re-test the PageBalloon mark and the PagePrivate flag
- *      under the proper page lock, to ensure isolating a valid balloon page
- *      (not yet isolated, nor under release procedure)
- *
- *  iv. isolation or dequeueing procedure must clear PagePrivate flag under
- *      page lock together with removing page from balloon device page list.
+ *  ii. isolation or dequeueing procedure must remove the page from balloon
+ *      device page list under b_dev_info->pages_lock.
  *
  * The functions provided by this interface are placed to help on coping with
  * the aforementioned balloon page corner case, as well as to ensure the simple
-- 
2.17.2

Re: [PATCH v1 1/8] mm: balloon: update comment about isolation/migration/compaction

[Michael S. Tsirkin]

On Mon, Nov 19, 2018 at 11:16:09AM +0100, David Hildenbrand wrote:
> Commit b1123ea6d3b3 ("mm: balloon: use general non-lru movable page
> feature") reworked balloon handling to make use of the general
> non-lru movable page feature. The big comment block in
> balloon_compaction.h contains quite some outdated information. Let's fix
> this.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  include/linux/balloon_compaction.h | 26 +++++++++-----------------
>  1 file changed, 9 insertions(+), 17 deletions(-)
> 
> diff --git a/include/linux/balloon_compaction.h b/include/linux/balloon_compaction.h
> index 53051f3d8f25..cbe50da5a59d 100644
> --- a/include/linux/balloon_compaction.h
> +++ b/include/linux/balloon_compaction.h
> @@ -4,15 +4,18 @@
>   *
>   * Common interface definitions for making balloon pages movable by compaction.
>   *
> - * Despite being perfectly possible to perform ballooned pages migration, they
> - * make a special corner case to compaction scans because balloon pages are not
> - * enlisted at any LRU list like the other pages we do compact / migrate.
> + * Balloon page migration makes use of the general non-lru movable page
> + * feature.
> + *
> + * page->private is used to reference the responsible balloon device.
> + * page->mapping is used in context of non-lru page migration to reference
> + * the address space operations for page isolation/migration/compaction.
>   *
>   * As the page isolation scanning step a compaction thread does is a lockless
>   * procedure (from a page standpoint), it might bring some racy situations while
>   * performing balloon page compaction. In order to sort out these racy scenarios
>   * and safely perform balloon's page compaction and migration we must, always,
> - * ensure following these three simple rules:
> + * ensure following these simple rules:
>   *
>   *   i. when updating a balloon's page ->mapping element, strictly do it under
>   *      the following lock order, independently of the far superior
> @@ -21,19 +24,8 @@
>   *	      +--spin_lock_irq(&b_dev_info->pages_lock);
>   *	            ... page->mapping updates here ...
>   *
> - *  ii. before isolating or dequeueing a balloon page from the balloon device
> - *      pages list, the page reference counter must be raised by one and the
> - *      extra refcount must be dropped when the page is enqueued back into
> - *      the balloon device page list, thus a balloon page keeps its reference
> - *      counter raised only while it is under our special handling;
> - *
> - * iii. after the lockless scan step have selected a potential balloon page for
> - *      isolation, re-test the PageBalloon mark and the PagePrivate flag
> - *      under the proper page lock, to ensure isolating a valid balloon page
> - *      (not yet isolated, nor under release procedure)
> - *
> - *  iv. isolation or dequeueing procedure must clear PagePrivate flag under
> - *      page lock together with removing page from balloon device page list.
> + *  ii. isolation or dequeueing procedure must remove the page from balloon
> + *      device page list under b_dev_info->pages_lock.
>   *
>   * The functions provided by this interface are placed to help on coping with
>   * the aforementioned balloon page corner case, as well as to ensure the simple
> -- 
> 2.17.2

[PATCH v1 2/8] mm: convert PG_balloon to PG_offline

[David Hildenbrand]

PG_balloon was introduced to implement page migration/compaction for pages
inflated in virtio-balloon. Nowadays, it is only a marker that a page is
part of virtio-balloon and therefore logically offline.

We also want to make use of this flag in other balloon drivers - for
inflated pages or when onlining a section but keeping some pages offline
(e.g. used right now by XEN and Hyper-V via set_online_page_callback()).

We are going to expose this flag to dump tools like makedumpfile. But
instead of exposing PG_balloon, let's generalize the concept of marking
pages as logically offline, so it can be reused for other purposes
later on.

Rename PG_balloon to PG_offline. This is an indicator that the page is
logically offline, the content stale and that it should not be touched
(e.g. a hypervisor would have to allocate backing storage in order for the
guest to dump an unused page).  We can then e.g. exclude such pages from
dumps.

We replace and reuse KPF_BALLOON (23), as this shouldn't really harm
(and for now the semantics stay the same).  In following patches, we will
make use of this bit also in other balloon drivers. While at it, document
PGTABLE.

Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Mike Rapoport <rppt@linux.vnet.ibm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christian Hansen <chansen3@cisco.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
Cc: Alexander Duyck <alexander.h.duyck@linux.intel.com>
Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Cc: Miles Chen <miles.chen@mediatek.com>
Cc: David Rientjes <rientjes@google.com>
Cc: Konstantin Khlebnikov <koct9i@gmail.com>
Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 Documentation/admin-guide/mm/pagemap.rst |  9 ++++++---
 fs/proc/page.c                           |  4 ++--
 include/linux/balloon_compaction.h       |  8 ++++----
 include/linux/page-flags.h               | 11 +++++++----
 include/uapi/linux/kernel-page-flags.h   |  2 +-
 tools/vm/page-types.c                    |  2 +-
 6 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/Documentation/admin-guide/mm/pagemap.rst b/Documentation/admin-guide/mm/pagemap.rst
index 3f7bade2c231..340a5aee9b80 100644
--- a/Documentation/admin-guide/mm/pagemap.rst
+++ b/Documentation/admin-guide/mm/pagemap.rst
@@ -75,9 +75,10 @@ number of times a page is mapped.
     20. NOPAGE
     21. KSM
     22. THP
-    23. BALLOON
+    23. OFFLINE
     24. ZERO_PAGE
     25. IDLE
+    26. PGTABLE
 
  * ``/proc/kpagecgroup``.  This file contains a 64-bit inode number of the
    memory cgroup each page is charged to, indexed by PFN. Only available when
@@ -118,8 +119,8 @@ Short descriptions to the page flags
     identical memory pages dynamically shared between one or more processes
 22 - THP
     contiguous pages which construct transparent hugepages
-23 - BALLOON
-    balloon compaction page
+23 - OFFLINE
+    page is logically offline
 24 - ZERO_PAGE
     zero page for pfn_zero or huge_zero page
 25 - IDLE
@@ -128,6 +129,8 @@ Short descriptions to the page flags
     Note that this flag may be stale in case the page was accessed via
     a PTE. To make sure the flag is up-to-date one has to read
     ``/sys/kernel/mm/page_idle/bitmap`` first.
+26 - PGTABLE
+    page is in use as a page table
 
 IO related page flags
 ---------------------
diff --git a/fs/proc/page.c b/fs/proc/page.c
index 6c517b11acf8..378401af4d9d 100644
--- a/fs/proc/page.c
+++ b/fs/proc/page.c
@@ -152,8 +152,8 @@ u64 stable_page_flags(struct page *page)
 	else if (page_count(page) == 0 && is_free_buddy_page(page))
 		u |= 1 << KPF_BUDDY;
 
-	if (PageBalloon(page))
-		u |= 1 << KPF_BALLOON;
+	if (PageOffline(page))
+		u |= 1 << KPF_OFFLINE;
 	if (PageTable(page))
 		u |= 1 << KPF_PGTABLE;
 
diff --git a/include/linux/balloon_compaction.h b/include/linux/balloon_compaction.h
index cbe50da5a59d..f111c780ef1d 100644
--- a/include/linux/balloon_compaction.h
+++ b/include/linux/balloon_compaction.h
@@ -95,7 +95,7 @@ extern int balloon_page_migrate(struct address_space *mapping,
 static inline void balloon_page_insert(struct balloon_dev_info *balloon,
 				       struct page *page)
 {
-	__SetPageBalloon(page);
+	__SetPageOffline(page);
 	__SetPageMovable(page, balloon->inode->i_mapping);
 	set_page_private(page, (unsigned long)balloon);
 	list_add(&page->lru, &balloon->pages);
@@ -111,7 +111,7 @@ static inline void balloon_page_insert(struct balloon_dev_info *balloon,
  */
 static inline void balloon_page_delete(struct page *page)
 {
-	__ClearPageBalloon(page);
+	__ClearPageOffline(page);
 	__ClearPageMovable(page);
 	set_page_private(page, 0);
 	/*
@@ -141,13 +141,13 @@ static inline gfp_t balloon_mapping_gfp_mask(void)
 static inline void balloon_page_insert(struct balloon_dev_info *balloon,
 				       struct page *page)
 {
-	__SetPageBalloon(page);
+	__SetPageOffline(page);
 	list_add(&page->lru, &balloon->pages);
 }
 
 static inline void balloon_page_delete(struct page *page)
 {
-	__ClearPageBalloon(page);
+	__ClearPageOffline(page);
 	list_del(&page->lru);
 }
 
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 50ce1bddaf56..f91da3d0a67e 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -670,7 +670,7 @@ PAGEFLAG_FALSE(DoubleMap)
 #define PAGE_TYPE_BASE	0xf0000000
 /* Reserve		0x0000007f to catch underflows of page_mapcount */
 #define PG_buddy	0x00000080
-#define PG_balloon	0x00000100
+#define PG_offline	0x00000100
 #define PG_kmemcg	0x00000200
 #define PG_table	0x00000400
 
@@ -700,10 +700,13 @@ static __always_inline void __ClearPage##uname(struct page *page)	\
 PAGE_TYPE_OPS(Buddy, buddy)
 
 /*
- * PageBalloon() is true for pages that are on the balloon page list
- * (see mm/balloon_compaction.c).
+ * PageOffline() indicates that the pages is logically offline although the
+ * containing section is online. (e.g. inflated in a balloon driver or
+ * not onlined when onlining the section).
+ * The content of these pages is effectively stale. Such pages should not
+ * be touched (read/write/dump/save) except by their owner.
  */
-PAGE_TYPE_OPS(Balloon, balloon)
+PAGE_TYPE_OPS(Offline, offline)
 
 /*
  * If kmemcg is enabled, the buddy allocator will set PageKmemcg() on
diff --git a/include/uapi/linux/kernel-page-flags.h b/include/uapi/linux/kernel-page-flags.h
index 21b9113c69da..6f2f2720f3ac 100644
--- a/include/uapi/linux/kernel-page-flags.h
+++ b/include/uapi/linux/kernel-page-flags.h
@@ -32,7 +32,7 @@
 
 #define KPF_KSM			21
 #define KPF_THP			22
-#define KPF_BALLOON		23
+#define KPF_OFFLINE		23
 #define KPF_ZERO_PAGE		24
 #define KPF_IDLE		25
 #define KPF_PGTABLE		26
diff --git a/tools/vm/page-types.c b/tools/vm/page-types.c
index 37908a83ddc2..6c38d3b862e4 100644
--- a/tools/vm/page-types.c
+++ b/tools/vm/page-types.c
@@ -133,7 +133,7 @@ static const char * const page_flag_names[] = {
 	[KPF_NOPAGE]		= "n:nopage",
 	[KPF_KSM]		= "x:ksm",
 	[KPF_THP]		= "t:thp",
-	[KPF_BALLOON]		= "o:balloon",
+	[KPF_OFFLINE]		= "o:offline",
 	[KPF_PGTABLE]		= "g:pgtable",
 	[KPF_ZERO_PAGE]		= "z:zero_page",
 	[KPF_IDLE]              = "i:idle_page",
-- 
2.17.2

Re: [PATCH v1 2/8] mm: convert PG_balloon to PG_offline

[Konstantin Khlebnikov]

On Mon, Nov 19, 2018 at 1:16 PM David Hildenbrand <david@redhat.com> wrote:
>
> PG_balloon was introduced to implement page migration/compaction for pages
> inflated in virtio-balloon. Nowadays, it is only a marker that a page is
> part of virtio-balloon and therefore logically offline.
>
> We also want to make use of this flag in other balloon drivers - for
> inflated pages or when onlining a section but keeping some pages offline
> (e.g. used right now by XEN and Hyper-V via set_online_page_callback()).
>
> We are going to expose this flag to dump tools like makedumpfile. But
> instead of exposing PG_balloon, let's generalize the concept of marking
> pages as logically offline, so it can be reused for other purposes
> later on.
>
> Rename PG_balloon to PG_offline. This is an indicator that the page is
> logically offline, the content stale and that it should not be touched
> (e.g. a hypervisor would have to allocate backing storage in order for the
> guest to dump an unused page).  We can then e.g. exclude such pages from
> dumps.
>
> We replace and reuse KPF_BALLOON (23), as this shouldn't really harm
> (and for now the semantics stay the same).  In following patches, we will
> make use of this bit also in other balloon drivers. While at it, document
> PGTABLE.

Ok, makes sense.

Acked-by: Konstantin Khlebnikov <koct9i@gmail.com>

>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Alexey Dobriyan <adobriyan@gmail.com>
> Cc: Mike Rapoport <rppt@linux.vnet.ibm.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Christian Hansen <chansen3@cisco.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
> Cc: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Cc: Miles Chen <miles.chen@mediatek.com>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Konstantin Khlebnikov <koct9i@gmail.com>
> Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  Documentation/admin-guide/mm/pagemap.rst |  9 ++++++---
>  fs/proc/page.c                           |  4 ++--
>  include/linux/balloon_compaction.h       |  8 ++++----
>  include/linux/page-flags.h               | 11 +++++++----
>  include/uapi/linux/kernel-page-flags.h   |  2 +-
>  tools/vm/page-types.c                    |  2 +-
>  6 files changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/admin-guide/mm/pagemap.rst b/Documentation/admin-guide/mm/pagemap.rst
> index 3f7bade2c231..340a5aee9b80 100644
> --- a/Documentation/admin-guide/mm/pagemap.rst
> +++ b/Documentation/admin-guide/mm/pagemap.rst
> @@ -75,9 +75,10 @@ number of times a page is mapped.
>      20. NOPAGE
>      21. KSM
>      22. THP
> -    23. BALLOON
> +    23. OFFLINE
>      24. ZERO_PAGE
>      25. IDLE
> +    26. PGTABLE
>
>   * ``/proc/kpagecgroup``.  This file contains a 64-bit inode number of the
>     memory cgroup each page is charged to, indexed by PFN. Only available when
> @@ -118,8 +119,8 @@ Short descriptions to the page flags
>      identical memory pages dynamically shared between one or more processes
>  22 - THP
>      contiguous pages which construct transparent hugepages
> -23 - BALLOON
> -    balloon compaction page
> +23 - OFFLINE
> +    page is logically offline
>  24 - ZERO_PAGE
>      zero page for pfn_zero or huge_zero page
>  25 - IDLE
> @@ -128,6 +129,8 @@ Short descriptions to the page flags
>      Note that this flag may be stale in case the page was accessed via
>      a PTE. To make sure the flag is up-to-date one has to read
>      ``/sys/kernel/mm/page_idle/bitmap`` first.
> +26 - PGTABLE
> +    page is in use as a page table
>
>  IO related page flags
>  ---------------------
> diff --git a/fs/proc/page.c b/fs/proc/page.c
> index 6c517b11acf8..378401af4d9d 100644
> --- a/fs/proc/page.c
> +++ b/fs/proc/page.c
> @@ -152,8 +152,8 @@ u64 stable_page_flags(struct page *page)
>         else if (page_count(page) == 0 && is_free_buddy_page(page))
>                 u |= 1 << KPF_BUDDY;
>
> -       if (PageBalloon(page))
> -               u |= 1 << KPF_BALLOON;
> +       if (PageOffline(page))
> +               u |= 1 << KPF_OFFLINE;
>         if (PageTable(page))
>                 u |= 1 << KPF_PGTABLE;
>
> diff --git a/include/linux/balloon_compaction.h b/include/linux/balloon_compaction.h
> index cbe50da5a59d..f111c780ef1d 100644
> --- a/include/linux/balloon_compaction.h
> +++ b/include/linux/balloon_compaction.h
> @@ -95,7 +95,7 @@ extern int balloon_page_migrate(struct address_space *mapping,
>  static inline void balloon_page_insert(struct balloon_dev_info *balloon,
>                                        struct page *page)
>  {
> -       __SetPageBalloon(page);
> +       __SetPageOffline(page);
>         __SetPageMovable(page, balloon->inode->i_mapping);
>         set_page_private(page, (unsigned long)balloon);
>         list_add(&page->lru, &balloon->pages);
> @@ -111,7 +111,7 @@ static inline void balloon_page_insert(struct balloon_dev_info *balloon,
>   */
>  static inline void balloon_page_delete(struct page *page)
>  {
> -       __ClearPageBalloon(page);
> +       __ClearPageOffline(page);
>         __ClearPageMovable(page);
>         set_page_private(page, 0);
>         /*
> @@ -141,13 +141,13 @@ static inline gfp_t balloon_mapping_gfp_mask(void)
>  static inline void balloon_page_insert(struct balloon_dev_info *balloon,
>                                        struct page *page)
>  {
> -       __SetPageBalloon(page);
> +       __SetPageOffline(page);
>         list_add(&page->lru, &balloon->pages);
>  }
>
>  static inline void balloon_page_delete(struct page *page)
>  {
> -       __ClearPageBalloon(page);
> +       __ClearPageOffline(page);
>         list_del(&page->lru);
>  }
>
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 50ce1bddaf56..f91da3d0a67e 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -670,7 +670,7 @@ PAGEFLAG_FALSE(DoubleMap)
>  #define PAGE_TYPE_BASE 0xf0000000
>  /* Reserve             0x0000007f to catch underflows of page_mapcount */
>  #define PG_buddy       0x00000080
> -#define PG_balloon     0x00000100
> +#define PG_offline     0x00000100
>  #define PG_kmemcg      0x00000200
>  #define PG_table       0x00000400
>
> @@ -700,10 +700,13 @@ static __always_inline void __ClearPage##uname(struct page *page) \
>  PAGE_TYPE_OPS(Buddy, buddy)
>
>  /*
> - * PageBalloon() is true for pages that are on the balloon page list
> - * (see mm/balloon_compaction.c).
> + * PageOffline() indicates that the pages is logically offline although the
> + * containing section is online. (e.g. inflated in a balloon driver or
> + * not onlined when onlining the section).
> + * The content of these pages is effectively stale. Such pages should not
> + * be touched (read/write/dump/save) except by their owner.
>   */
> -PAGE_TYPE_OPS(Balloon, balloon)
> +PAGE_TYPE_OPS(Offline, offline)
>
>  /*
>   * If kmemcg is enabled, the buddy allocator will set PageKmemcg() on
> diff --git a/include/uapi/linux/kernel-page-flags.h b/include/uapi/linux/kernel-page-flags.h
> index 21b9113c69da..6f2f2720f3ac 100644
> --- a/include/uapi/linux/kernel-page-flags.h
> +++ b/include/uapi/linux/kernel-page-flags.h
> @@ -32,7 +32,7 @@
>
>  #define KPF_KSM                        21
>  #define KPF_THP                        22
> -#define KPF_BALLOON            23
> +#define KPF_OFFLINE            23
>  #define KPF_ZERO_PAGE          24
>  #define KPF_IDLE               25
>  #define KPF_PGTABLE            26
> diff --git a/tools/vm/page-types.c b/tools/vm/page-types.c
> index 37908a83ddc2..6c38d3b862e4 100644
> --- a/tools/vm/page-types.c
> +++ b/tools/vm/page-types.c
> @@ -133,7 +133,7 @@ static const char * const page_flag_names[] = {
>         [KPF_NOPAGE]            = "n:nopage",
>         [KPF_KSM]               = "x:ksm",
>         [KPF_THP]               = "t:thp",
> -       [KPF_BALLOON]           = "o:balloon",
> +       [KPF_OFFLINE]           = "o:offline",

Even 'o' keyword is better suits for Offline than for ballOon =)

>         [KPF_PGTABLE]           = "g:pgtable",
>         [KPF_ZERO_PAGE]         = "z:zero_page",
>         [KPF_IDLE]              = "i:idle_page",
> --
> 2.17.2
>

Re: [PATCH v1 2/8] mm: convert PG_balloon to PG_offline

[David Hildenbrand]

>  
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 50ce1bddaf56..f91da3d0a67e 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -670,7 +670,7 @@ PAGEFLAG_FALSE(DoubleMap)
>  #define PAGE_TYPE_BASE	0xf0000000
>  /* Reserve		0x0000007f to catch underflows of page_mapcount */
>  #define PG_buddy	0x00000080
> -#define PG_balloon	0x00000100
> +#define PG_offline	0x00000100
>  #define PG_kmemcg	0x00000200
>  #define PG_table	0x00000400
>  
> @@ -700,10 +700,13 @@ static __always_inline void __ClearPage##uname(struct page *page)	\
>  PAGE_TYPE_OPS(Buddy, buddy)
>  
>  /*
> - * PageBalloon() is true for pages that are on the balloon page list
> - * (see mm/balloon_compaction.c).
> + * PageOffline() indicates that the pages is logically offline although the

s/pages/page/

:)


-- 

Thanks,

David / dhildenb

Re: [PATCH v1 2/8] mm: convert PG_balloon to PG_offline

[Michael S. Tsirkin]

On Mon, Nov 19, 2018 at 11:16:10AM +0100, David Hildenbrand wrote:
> PG_balloon was introduced to implement page migration/compaction for pages
> inflated in virtio-balloon. Nowadays, it is only a marker that a page is
> part of virtio-balloon and therefore logically offline.
> 
> We also want to make use of this flag in other balloon drivers - for
> inflated pages or when onlining a section but keeping some pages offline
> (e.g. used right now by XEN and Hyper-V via set_online_page_callback()).
> 
> We are going to expose this flag to dump tools like makedumpfile. But
> instead of exposing PG_balloon, let's generalize the concept of marking
> pages as logically offline, so it can be reused for other purposes
> later on.
> 
> Rename PG_balloon to PG_offline. This is an indicator that the page is
> logically offline, the content stale and that it should not be touched
> (e.g. a hypervisor would have to allocate backing storage in order for the
> guest to dump an unused page).  We can then e.g. exclude such pages from
> dumps.
> 
> We replace and reuse KPF_BALLOON (23), as this shouldn't really harm
> (and for now the semantics stay the same).  In following patches, we will
> make use of this bit also in other balloon drivers. While at it, document
> PGTABLE.
> 
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Alexey Dobriyan <adobriyan@gmail.com>
> Cc: Mike Rapoport <rppt@linux.vnet.ibm.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Christian Hansen <chansen3@cisco.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
> Cc: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Cc: Miles Chen <miles.chen@mediatek.com>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Konstantin Khlebnikov <koct9i@gmail.com>
> Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  Documentation/admin-guide/mm/pagemap.rst |  9 ++++++---
>  fs/proc/page.c                           |  4 ++--
>  include/linux/balloon_compaction.h       |  8 ++++----
>  include/linux/page-flags.h               | 11 +++++++----
>  include/uapi/linux/kernel-page-flags.h   |  2 +-
>  tools/vm/page-types.c                    |  2 +-
>  6 files changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/admin-guide/mm/pagemap.rst b/Documentation/admin-guide/mm/pagemap.rst
> index 3f7bade2c231..340a5aee9b80 100644
> --- a/Documentation/admin-guide/mm/pagemap.rst
> +++ b/Documentation/admin-guide/mm/pagemap.rst
> @@ -75,9 +75,10 @@ number of times a page is mapped.
>      20. NOPAGE
>      21. KSM
>      22. THP
> -    23. BALLOON
> +    23. OFFLINE
>      24. ZERO_PAGE
>      25. IDLE
> +    26. PGTABLE
>  
>   * ``/proc/kpagecgroup``.  This file contains a 64-bit inode number of the
>     memory cgroup each page is charged to, indexed by PFN. Only available when
> @@ -118,8 +119,8 @@ Short descriptions to the page flags
>      identical memory pages dynamically shared between one or more processes
>  22 - THP
>      contiguous pages which construct transparent hugepages
> -23 - BALLOON
> -    balloon compaction page
> +23 - OFFLINE
> +    page is logically offline
>  24 - ZERO_PAGE
>      zero page for pfn_zero or huge_zero page
>  25 - IDLE
> @@ -128,6 +129,8 @@ Short descriptions to the page flags
>      Note that this flag may be stale in case the page was accessed via
>      a PTE. To make sure the flag is up-to-date one has to read
>      ``/sys/kernel/mm/page_idle/bitmap`` first.
> +26 - PGTABLE
> +    page is in use as a page table
>  
>  IO related page flags
>  ---------------------
> diff --git a/fs/proc/page.c b/fs/proc/page.c
> index 6c517b11acf8..378401af4d9d 100644
> --- a/fs/proc/page.c
> +++ b/fs/proc/page.c
> @@ -152,8 +152,8 @@ u64 stable_page_flags(struct page *page)
>  	else if (page_count(page) == 0 && is_free_buddy_page(page))
>  		u |= 1 << KPF_BUDDY;
>  
> -	if (PageBalloon(page))
> -		u |= 1 << KPF_BALLOON;
> +	if (PageOffline(page))
> +		u |= 1 << KPF_OFFLINE;
>  	if (PageTable(page))
>  		u |= 1 << KPF_PGTABLE;
>  
> diff --git a/include/linux/balloon_compaction.h b/include/linux/balloon_compaction.h
> index cbe50da5a59d..f111c780ef1d 100644
> --- a/include/linux/balloon_compaction.h
> +++ b/include/linux/balloon_compaction.h
> @@ -95,7 +95,7 @@ extern int balloon_page_migrate(struct address_space *mapping,
>  static inline void balloon_page_insert(struct balloon_dev_info *balloon,
>  				       struct page *page)
>  {
> -	__SetPageBalloon(page);
> +	__SetPageOffline(page);
>  	__SetPageMovable(page, balloon->inode->i_mapping);
>  	set_page_private(page, (unsigned long)balloon);
>  	list_add(&page->lru, &balloon->pages);
> @@ -111,7 +111,7 @@ static inline void balloon_page_insert(struct balloon_dev_info *balloon,
>   */
>  static inline void balloon_page_delete(struct page *page)
>  {
> -	__ClearPageBalloon(page);
> +	__ClearPageOffline(page);
>  	__ClearPageMovable(page);
>  	set_page_private(page, 0);
>  	/*
> @@ -141,13 +141,13 @@ static inline gfp_t balloon_mapping_gfp_mask(void)
>  static inline void balloon_page_insert(struct balloon_dev_info *balloon,
>  				       struct page *page)
>  {
> -	__SetPageBalloon(page);
> +	__SetPageOffline(page);
>  	list_add(&page->lru, &balloon->pages);
>  }
>  
>  static inline void balloon_page_delete(struct page *page)
>  {
> -	__ClearPageBalloon(page);
> +	__ClearPageOffline(page);
>  	list_del(&page->lru);
>  }
>  
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 50ce1bddaf56..f91da3d0a67e 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -670,7 +670,7 @@ PAGEFLAG_FALSE(DoubleMap)
>  #define PAGE_TYPE_BASE	0xf0000000
>  /* Reserve		0x0000007f to catch underflows of page_mapcount */
>  #define PG_buddy	0x00000080
> -#define PG_balloon	0x00000100
> +#define PG_offline	0x00000100
>  #define PG_kmemcg	0x00000200
>  #define PG_table	0x00000400
>  
> @@ -700,10 +700,13 @@ static __always_inline void __ClearPage##uname(struct page *page)	\
>  PAGE_TYPE_OPS(Buddy, buddy)
>  
>  /*
> - * PageBalloon() is true for pages that are on the balloon page list
> - * (see mm/balloon_compaction.c).
> + * PageOffline() indicates that the pages is logically offline although the

with this typo fixed ..

> + * containing section is online. (e.g. inflated in a balloon driver or
> + * not onlined when onlining the section).
> + * The content of these pages is effectively stale. Such pages should not
> + * be touched (read/write/dump/save) except by their owner.
>   */
> -PAGE_TYPE_OPS(Balloon, balloon)
> +PAGE_TYPE_OPS(Offline, offline)
>  
>  /*
>   * If kmemcg is enabled, the buddy allocator will set PageKmemcg() on
> diff --git a/include/uapi/linux/kernel-page-flags.h b/include/uapi/linux/kernel-page-flags.h
> index 21b9113c69da..6f2f2720f3ac 100644
> --- a/include/uapi/linux/kernel-page-flags.h
> +++ b/include/uapi/linux/kernel-page-flags.h
> @@ -32,7 +32,7 @@
>  
>  #define KPF_KSM			21
>  #define KPF_THP			22
> -#define KPF_BALLOON		23
> +#define KPF_OFFLINE		23
>  #define KPF_ZERO_PAGE		24
>  #define KPF_IDLE		25
>  #define KPF_PGTABLE		26
> diff --git a/tools/vm/page-types.c b/tools/vm/page-types.c
> index 37908a83ddc2..6c38d3b862e4 100644
> --- a/tools/vm/page-types.c
> +++ b/tools/vm/page-types.c
> @@ -133,7 +133,7 @@ static const char * const page_flag_names[] = {
>  	[KPF_NOPAGE]		= "n:nopage",
>  	[KPF_KSM]		= "x:ksm",
>  	[KPF_THP]		= "t:thp",
> -	[KPF_BALLOON]		= "o:balloon",
> +	[KPF_OFFLINE]		= "o:offline",
>  	[KPF_PGTABLE]		= "g:pgtable",
>  	[KPF_ZERO_PAGE]		= "z:zero_page",
>  	[KPF_IDLE]              = "i:idle_page",

for balloon changes:

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> -- 
> 2.17.2

Re: [PATCH v1 2/8] mm: convert PG_balloon to PG_offline

[Pankaj Gupta]

> 
> PG_balloon was introduced to implement page migration/compaction for pages
> inflated in virtio-balloon. Nowadays, it is only a marker that a page is
> part of virtio-balloon and therefore logically offline.
> 
> We also want to make use of this flag in other balloon drivers - for
> inflated pages or when onlining a section but keeping some pages offline
> (e.g. used right now by XEN and Hyper-V via set_online_page_callback()).
> 
> We are going to expose this flag to dump tools like makedumpfile. But
> instead of exposing PG_balloon, let's generalize the concept of marking
> pages as logically offline, so it can be reused for other purposes
> later on.
> 
> Rename PG_balloon to PG_offline. This is an indicator that the page is
> logically offline, the content stale and that it should not be touched
> (e.g. a hypervisor would have to allocate backing storage in order for the
> guest to dump an unused page).  We can then e.g. exclude such pages from
> dumps.
> 
> We replace and reuse KPF_BALLOON (23), as this shouldn't really harm
> (and for now the semantics stay the same).  In following patches, we will
> make use of this bit also in other balloon drivers. While at it, document
> PGTABLE.
> 
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Alexey Dobriyan <adobriyan@gmail.com>
> Cc: Mike Rapoport <rppt@linux.vnet.ibm.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Christian Hansen <chansen3@cisco.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Pavel Tatashin <pasha.tatashin@oracle.com>
> Cc: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> Cc: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Cc: Miles Chen <miles.chen@mediatek.com>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Konstantin Khlebnikov <koct9i@gmail.com>
> Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  Documentation/admin-guide/mm/pagemap.rst |  9 ++++++---
>  fs/proc/page.c                           |  4 ++--
>  include/linux/balloon_compaction.h       |  8 ++++----
>  include/linux/page-flags.h               | 11 +++++++----
>  include/uapi/linux/kernel-page-flags.h   |  2 +-
>  tools/vm/page-types.c                    |  2 +-
>  6 files changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/admin-guide/mm/pagemap.rst
> b/Documentation/admin-guide/mm/pagemap.rst
> index 3f7bade2c231..340a5aee9b80 100644
> --- a/Documentation/admin-guide/mm/pagemap.rst
> +++ b/Documentation/admin-guide/mm/pagemap.rst
> @@ -75,9 +75,10 @@ number of times a page is mapped.
>      20. NOPAGE
>      21. KSM
>      22. THP
> -    23. BALLOON
> +    23. OFFLINE
>      24. ZERO_PAGE
>      25. IDLE
> +    26. PGTABLE
>  
>   * ``/proc/kpagecgroup``.  This file contains a 64-bit inode number of the
>     memory cgroup each page is charged to, indexed by PFN. Only available
>     when
> @@ -118,8 +119,8 @@ Short descriptions to the page flags
>      identical memory pages dynamically shared between one or more processes
>  22 - THP
>      contiguous pages which construct transparent hugepages
> -23 - BALLOON
> -    balloon compaction page
> +23 - OFFLINE
> +    page is logically offline
>  24 - ZERO_PAGE
>      zero page for pfn_zero or huge_zero page
>  25 - IDLE
> @@ -128,6 +129,8 @@ Short descriptions to the page flags
>      Note that this flag may be stale in case the page was accessed via
>      a PTE. To make sure the flag is up-to-date one has to read
>      ``/sys/kernel/mm/page_idle/bitmap`` first.
> +26 - PGTABLE
> +    page is in use as a page table
>  
>  IO related page flags
>  ---------------------
> diff --git a/fs/proc/page.c b/fs/proc/page.c
> index 6c517b11acf8..378401af4d9d 100644
> --- a/fs/proc/page.c
> +++ b/fs/proc/page.c
> @@ -152,8 +152,8 @@ u64 stable_page_flags(struct page *page)
>  	else if (page_count(page) == 0 && is_free_buddy_page(page))
>  		u |= 1 << KPF_BUDDY;
>  
> -	if (PageBalloon(page))
> -		u |= 1 << KPF_BALLOON;
> +	if (PageOffline(page))
> +		u |= 1 << KPF_OFFLINE;
>  	if (PageTable(page))
>  		u |= 1 << KPF_PGTABLE;
>  
> diff --git a/include/linux/balloon_compaction.h
> b/include/linux/balloon_compaction.h
> index cbe50da5a59d..f111c780ef1d 100644
> --- a/include/linux/balloon_compaction.h
> +++ b/include/linux/balloon_compaction.h
> @@ -95,7 +95,7 @@ extern int balloon_page_migrate(struct address_space
> *mapping,
>  static inline void balloon_page_insert(struct balloon_dev_info *balloon,
>  				       struct page *page)
>  {
> -	__SetPageBalloon(page);
> +	__SetPageOffline(page);
>  	__SetPageMovable(page, balloon->inode->i_mapping);
>  	set_page_private(page, (unsigned long)balloon);
>  	list_add(&page->lru, &balloon->pages);
> @@ -111,7 +111,7 @@ static inline void balloon_page_insert(struct
> balloon_dev_info *balloon,
>   */
>  static inline void balloon_page_delete(struct page *page)
>  {
> -	__ClearPageBalloon(page);
> +	__ClearPageOffline(page);
>  	__ClearPageMovable(page);
>  	set_page_private(page, 0);
>  	/*
> @@ -141,13 +141,13 @@ static inline gfp_t balloon_mapping_gfp_mask(void)
>  static inline void balloon_page_insert(struct balloon_dev_info *balloon,
>  				       struct page *page)
>  {
> -	__SetPageBalloon(page);
> +	__SetPageOffline(page);
>  	list_add(&page->lru, &balloon->pages);
>  }
>  
>  static inline void balloon_page_delete(struct page *page)
>  {
> -	__ClearPageBalloon(page);
> +	__ClearPageOffline(page);
>  	list_del(&page->lru);
>  }
>  
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 50ce1bddaf56..f91da3d0a67e 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -670,7 +670,7 @@ PAGEFLAG_FALSE(DoubleMap)
>  #define PAGE_TYPE_BASE	0xf0000000
>  /* Reserve		0x0000007f to catch underflows of page_mapcount */
>  #define PG_buddy	0x00000080
> -#define PG_balloon	0x00000100
> +#define PG_offline	0x00000100
>  #define PG_kmemcg	0x00000200
>  #define PG_table	0x00000400
>  
> @@ -700,10 +700,13 @@ static __always_inline void __ClearPage##uname(struct
> page *page)	\
>  PAGE_TYPE_OPS(Buddy, buddy)
>  
>  /*
> - * PageBalloon() is true for pages that are on the balloon page list
> - * (see mm/balloon_compaction.c).
> + * PageOffline() indicates that the pages is logically offline although the
> + * containing section is online. (e.g. inflated in a balloon driver or
> + * not onlined when onlining the section).
> + * The content of these pages is effectively stale. Such pages should not
> + * be touched (read/write/dump/save) except by their owner.
>   */
> -PAGE_TYPE_OPS(Balloon, balloon)
> +PAGE_TYPE_OPS(Offline, offline)
>  
>  /*
>   * If kmemcg is enabled, the buddy allocator will set PageKmemcg() on
> diff --git a/include/uapi/linux/kernel-page-flags.h
> b/include/uapi/linux/kernel-page-flags.h
> index 21b9113c69da..6f2f2720f3ac 100644
> --- a/include/uapi/linux/kernel-page-flags.h
> +++ b/include/uapi/linux/kernel-page-flags.h
> @@ -32,7 +32,7 @@
>  
>  #define KPF_KSM			21
>  #define KPF_THP			22
> -#define KPF_BALLOON		23
> +#define KPF_OFFLINE		23
>  #define KPF_ZERO_PAGE		24
>  #define KPF_IDLE		25
>  #define KPF_PGTABLE		26
> diff --git a/tools/vm/page-types.c b/tools/vm/page-types.c
> index 37908a83ddc2..6c38d3b862e4 100644
> --- a/tools/vm/page-types.c
> +++ b/tools/vm/page-types.c
> @@ -133,7 +133,7 @@ static const char * const page_flag_names[] = {
>  	[KPF_NOPAGE]		= "n:nopage",
>  	[KPF_KSM]		= "x:ksm",
>  	[KPF_THP]		= "t:thp",
> -	[KPF_BALLOON]		= "o:balloon",
> +	[KPF_OFFLINE]		= "o:offline",
>  	[KPF_PGTABLE]		= "g:pgtable",
>  	[KPF_ZERO_PAGE]		= "z:zero_page",
>  	[KPF_IDLE]              = "i:idle_page",
> --
> 2.17.2

Acked-by: Pankaj gupta <pagupta@redhat.com>

> 
> 

[PATCH v1 3/8] kexec: export PG_offline to VMCOREINFO

[David Hildenbrand]

Right now, pages inflated as part of a balloon driver will be dumped
by dump tools like makedumpfile. While XEN is able to check in the
crash kernel whether a certain pfn is actuall backed by memory in the
hypervisor (see xen_oldmem_pfn_is_ram) and optimize this case, dumps of
other balloon inflated memory will essentially result in zero pages getting
allocated by the hypervisor and the dump getting filled with this data.

The allocation and reading of zero pages can directly be avoided if a
dumping tool could know which pages only contain stale information not to
be dumped.

We now have PG_offline which can be (and already is by virtio-balloon)
used for marking pages as logically offline. Follow up patches will
make use of this flag also in other balloon implementations.

Let's export PG_offline via PAGE_OFFLINE_MAPCOUNT_VALUE, so
makedumpfile can directly skip pages that are logically offline and the
content therefore stale.

Please note that this is also helpful for a problem we were seeing under
Hyper-V: Dumping logically offline memory (pages kept fake offline while
onlining a section via online_page_callback) would under some condicions
result in a kernel panic when dumping them.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Dave Young <dyoung@redhat.com>
Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Baoquan He <bhe@redhat.com>
Cc: Omar Sandoval <osandov@fb.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Lianbo Jiang <lijiang@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 kernel/crash_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/crash_core.c b/kernel/crash_core.c
index 933cb3e45b98..093c9f917ed0 100644
--- a/kernel/crash_core.c
+++ b/kernel/crash_core.c
@@ -464,6 +464,8 @@ static int __init crash_save_vmcoreinfo_init(void)
 	VMCOREINFO_NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE);
 #ifdef CONFIG_HUGETLB_PAGE
 	VMCOREINFO_NUMBER(HUGETLB_PAGE_DTOR);
+#define PAGE_OFFLINE_MAPCOUNT_VALUE	(~PG_offline)
+	VMCOREINFO_NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE);
 #endif
 
 	arch_crash_save_vmcoreinfo();
-- 
2.17.2

Re: [PATCH v1 3/8] kexec: export PG_offline to VMCOREINFO

[Michael S. Tsirkin]

On Mon, Nov 19, 2018 at 11:16:11AM +0100, David Hildenbrand wrote:
> Right now, pages inflated as part of a balloon driver will be dumped
> by dump tools like makedumpfile. While XEN is able to check in the
> crash kernel whether a certain pfn is actuall backed by memory in the
> hypervisor (see xen_oldmem_pfn_is_ram) and optimize this case, dumps of
> other balloon inflated memory will essentially result in zero pages getting
> allocated by the hypervisor and the dump getting filled with this data.
> 
> The allocation and reading of zero pages can directly be avoided if a
> dumping tool could know which pages only contain stale information not to
> be dumped.
> 
> We now have PG_offline which can be (and already is by virtio-balloon)
> used for marking pages as logically offline. Follow up patches will
> make use of this flag also in other balloon implementations.
> 
> Let's export PG_offline via PAGE_OFFLINE_MAPCOUNT_VALUE, so
> makedumpfile can directly skip pages that are logically offline and the
> content therefore stale.
> 
> Please note that this is also helpful for a problem we were seeing under
> Hyper-V: Dumping logically offline memory (pages kept fake offline while
> onlining a section via online_page_callback) would under some condicions
> result in a kernel panic when dumping them.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Dave Young <dyoung@redhat.com>
> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Omar Sandoval <osandov@fb.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Lianbo Jiang <lijiang@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  kernel/crash_core.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 933cb3e45b98..093c9f917ed0 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -464,6 +464,8 @@ static int __init crash_save_vmcoreinfo_init(void)
>  	VMCOREINFO_NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE);
>  #ifdef CONFIG_HUGETLB_PAGE
>  	VMCOREINFO_NUMBER(HUGETLB_PAGE_DTOR);
> +#define PAGE_OFFLINE_MAPCOUNT_VALUE	(~PG_offline)
> +	VMCOREINFO_NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE);
>  #endif
>  
>  	arch_crash_save_vmcoreinfo();
> -- 
> 2.17.2

Re: [PATCH v1 3/8] kexec: export PG_offline to VMCOREINFO

[Dave Young]

On 11/19/18 at 11:16am, David Hildenbrand wrote:
> Right now, pages inflated as part of a balloon driver will be dumped
> by dump tools like makedumpfile. While XEN is able to check in the
> crash kernel whether a certain pfn is actuall backed by memory in the
> hypervisor (see xen_oldmem_pfn_is_ram) and optimize this case, dumps of
> other balloon inflated memory will essentially result in zero pages getting
> allocated by the hypervisor and the dump getting filled with this data.
> 
> The allocation and reading of zero pages can directly be avoided if a
> dumping tool could know which pages only contain stale information not to
> be dumped.
> 
> We now have PG_offline which can be (and already is by virtio-balloon)
> used for marking pages as logically offline. Follow up patches will
> make use of this flag also in other balloon implementations.
> 
> Let's export PG_offline via PAGE_OFFLINE_MAPCOUNT_VALUE, so
> makedumpfile can directly skip pages that are logically offline and the
> content therefore stale.
> 
> Please note that this is also helpful for a problem we were seeing under
> Hyper-V: Dumping logically offline memory (pages kept fake offline while
> onlining a section via online_page_callback) would under some condicions
> result in a kernel panic when dumping them.
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Dave Young <dyoung@redhat.com>
> Cc: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> Cc: Baoquan He <bhe@redhat.com>
> Cc: Omar Sandoval <osandov@fb.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Lianbo Jiang <lijiang@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  kernel/crash_core.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 933cb3e45b98..093c9f917ed0 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -464,6 +464,8 @@ static int __init crash_save_vmcoreinfo_init(void)
>  	VMCOREINFO_NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE);
>  #ifdef CONFIG_HUGETLB_PAGE
>  	VMCOREINFO_NUMBER(HUGETLB_PAGE_DTOR);
> +#define PAGE_OFFLINE_MAPCOUNT_VALUE	(~PG_offline)
> +	VMCOREINFO_NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE);
>  #endif
>  
>  	arch_crash_save_vmcoreinfo();
> -- 
> 2.17.2
> 

Acked-by: Dave Young <dyoung@redhat.com>

Thanks
Dave

Re: [PATCH v1 3/8] kexec: export PG_offline to VMCOREINFO

[Baoquan He]

On 11/19/18 at 11:16am, David Hildenbrand wrote:
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
> index 933cb3e45b98..093c9f917ed0 100644
> --- a/kernel/crash_core.c
> +++ b/kernel/crash_core.c
> @@ -464,6 +464,8 @@ static int __init crash_save_vmcoreinfo_init(void)
>  	VMCOREINFO_NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE);
>  #ifdef CONFIG_HUGETLB_PAGE
>  	VMCOREINFO_NUMBER(HUGETLB_PAGE_DTOR);
> +#define PAGE_OFFLINE_MAPCOUNT_VALUE	(~PG_offline)
> +	VMCOREINFO_NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE);
>  #endif

This solution looks good to me. One small concern is why we don't
export PG_offline to vmcoreinfo directly, then define
PAGE_OFFLINE_MAPCOUNT_VALUE in makedumpfile. We have been exporting
kernel data/MACRO directly, why this one is exceptional.

Thanks
Baoquan

Re: [PATCH v1 3/8] kexec: export PG_offline to VMCOREINFO

[David Hildenbrand]

On 21.11.18 07:04, Baoquan He wrote:
> On 11/19/18 at 11:16am, David Hildenbrand wrote:
>> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>> index 933cb3e45b98..093c9f917ed0 100644
>> --- a/kernel/crash_core.c
>> +++ b/kernel/crash_core.c
>> @@ -464,6 +464,8 @@ static int __init crash_save_vmcoreinfo_init(void)
>>  	VMCOREINFO_NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE);
>>  #ifdef CONFIG_HUGETLB_PAGE
>>  	VMCOREINFO_NUMBER(HUGETLB_PAGE_DTOR);
>> +#define PAGE_OFFLINE_MAPCOUNT_VALUE	(~PG_offline)
>> +	VMCOREINFO_NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE);
>>  #endif
> 
> This solution looks good to me. One small concern is why we don't
> export PG_offline to vmcoreinfo directly, then define
> PAGE_OFFLINE_MAPCOUNT_VALUE in makedumpfile. We have been exporting
> kernel data/MACRO directly, why this one is exceptional.
> 

1. We are much more similar to PG_buddy (in contrast to actual page
flags), and for PG_buddy it is historically handled like this (and I
think it makes sense to expose these as actual MAPCOUNT_VALUEs).

2. Right now only one page type per page is supported. Therefore only
exactly one value in mapcount indicates e.g. PageBuddy()/PageOffline().

Now, if we ever decide to change this (e.g. treat them like real flags),
it is much easier to switch to PG_offline/PG_buddy then. We can directly
see in makedumpfile that .*_MAPCOUNT_VALUE is no longer available but
instead e.g. PG_offline and PG_buddy. Instead we would no see a change
in makedumpfile and would have to rely on other properties.

If there are no strong opinions I will leave it like this.

Thanks!

> Thanks
> Baoquan
> 


-- 

Thanks,

David / dhildenb

[PATCH v1 4/8] xen/balloon: mark inflated pages PG_offline

[David Hildenbrand]

Mark inflated and never onlined pages PG_offline, to tell the world that
the content is stale and should not be dumped.

Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 drivers/xen/balloon.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 12148289debd..14dd6b814db3 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -425,6 +425,7 @@ static int xen_bring_pgs_online(struct page *pg, unsigned int order)
 	for (i = 0; i < size; i++) {
 		p = pfn_to_page(start_pfn + i);
 		__online_page_set_limits(p);
+		__SetPageOffline(p);
 		__balloon_append(p);
 	}
 	mutex_unlock(&balloon_mutex);
@@ -493,6 +494,7 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
 		xenmem_reservation_va_mapping_update(1, &page, &frame_list[i]);
 
 		/* Relinquish the page back to the allocator. */
+		__ClearPageOffline(page);
 		free_reserved_page(page);
 	}
 
@@ -519,6 +521,7 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
 			state = BP_EAGAIN;
 			break;
 		}
+		__SetPageOffline(page);
 		adjust_managed_page_count(page, -1);
 		xenmem_reservation_scrub_page(page);
 		list_add(&page->lru, &pages);
-- 
2.17.2

Re: [PATCH v1 4/8] xen/balloon: mark inflated pages PG_offline

[Juergen Gross]

On 19/11/2018 11:16, David Hildenbrand wrote:
> Mark inflated and never onlined pages PG_offline, to tell the world that
> the content is stale and should not be dumped.
> 
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  drivers/xen/balloon.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
> index 12148289debd..14dd6b814db3 100644
> --- a/drivers/xen/balloon.c
> +++ b/drivers/xen/balloon.c
> @@ -425,6 +425,7 @@ static int xen_bring_pgs_online(struct page *pg, unsigned int order)
>  	for (i = 0; i < size; i++) {
>  		p = pfn_to_page(start_pfn + i);
>  		__online_page_set_limits(p);
> +		__SetPageOffline(p);
>  		__balloon_append(p);
>  	}

This seems not to be based on current master. Could you please tell
against which tree this should be reviewed?


Juergen

Re: [PATCH v1 4/8] xen/balloon: mark inflated pages PG_offline

[David Hildenbrand]

On 19.11.18 13:22, Juergen Gross wrote:
> On 19/11/2018 11:16, David Hildenbrand wrote:
>> Mark inflated and never onlined pages PG_offline, to tell the world that
>> the content is stale and should not be dumped.
>>
>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> Cc: Juergen Gross <jgross@suse.com>
>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Matthew Wilcox <willy@infradead.org>
>> Cc: Michal Hocko <mhocko@suse.com>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  drivers/xen/balloon.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
>> index 12148289debd..14dd6b814db3 100644
>> --- a/drivers/xen/balloon.c
>> +++ b/drivers/xen/balloon.c
>> @@ -425,6 +425,7 @@ static int xen_bring_pgs_online(struct page *pg, unsigned int order)
>>  	for (i = 0; i < size; i++) {
>>  		p = pfn_to_page(start_pfn + i);
>>  		__online_page_set_limits(p);
>> +		__SetPageOffline(p);
>>  		__balloon_append(p);
>>  	}
> 
> This seems not to be based on current master. Could you please tell
> against which tree this should be reviewed?
> 
Hi Juergen,

this is based on linux-next/master.

Thanks!

> 
> Juergen
> 


-- 

Thanks,

David / dhildenb

[PATCH v1 5/8] hv_balloon: mark inflated pages PG_offline

[David Hildenbrand]

Mark inflated and never onlined pages PG_offline, to tell the world that
the content is stale and should not be dumped.

Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: Kairui Song <kasong@redhat.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 drivers/hv/hv_balloon.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 211f3fe3a038..47719862e57f 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -681,8 +681,13 @@ static struct notifier_block hv_memory_nb = {
 /* Check if the particular page is backed and can be onlined and online it. */
 static void hv_page_online_one(struct hv_hotadd_state *has, struct page *pg)
 {
-	if (!has_pfn_is_backed(has, page_to_pfn(pg)))
+	if (!has_pfn_is_backed(has, page_to_pfn(pg))) {
+		if (!PageOffline(pg))
+			__SetPageOffline(pg);
 		return;
+	}
+	if (PageOffline(pg))
+		__ClearPageOffline(pg);
 
 	/* This frame is currently backed; online the page. */
 	__online_page_set_limits(pg);
@@ -1201,6 +1206,7 @@ static void free_balloon_pages(struct hv_dynmem_device *dm,
 
 	for (i = 0; i < num_pages; i++) {
 		pg = pfn_to_page(i + start_frame);
+		__ClearPageOffline(pg);
 		__free_page(pg);
 		dm->num_pages_ballooned--;
 	}
@@ -1213,7 +1219,7 @@ static unsigned int alloc_balloon_pages(struct hv_dynmem_device *dm,
 					struct dm_balloon_response *bl_resp,
 					int alloc_unit)
 {
-	unsigned int i = 0;
+	unsigned int i, j;
 	struct page *pg;
 
 	if (num_pages < alloc_unit)
@@ -1245,6 +1251,10 @@ static unsigned int alloc_balloon_pages(struct hv_dynmem_device *dm,
 		if (alloc_unit != 1)
 			split_page(pg, get_order(alloc_unit << PAGE_SHIFT));
 
+		/* mark all pages offline */
+		for (j = 0; j < (1 << get_order(alloc_unit << PAGE_SHIFT)); j++)
+			__SetPageOffline(pg + j);
+
 		bl_resp->range_count++;
 		bl_resp->range_array[i].finfo.start_page =
 			page_to_pfn(pg);
-- 
2.17.2

Re: [PATCH v1 5/8] hv_balloon: mark inflated pages PG_offline

[Pankaj Gupta]

Hi David,

> 
> Mark inflated and never onlined pages PG_offline, to tell the world that
> the content is stale and should not be dumped.
> 
> Cc: "K. Y. Srinivasan" <kys@microsoft.com>
> Cc: Haiyang Zhang <haiyangz@microsoft.com>
> Cc: Stephen Hemminger <sthemmin@microsoft.com>
> Cc: Kairui Song <kasong@redhat.com>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  drivers/hv/hv_balloon.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index 211f3fe3a038..47719862e57f 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c
> @@ -681,8 +681,13 @@ static struct notifier_block hv_memory_nb = {
>  /* Check if the particular page is backed and can be onlined and online it.
>  */
>  static void hv_page_online_one(struct hv_hotadd_state *has, struct page *pg)
>  {
> -	if (!has_pfn_is_backed(has, page_to_pfn(pg)))
> +	if (!has_pfn_is_backed(has, page_to_pfn(pg))) {
> +		if (!PageOffline(pg))
> +			__SetPageOffline(pg);
>  		return;
> +	}
> +	if (PageOffline(pg))
> +		__ClearPageOffline(pg);
>  
>  	/* This frame is currently backed; online the page. */
>  	__online_page_set_limits(pg);
> @@ -1201,6 +1206,7 @@ static void free_balloon_pages(struct hv_dynmem_device
> *dm,
>  
>  	for (i = 0; i < num_pages; i++) {
>  		pg = pfn_to_page(i + start_frame);
> +		__ClearPageOffline(pg);

Just thinking, do we need to care for clearing PageOffline flag before freeing
a balloon'd page?

Thanks,
Pankaj

>  		__free_page(pg);
>  		dm->num_pages_ballooned--;
>  	}
> @@ -1213,7 +1219,7 @@ static unsigned int alloc_balloon_pages(struct
> hv_dynmem_device *dm,
>  					struct dm_balloon_response *bl_resp,
>  					int alloc_unit)
>  {
> -	unsigned int i = 0;
> +	unsigned int i, j;
>  	struct page *pg;
>  
>  	if (num_pages < alloc_unit)
> @@ -1245,6 +1251,10 @@ static unsigned int alloc_balloon_pages(struct
> hv_dynmem_device *dm,
>  		if (alloc_unit != 1)
>  			split_page(pg, get_order(alloc_unit << PAGE_SHIFT));
>  
> +		/* mark all pages offline */
> +		for (j = 0; j < (1 << get_order(alloc_unit << PAGE_SHIFT)); j++)
> +			__SetPageOffline(pg + j);
> +
>  		bl_resp->range_count++;
>  		bl_resp->range_array[i].finfo.start_page =
>  			page_to_pfn(pg);
> --
> 2.17.2
> 
> 

Re: [PATCH v1 5/8] hv_balloon: mark inflated pages PG_offline

[David Hildenbrand]

On 20.11.18 09:45, Pankaj Gupta wrote:
> 
> Hi David,
> 
>>
>> Mark inflated and never onlined pages PG_offline, to tell the world that
>> the content is stale and should not be dumped.
>>
>> Cc: "K. Y. Srinivasan" <kys@microsoft.com>
>> Cc: Haiyang Zhang <haiyangz@microsoft.com>
>> Cc: Stephen Hemminger <sthemmin@microsoft.com>
>> Cc: Kairui Song <kasong@redhat.com>
>> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Matthew Wilcox <willy@infradead.org>
>> Cc: Michal Hocko <mhocko@suse.com>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  drivers/hv/hv_balloon.c | 14 ++++++++++++--
>>  1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
>> index 211f3fe3a038..47719862e57f 100644
>> --- a/drivers/hv/hv_balloon.c
>> +++ b/drivers/hv/hv_balloon.c
>> @@ -681,8 +681,13 @@ static struct notifier_block hv_memory_nb = {
>>  /* Check if the particular page is backed and can be onlined and online it.
>>  */
>>  static void hv_page_online_one(struct hv_hotadd_state *has, struct page *pg)
>>  {
>> -	if (!has_pfn_is_backed(has, page_to_pfn(pg)))
>> +	if (!has_pfn_is_backed(has, page_to_pfn(pg))) {
>> +		if (!PageOffline(pg))
>> +			__SetPageOffline(pg);
>>  		return;
>> +	}
>> +	if (PageOffline(pg))
>> +		__ClearPageOffline(pg);
>>  
>>  	/* This frame is currently backed; online the page. */
>>  	__online_page_set_limits(pg);
>> @@ -1201,6 +1206,7 @@ static void free_balloon_pages(struct hv_dynmem_device
>> *dm,
>>  
>>  	for (i = 0; i < num_pages; i++) {
>>  		pg = pfn_to_page(i + start_frame);
>> +		__ClearPageOffline(pg);
> 
> Just thinking, do we need to care for clearing PageOffline flag before freeing
> a balloon'd page?

Yes we have to otherwise the code will crash when trying to set PageBuddy.

(only one page type at a time may be set right now, and it makes sense.
A page that is offline cannot e.g. be a buddy page)

So PageOffline is completely managed by the page owner.

> 
> Thanks,
> Pankaj


-- 

Thanks,

David / dhildenb

Re: [PATCH v1 5/8] hv_balloon: mark inflated pages PG_offline

[Pankaj Gupta]

> >> ---
> >>  drivers/hv/hv_balloon.c | 14 ++++++++++++--
> >>  1 file changed, 12 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> >> index 211f3fe3a038..47719862e57f 100644
> >> --- a/drivers/hv/hv_balloon.c
> >> +++ b/drivers/hv/hv_balloon.c
> >> @@ -681,8 +681,13 @@ static struct notifier_block hv_memory_nb = {
> >>  /* Check if the particular page is backed and can be onlined and online
> >>  it.
> >>  */
> >>  static void hv_page_online_one(struct hv_hotadd_state *has, struct page
> >>  *pg)
> >>  {
> >> -	if (!has_pfn_is_backed(has, page_to_pfn(pg)))
> >> +	if (!has_pfn_is_backed(has, page_to_pfn(pg))) {
> >> +		if (!PageOffline(pg))
> >> +			__SetPageOffline(pg);
> >>  		return;
> >> +	}
> >> +	if (PageOffline(pg))
> >> +		__ClearPageOffline(pg);
> >>  
> >>  	/* This frame is currently backed; online the page. */
> >>  	__online_page_set_limits(pg);
> >> @@ -1201,6 +1206,7 @@ static void free_balloon_pages(struct
> >> hv_dynmem_device
> >> *dm,
> >>  
> >>  	for (i = 0; i < num_pages; i++) {
> >>  		pg = pfn_to_page(i + start_frame);
> >> +		__ClearPageOffline(pg);
> > 
> > Just thinking, do we need to care for clearing PageOffline flag before
> > freeing
> > a balloon'd page?
> 
> Yes we have to otherwise the code will crash when trying to set PageBuddy.
> 
> (only one page type at a time may be set right now, and it makes sense.
> A page that is offline cannot e.g. be a buddy page)

o.k
> 
> So PageOffline is completely managed by the page owner.

Makes sense. Thanks for explaining.

Pankaj
> 
> > 
> > Thanks,
> > Pankaj
> 
> 
> --
> 
> Thanks,
> 
> David / dhildenb
> 

Re: [PATCH v1 5/8] hv_balloon: mark inflated pages PG_offline

[Pankaj Gupta]

> > >> ---
> > >>  drivers/hv/hv_balloon.c | 14 ++++++++++++--
> > >>  1 file changed, 12 insertions(+), 2 deletions(-)
> > >>
> > >> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> > >> index 211f3fe3a038..47719862e57f 100644
> > >> --- a/drivers/hv/hv_balloon.c
> > >> +++ b/drivers/hv/hv_balloon.c
> > >> @@ -681,8 +681,13 @@ static struct notifier_block hv_memory_nb = {
> > >>  /* Check if the particular page is backed and can be onlined and online
> > >>  it.
> > >>  */
> > >>  static void hv_page_online_one(struct hv_hotadd_state *has, struct page
> > >>  *pg)
> > >>  {
> > >> -	if (!has_pfn_is_backed(has, page_to_pfn(pg)))
> > >> +	if (!has_pfn_is_backed(has, page_to_pfn(pg))) {
> > >> +		if (!PageOffline(pg))
> > >> +			__SetPageOffline(pg);
> > >>  		return;
> > >> +	}
> > >> +	if (PageOffline(pg))
> > >> +		__ClearPageOffline(pg);
> > >>  
> > >>  	/* This frame is currently backed; online the page. */
> > >>  	__online_page_set_limits(pg);
> > >> @@ -1201,6 +1206,7 @@ static void free_balloon_pages(struct
> > >> hv_dynmem_device
> > >> *dm,
> > >>  
> > >>  	for (i = 0; i < num_pages; i++) {
> > >>  		pg = pfn_to_page(i + start_frame);
> > >> +		__ClearPageOffline(pg);
> > > 
> > > Just thinking, do we need to care for clearing PageOffline flag before
> > > freeing
> > > a balloon'd page?
> > 
> > Yes we have to otherwise the code will crash when trying to set PageBuddy.
> > 
> > (only one page type at a time may be set right now, and it makes sense.
> > A page that is offline cannot e.g. be a buddy page)
> 
> o.k
> > 
> > So PageOffline is completely managed by the page owner.
> 
> Makes sense. Thanks for explaining.

Looks good to me.

Acked-by: Pankaj gupta <pagupta@redhat.com>

[PATCH v1 6/8] vmw_balloon: mark inflated pages PG_offline

[David Hildenbrand]

Mark inflated and never onlined pages PG_offline, to tell the world that
the content is stale and should not be dumped.

Cc: Xavier Deguillard <xdeguillard@vmware.com>
Cc: Nadav Amit <namit@vmware.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Julien Freche <jfreche@vmware.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 drivers/misc/vmw_balloon.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
index e6126a4b95d3..8cc8bd9a4e32 100644
--- a/drivers/misc/vmw_balloon.c
+++ b/drivers/misc/vmw_balloon.c
@@ -544,6 +544,36 @@ unsigned int vmballoon_page_order(enum vmballoon_page_size_type page_size)
 	return page_size == VMW_BALLOON_2M_PAGE ? VMW_BALLOON_2M_ORDER : 0;
 }
 
+/**
+ * vmballoon_mark_page_offline() - mark a page as offline
+ * @page: pointer for the page
+ * @page_size: the size of the page.
+ */
+static void
+vmballoon_mark_page_offline(struct page *page,
+			    enum vmballoon_page_size_type page_size)
+{
+	int i;
+
+	for (i = 0; i < 1ULL << vmballoon_page_order(page_size); i++)
+		__SetPageOffline(page + i);
+}
+
+/**
+ * vmballoon_mark_page_online() - mark a page as online
+ * @page: pointer for the page
+ * @page_size: the size of the page.
+ */
+static void
+vmballoon_mark_page_online(struct page *page,
+			   enum vmballoon_page_size_type page_size)
+{
+	int i;
+
+	for (i = 0; i < 1ULL << vmballoon_page_order(page_size); i++)
+		__ClearPageOffline(page + i);
+}
+
 /**
  * vmballoon_page_in_frames() - returns the number of frames in a page.
  * @page_size: the size of the page.
@@ -612,6 +642,7 @@ static int vmballoon_alloc_page_list(struct vmballoon *b,
 					 ctl->page_size);
 
 		if (page) {
+			vmballoon_mark_page_offline(page, ctl->page_size);
 			/* Success. Add the page to the list and continue. */
 			list_add(&page->lru, &ctl->pages);
 			continue;
@@ -850,6 +881,7 @@ static void vmballoon_release_page_list(struct list_head *page_list,
 
 	list_for_each_entry_safe(page, tmp, page_list, lru) {
 		list_del(&page->lru);
+		vmballoon_mark_page_online(page, page_size);
 		__free_pages(page, vmballoon_page_order(page_size));
 	}
 
-- 
2.17.2

Re: [PATCH v1 6/8] vmw_balloon: mark inflated pages PG_offline

[Nadav Amit]

Thanks for this patch!

> On Nov 19, 2018, at 2:16 AM, David Hildenbrand <david@redhat.com> wrote:
> 
> Mark inflated and never onlined pages PG_offline, to tell the world that
> the content is stale and should not be dumped.
> 
> Cc: Xavier Deguillard <xdeguillard@vmware.com>
> Cc: Nadav Amit <namit@vmware.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Julien Freche <jfreche@vmware.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> drivers/misc/vmw_balloon.c | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
> index e6126a4b95d3..8cc8bd9a4e32 100644
> --- a/drivers/misc/vmw_balloon.c
> +++ b/drivers/misc/vmw_balloon.c
> @@ -544,6 +544,36 @@ unsigned int vmballoon_page_order(enum vmballoon_page_size_type page_size)
> 	return page_size == VMW_BALLOON_2M_PAGE ? VMW_BALLOON_2M_ORDER : 0;
> }
> 
> +/**
> + * vmballoon_mark_page_offline() - mark a page as offline
> + * @page: pointer for the page

If possible, please add a period at the end of the sentence (yes, I know I
got it wrong in some places too).

> + * @page_size: the size of the page.
> + */
> +static void
> +vmballoon_mark_page_offline(struct page *page,
> +			    enum vmballoon_page_size_type page_size)
> +{
> +	int i;
> +
> +	for (i = 0; i < 1ULL << vmballoon_page_order(page_size); i++)

Can you please do instead:

	unsigned int;

	for (i = 0; i < vmballoon_page_in_frames(page_size); i++)


> +		__SetPageOffline(page + i);
> +}
> +
> +/**
> + * vmballoon_mark_page_online() - mark a page as online
> + * @page: pointer for the page
> + * @page_size: the size of the page.
> + */
> +static void
> +vmballoon_mark_page_online(struct page *page,
> +			   enum vmballoon_page_size_type page_size)
> +{
> +	int i;
> +
> +	for (i = 0; i < 1ULL << vmballoon_page_order(page_size); i++)
> +		__ClearPageOffline(page + i);

Same here (use vmballoon_page_in_frames).

> +}
> +
> /**
>  * vmballoon_page_in_frames() - returns the number of frames in a page.
>  * @page_size: the size of the page.
> @@ -612,6 +642,7 @@ static int vmballoon_alloc_page_list(struct vmballoon *b,
> 					 ctl->page_size);
> 
> 		if (page) {
> +			vmballoon_mark_page_offline(page, ctl->page_size);
> 			/* Success. Add the page to the list and continue. */
> 			list_add(&page->lru, &ctl->pages);
> 			continue;
> @@ -850,6 +881,7 @@ static void vmballoon_release_page_list(struct list_head *page_list,
> 
> 	list_for_each_entry_safe(page, tmp, page_list, lru) {
> 		list_del(&page->lru);
> +		vmballoon_mark_page_online(page, page_size);
> 		__free_pages(page, vmballoon_page_order(page_size));
> 	}

We would like to test it in the next few days, but in the meanwhile, after
you address these minor issues:

Acked-by: Nadav Amit <namit@vmware.com>

Thanks again,
Nadav 

Re: [PATCH v1 6/8] vmw_balloon: mark inflated pages PG_offline

[David Hildenbrand]

On 21.11.18 04:22, Nadav Amit wrote:
> Thanks for this patch!
> 
>> On Nov 19, 2018, at 2:16 AM, David Hildenbrand <david@redhat.com> wrote:
>>
>> Mark inflated and never onlined pages PG_offline, to tell the world that
>> the content is stale and should not be dumped.
>>
>> Cc: Xavier Deguillard <xdeguillard@vmware.com>
>> Cc: Nadav Amit <namit@vmware.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Julien Freche <jfreche@vmware.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Matthew Wilcox <willy@infradead.org>
>> Cc: Michal Hocko <mhocko@suse.com>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>> drivers/misc/vmw_balloon.c | 32 ++++++++++++++++++++++++++++++++
>> 1 file changed, 32 insertions(+)
>>
>> diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
>> index e6126a4b95d3..8cc8bd9a4e32 100644
>> --- a/drivers/misc/vmw_balloon.c
>> +++ b/drivers/misc/vmw_balloon.c
>> @@ -544,6 +544,36 @@ unsigned int vmballoon_page_order(enum vmballoon_page_size_type page_size)
>> 	return page_size == VMW_BALLOON_2M_PAGE ? VMW_BALLOON_2M_ORDER : 0;
>> }
>>
>> +/**
>> + * vmballoon_mark_page_offline() - mark a page as offline
>> + * @page: pointer for the page
> 
> If possible, please add a period at the end of the sentence (yes, I know I
> got it wrong in some places too).

Sure :)

> 
>> + * @page_size: the size of the page.
>> + */
>> +static void
>> +vmballoon_mark_page_offline(struct page *page,
>> +			    enum vmballoon_page_size_type page_size)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < 1ULL << vmballoon_page_order(page_size); i++)
> 
> Can you please do instead:
> 
> 	unsigned int;
> 
> 	for (i = 0; i < vmballoon_page_in_frames(page_size); i++)
> 

Will do, will have to move both functions a little bit down in the file
(exactly one function).


> We would like to test it in the next few days, but in the meanwhile, after
> you address these minor issues:
> 
> Acked-by: Nadav Amit <namit@vmware.com>

Thanks!

> 
> Thanks again,
> Nadav 
> 


-- 

Thanks,

David / dhildenb

[PATCH v1 7/8] PM / Hibernate: use pfn_to_online_page()

[David Hildenbrand]

Let's use pfn_to_online_page() instead of pfn_to_page() when checking
for saveable pages to not save/restore offline memory sections.

Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Len Brown <len.brown@intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Suggested-by: Michal Hocko <mhocko@kernel.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 kernel/power/snapshot.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index 640b2034edd6..87e6dd57819f 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -1215,8 +1215,8 @@ static struct page *saveable_highmem_page(struct zone *zone, unsigned long pfn)
 	if (!pfn_valid(pfn))
 		return NULL;
 
-	page = pfn_to_page(pfn);
-	if (page_zone(page) != zone)
+	page = pfn_to_online_page(pfn);
+	if (!page || page_zone(page) != zone)
 		return NULL;
 
 	BUG_ON(!PageHighMem(page));
@@ -1277,8 +1277,8 @@ static struct page *saveable_page(struct zone *zone, unsigned long pfn)
 	if (!pfn_valid(pfn))
 		return NULL;
 
-	page = pfn_to_page(pfn);
-	if (page_zone(page) != zone)
+	page = pfn_to_online_page(pfn);
+	if (!page || page_zone(page) != zone)
 		return NULL;
 
 	BUG_ON(PageHighMem(page));
-- 
2.17.2

Re: [PATCH v1 7/8] PM / Hibernate: use pfn_to_online_page()

[Michal Hocko]

On Mon 19-11-18 11:16:15, David Hildenbrand wrote:
> Let's use pfn_to_online_page() instead of pfn_to_page() when checking
> for saveable pages to not save/restore offline memory sections.
> 
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Len Brown <len.brown@intel.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Suggested-by: Michal Hocko <mhocko@kernel.org>
> Signed-off-by: David Hildenbrand <david@redhat.com>

I have only a very vague understanding of this specific code but I do
not really see any real reason for checking offlined ranges. Also
offline pfn ranges might have uninitialized struct pages so making
any decisions of the struct page is basically undefined behavior.

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  kernel/power/snapshot.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
> index 640b2034edd6..87e6dd57819f 100644
> --- a/kernel/power/snapshot.c
> +++ b/kernel/power/snapshot.c
> @@ -1215,8 +1215,8 @@ static struct page *saveable_highmem_page(struct zone *zone, unsigned long pfn)
>  	if (!pfn_valid(pfn))
>  		return NULL;
>  
> -	page = pfn_to_page(pfn);
> -	if (page_zone(page) != zone)
> +	page = pfn_to_online_page(pfn);
> +	if (!page || page_zone(page) != zone)
>  		return NULL;
>  
>  	BUG_ON(!PageHighMem(page));
> @@ -1277,8 +1277,8 @@ static struct page *saveable_page(struct zone *zone, unsigned long pfn)
>  	if (!pfn_valid(pfn))
>  		return NULL;
>  
> -	page = pfn_to_page(pfn);
> -	if (page_zone(page) != zone)
> +	page = pfn_to_online_page(pfn);
> +	if (!page || page_zone(page) != zone)
>  		return NULL;
>  
>  	BUG_ON(PageHighMem(page));
> -- 
> 2.17.2

-- 
Michal Hocko
SUSE Labs

Re: [PATCH v1 7/8] PM / Hibernate: use pfn_to_online_page()

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 427 bytes --]

On Mon 2018-11-19 11:16:15, David Hildenbrand wrote:
> Let's use pfn_to_online_page() instead of pfn_to_page() when checking
> for saveable pages to not save/restore offline memory sections.
> 
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>

Acked-by: Pavel Machek <pavel@ucw.cz>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v1 7/8] PM / Hibernate: use pfn_to_online_page()

[Rafael J. Wysocki]

On Monday, November 19, 2018 11:16:15 AM CET David Hildenbrand wrote:
> Let's use pfn_to_online_page() instead of pfn_to_page() when checking
> for saveable pages to not save/restore offline memory sections.
> 
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Len Brown <len.brown@intel.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Suggested-by: Michal Hocko <mhocko@kernel.org>
> Signed-off-by: David Hildenbrand <david@redhat.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  kernel/power/snapshot.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
> index 640b2034edd6..87e6dd57819f 100644
> --- a/kernel/power/snapshot.c
> +++ b/kernel/power/snapshot.c
> @@ -1215,8 +1215,8 @@ static struct page *saveable_highmem_page(struct zone *zone, unsigned long pfn)
>  	if (!pfn_valid(pfn))
>  		return NULL;
>  
> -	page = pfn_to_page(pfn);
> -	if (page_zone(page) != zone)
> +	page = pfn_to_online_page(pfn);
> +	if (!page || page_zone(page) != zone)
>  		return NULL;
>  
>  	BUG_ON(!PageHighMem(page));
> @@ -1277,8 +1277,8 @@ static struct page *saveable_page(struct zone *zone, unsigned long pfn)
>  	if (!pfn_valid(pfn))
>  		return NULL;
>  
> -	page = pfn_to_page(pfn);
> -	if (page_zone(page) != zone)
> +	page = pfn_to_online_page(pfn);
> +	if (!page || page_zone(page) != zone)
>  		return NULL;
>  
>  	BUG_ON(PageHighMem(page));
> 

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

[David Hildenbrand]

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" <rjw@rjwysocki.net>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Len Brown <len.brown@intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 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

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

[Rafael J. Wysocki]

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" <rjw@rjwysocki.net>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Len Brown <len.brown@intel.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> Signed-off-by: David Hildenbrand <david@redhat.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  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;
> 

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

[William Kucharski]

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@redhat.com> 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
> 

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

[David Hildenbrand]

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

[PATCH v1] makedumpfile: exclude pages that are logically offline

[David Hildenbrand]

Linux marks pages that are logically offline via a page flag (map count).
Such pages e.g. include pages infated as part of a balloon driver or
pages that were not actually onlined when onlining the whole section.

While the hypervisor usually allows to read such inflated memory, we
basically read and dump data that is completely irrelevant. Also, this
might result in quite some overhead in the hypervisor. In addition,
we saw some problems under Hyper-V, whereby we can crash the kernel by
dumping, when reading memory of a partially onlined memory segment
(for memory added by the Hyper-V balloon driver).

Therefore, don't read and dump pages that are marked as being logically
offline.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
 makedumpfile.c | 34 ++++++++++++++++++++++++++++++----
 makedumpfile.h |  1 +
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index 8923538..b8bfd4c 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -88,6 +88,7 @@ mdf_pfn_t pfn_cache_private;
 mdf_pfn_t pfn_user;
 mdf_pfn_t pfn_free;
 mdf_pfn_t pfn_hwpoison;
+mdf_pfn_t pfn_offline;
 
 mdf_pfn_t num_dumped;
 
@@ -249,6 +250,21 @@ isHugetlb(unsigned long dtor)
                     && (SYMBOL(free_huge_page) == dtor));
 }
 
+static int
+isOffline(unsigned long flags, unsigned int _mapcount)
+{
+	if (NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE) == NOT_FOUND_NUMBER)
+		return FALSE;
+
+	if (flags & (1UL << NUMBER(PG_slab)))
+		return FALSE;
+
+	if (_mapcount == (int)NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE))
+		return TRUE;
+
+	return FALSE;
+}
+
 static int
 is_cache_page(unsigned long flags)
 {
@@ -2287,6 +2303,8 @@ write_vmcoreinfo_data(void)
 	WRITE_NUMBER("PG_hwpoison", PG_hwpoison);
 
 	WRITE_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
+	WRITE_NUMBER("PAGE_OFFLINE_MAPCOUNT_VALUE",
+		     PAGE_OFFLINE_MAPCOUNT_VALUE);
 	WRITE_NUMBER("phys_base", phys_base);
 
 	WRITE_NUMBER("HUGETLB_PAGE_DTOR", HUGETLB_PAGE_DTOR);
@@ -2687,6 +2705,7 @@ read_vmcoreinfo(void)
 	READ_SRCFILE("pud_t", pud_t);
 
 	READ_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
+	READ_NUMBER("PAGE_OFFLINE_MAPCOUNT_VALUE", PAGE_OFFLINE_MAPCOUNT_VALUE);
 	READ_NUMBER("phys_base", phys_base);
 #ifdef __aarch64__
 	READ_NUMBER("VA_BITS", VA_BITS);
@@ -6041,6 +6060,12 @@ __exclude_unnecessary_pages(unsigned long mem_map,
 		else if (isHWPOISON(flags)) {
 			pfn_counter = &pfn_hwpoison;
 		}
+		/*
+		 * Exclude pages that are logically offline.
+		 */
+		else if (isOffline(flags, _mapcount)) {
+			pfn_counter = &pfn_offline;
+		}
 		/*
 		 * Unexcludable page
 		 */
@@ -7522,7 +7547,7 @@ write_elf_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page)
 	 */
 	if (info->flag_cyclic) {
 		pfn_zero = pfn_cache = pfn_cache_private = 0;
-		pfn_user = pfn_free = pfn_hwpoison = 0;
+		pfn_user = pfn_free = pfn_hwpoison = pfn_offline = 0;
 		pfn_memhole = info->max_mapnr;
 	}
 
@@ -8804,7 +8829,7 @@ write_kdump_pages_and_bitmap_cyclic(struct cache_data *cd_header, struct cache_d
 		 * Reset counter for debug message.
 		 */
 		pfn_zero = pfn_cache = pfn_cache_private = 0;
-		pfn_user = pfn_free = pfn_hwpoison = 0;
+		pfn_user = pfn_free = pfn_hwpoison = pfn_offline = 0;
 		pfn_memhole = info->max_mapnr;
 
 		/*
@@ -9749,7 +9774,7 @@ print_report(void)
 	pfn_original = info->max_mapnr - pfn_memhole;
 
 	pfn_excluded = pfn_zero + pfn_cache + pfn_cache_private
-	    + pfn_user + pfn_free + pfn_hwpoison;
+	    + pfn_user + pfn_free + pfn_hwpoison + pfn_offline;
 	shrinking = (pfn_original - pfn_excluded) * 100;
 	shrinking = shrinking / pfn_original;
 
@@ -9763,6 +9788,7 @@ print_report(void)
 	REPORT_MSG("    User process data pages : 0x%016llx\n", pfn_user);
 	REPORT_MSG("    Free pages              : 0x%016llx\n", pfn_free);
 	REPORT_MSG("    Hwpoison pages          : 0x%016llx\n", pfn_hwpoison);
+	REPORT_MSG("    Offline pages           : 0x%016llx\n", pfn_offline);
 	REPORT_MSG("  Remaining pages  : 0x%016llx\n",
 	    pfn_original - pfn_excluded);
 	REPORT_MSG("  (The number of pages is reduced to %lld%%.)\n",
@@ -9790,7 +9816,7 @@ print_mem_usage(void)
 	pfn_original = info->max_mapnr - pfn_memhole;
 
 	pfn_excluded = pfn_zero + pfn_cache + pfn_cache_private
-	    + pfn_user + pfn_free + pfn_hwpoison;
+	    + pfn_user + pfn_free + pfn_hwpoison + pfn_offline;
 	shrinking = (pfn_original - pfn_excluded) * 100;
 	shrinking = shrinking / pfn_original;
 	total_size = info->page_size * pfn_original;
diff --git a/makedumpfile.h b/makedumpfile.h
index f02f86d..e3a2b29 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -1927,6 +1927,7 @@ struct number_table {
 	long    PG_hwpoison;
 
 	long	PAGE_BUDDY_MAPCOUNT_VALUE;
+	long	PAGE_OFFLINE_MAPCOUNT_VALUE;
 	long	SECTION_SIZE_BITS;
 	long	MAX_PHYSMEM_BITS;
 	long    HUGETLB_PAGE_DTOR;
-- 
2.17.2

RE: [PATCH v1] makedumpfile: exclude pages that are logically offline

[Kazuhito Hagio]

Hi David,

> Linux marks pages that are logically offline via a page flag (map count).
> Such pages e.g. include pages infated as part of a balloon driver or
> pages that were not actually onlined when onlining the whole section.
> 
> While the hypervisor usually allows to read such inflated memory, we
> basically read and dump data that is completely irrelevant. Also, this
> might result in quite some overhead in the hypervisor. In addition,
> we saw some problems under Hyper-V, whereby we can crash the kernel by
> dumping, when reading memory of a partially onlined memory segment
> (for memory added by the Hyper-V balloon driver).
> 
> Therefore, don't read and dump pages that are marked as being logically
> offline.
> 
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  makedumpfile.c | 34 ++++++++++++++++++++++++++++++----
>  makedumpfile.h |  1 +
>  2 files changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index 8923538..b8bfd4c 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -88,6 +88,7 @@ mdf_pfn_t pfn_cache_private;
>  mdf_pfn_t pfn_user;
>  mdf_pfn_t pfn_free;
>  mdf_pfn_t pfn_hwpoison;
> +mdf_pfn_t pfn_offline;
> 
>  mdf_pfn_t num_dumped;
> 
> @@ -249,6 +250,21 @@ isHugetlb(unsigned long dtor)
>                      && (SYMBOL(free_huge_page) == dtor));
>  }
> 
> +static int
> +isOffline(unsigned long flags, unsigned int _mapcount)
> +{
> +	if (NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE) == NOT_FOUND_NUMBER)
> +		return FALSE;

This is NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE), isn't it?
If so, I will correct it when merging.

Otherwise, looks good to me.

Thanks!
Kazu

> +
> +	if (flags & (1UL << NUMBER(PG_slab)))
> +		return FALSE;
> +
> +	if (_mapcount == (int)NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE))
> +		return TRUE;
> +
> +	return FALSE;
> +}
> +
>  static int
>  is_cache_page(unsigned long flags)
>  {
> @@ -2287,6 +2303,8 @@ write_vmcoreinfo_data(void)
>  	WRITE_NUMBER("PG_hwpoison", PG_hwpoison);
> 
>  	WRITE_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
> +	WRITE_NUMBER("PAGE_OFFLINE_MAPCOUNT_VALUE",
> +		     PAGE_OFFLINE_MAPCOUNT_VALUE);
>  	WRITE_NUMBER("phys_base", phys_base);
> 
>  	WRITE_NUMBER("HUGETLB_PAGE_DTOR", HUGETLB_PAGE_DTOR);
> @@ -2687,6 +2705,7 @@ read_vmcoreinfo(void)
>  	READ_SRCFILE("pud_t", pud_t);
> 
>  	READ_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
> +	READ_NUMBER("PAGE_OFFLINE_MAPCOUNT_VALUE", PAGE_OFFLINE_MAPCOUNT_VALUE);
>  	READ_NUMBER("phys_base", phys_base);
>  #ifdef __aarch64__
>  	READ_NUMBER("VA_BITS", VA_BITS);
> @@ -6041,6 +6060,12 @@ __exclude_unnecessary_pages(unsigned long mem_map,
>  		else if (isHWPOISON(flags)) {
>  			pfn_counter = &pfn_hwpoison;
>  		}
> +		/*
> +		 * Exclude pages that are logically offline.
> +		 */
> +		else if (isOffline(flags, _mapcount)) {
> +			pfn_counter = &pfn_offline;
> +		}
>  		/*
>  		 * Unexcludable page
>  		 */
> @@ -7522,7 +7547,7 @@ write_elf_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page)
>  	 */
>  	if (info->flag_cyclic) {
>  		pfn_zero = pfn_cache = pfn_cache_private = 0;
> -		pfn_user = pfn_free = pfn_hwpoison = 0;
> +		pfn_user = pfn_free = pfn_hwpoison = pfn_offline = 0;
>  		pfn_memhole = info->max_mapnr;
>  	}
> 
> @@ -8804,7 +8829,7 @@ write_kdump_pages_and_bitmap_cyclic(struct cache_data *cd_header, struct cache_d
>  		 * Reset counter for debug message.
>  		 */
>  		pfn_zero = pfn_cache = pfn_cache_private = 0;
> -		pfn_user = pfn_free = pfn_hwpoison = 0;
> +		pfn_user = pfn_free = pfn_hwpoison = pfn_offline = 0;
>  		pfn_memhole = info->max_mapnr;
> 
>  		/*
> @@ -9749,7 +9774,7 @@ print_report(void)
>  	pfn_original = info->max_mapnr - pfn_memhole;
> 
>  	pfn_excluded = pfn_zero + pfn_cache + pfn_cache_private
> -	    + pfn_user + pfn_free + pfn_hwpoison;
> +	    + pfn_user + pfn_free + pfn_hwpoison + pfn_offline;
>  	shrinking = (pfn_original - pfn_excluded) * 100;
>  	shrinking = shrinking / pfn_original;
> 
> @@ -9763,6 +9788,7 @@ print_report(void)
>  	REPORT_MSG("    User process data pages : 0x%016llx\n", pfn_user);
>  	REPORT_MSG("    Free pages              : 0x%016llx\n", pfn_free);
>  	REPORT_MSG("    Hwpoison pages          : 0x%016llx\n", pfn_hwpoison);
> +	REPORT_MSG("    Offline pages           : 0x%016llx\n", pfn_offline);
>  	REPORT_MSG("  Remaining pages  : 0x%016llx\n",
>  	    pfn_original - pfn_excluded);
>  	REPORT_MSG("  (The number of pages is reduced to %lld%%.)\n",
> @@ -9790,7 +9816,7 @@ print_mem_usage(void)
>  	pfn_original = info->max_mapnr - pfn_memhole;
> 
>  	pfn_excluded = pfn_zero + pfn_cache + pfn_cache_private
> -	    + pfn_user + pfn_free + pfn_hwpoison;
> +	    + pfn_user + pfn_free + pfn_hwpoison + pfn_offline;
>  	shrinking = (pfn_original - pfn_excluded) * 100;
>  	shrinking = shrinking / pfn_original;
>  	total_size = info->page_size * pfn_original;
> diff --git a/makedumpfile.h b/makedumpfile.h
> index f02f86d..e3a2b29 100644
> --- a/makedumpfile.h
> +++ b/makedumpfile.h
> @@ -1927,6 +1927,7 @@ struct number_table {
>  	long    PG_hwpoison;
> 
>  	long	PAGE_BUDDY_MAPCOUNT_VALUE;
> +	long	PAGE_OFFLINE_MAPCOUNT_VALUE;
>  	long	SECTION_SIZE_BITS;
>  	long	MAX_PHYSMEM_BITS;
>  	long    HUGETLB_PAGE_DTOR;
> --
> 2.17.2
> 

Re: [PATCH v1] makedumpfile: exclude pages that are logically offline

[David Hildenbrand]

On 21.11.18 15:58, Kazuhito Hagio wrote:
> Hi David,
> 
>> Linux marks pages that are logically offline via a page flag (map count).
>> Such pages e.g. include pages infated as part of a balloon driver or
>> pages that were not actually onlined when onlining the whole section.
>>
>> While the hypervisor usually allows to read such inflated memory, we
>> basically read and dump data that is completely irrelevant. Also, this
>> might result in quite some overhead in the hypervisor. In addition,
>> we saw some problems under Hyper-V, whereby we can crash the kernel by
>> dumping, when reading memory of a partially onlined memory segment
>> (for memory added by the Hyper-V balloon driver).
>>
>> Therefore, don't read and dump pages that are marked as being logically
>> offline.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  makedumpfile.c | 34 ++++++++++++++++++++++++++++++----
>>  makedumpfile.h |  1 +
>>  2 files changed, 31 insertions(+), 4 deletions(-)
>>
>> diff --git a/makedumpfile.c b/makedumpfile.c
>> index 8923538..b8bfd4c 100644
>> --- a/makedumpfile.c
>> +++ b/makedumpfile.c
>> @@ -88,6 +88,7 @@ mdf_pfn_t pfn_cache_private;
>>  mdf_pfn_t pfn_user;
>>  mdf_pfn_t pfn_free;
>>  mdf_pfn_t pfn_hwpoison;
>> +mdf_pfn_t pfn_offline;
>>
>>  mdf_pfn_t num_dumped;
>>
>> @@ -249,6 +250,21 @@ isHugetlb(unsigned long dtor)
>>                      && (SYMBOL(free_huge_page) == dtor));
>>  }
>>
>> +static int
>> +isOffline(unsigned long flags, unsigned int _mapcount)
>> +{
>> +	if (NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE) == NOT_FOUND_NUMBER)
>> +		return FALSE;
> 
> This is NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE), isn't it?
> If so, I will correct it when merging.
> 
> Otherwise, looks good to me.
> 
> Thanks!
> Kazu

Indeed,

I will most probably resend either way along with a new mm series!

Thanks a lot!


-- 

Thanks,

David / dhildenb