| Message ID | 20181119101616.8901-6-david@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1,1/8] mm: balloon: update comment about isolation/migration/compaction | expand |
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 > >
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
> >> --- > >> 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 >
> > >> --- > > >> 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>
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);
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(-)