Skip to contentSkip to conversation

xen/balloon: PG_offline cleanups

PATCH In review David Hildenbrand 3 participants 7 messages Oct 1, 2019 – Oct 2, 2019

Subsystems (2)
Memory management Xen hypervisor interface
lore
Share & tools
lore

David Hildenbrand

author
#0 lore raw

cover xen/balloon: PG_offline cleanups

Some cleanups based on top of:
[PATCH v1] xen/balloon: Set pages PageOffline() in balloon_add_region()

Make the PG_offline less error prone, by simply setting PG_offline when they enter the page list and clearing PG_offline when they leave the page list.

Only compile-tested.

David Hildenbrand (3):
xen/balloon: Drop __balloon_append() xen/balloon: Mark pages PG_offline in balloon_append() xen/balloon: Clear PG_offline in balloon_retrieve()

 drivers/xen/balloon.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)
Signature · 1 line
2.21.0
author
#1 lore raw

PATCH 1/3 xen/balloon: Drop __balloon_append()

Let's simply use balloon_append() directly.

Cc Boris Ostrovsky <[email protected]>
Cc Juergen Gross <[email protected]>
Cc Stefano Stabellini <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
---
 drivers/xen/balloon.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)
drivers/xen/balloon.c +3 -8
index 91cba70b69df..37443c5fda99 100644
@@ -156,7 +156,7 @@ static DECLARE_DELAYED_WORK(balloon_worker, balloon_process);
 	(GFP_HIGHUSER | __GFP_NOWARN | __GFP_NORETRY | __GFP_NOMEMALLOC)
 
 /* balloon_append: add the given page to the balloon. */
-static void __balloon_append(struct page *page)
+static void balloon_append(struct page *page)
 {
 	/* Lowmem is re-populated first, so highmem pages go at list tail. */
 	if (PageHighMem(page)) {

@@ -169,11 +169,6 @@ static void __balloon_append(struct page *page)
 	wake_up(&balloon_wq);
 }
 
-static void balloon_append(struct page *page)
-{
-	__balloon_append(page);
-}
-
 /* balloon_retrieve: rescue a page from the balloon, if it is not empty. */
 static struct page *balloon_retrieve(bool require_lowmem)
 {

@@ -378,7 +373,7 @@ static void xen_online_page(struct page *page, unsigned int order)
 		p = pfn_to_page(start_pfn + i);
 		__online_page_set_limits(p);
 		__SetPageOffline(p);
-		__balloon_append(p);
+		balloon_append(p);
 	}
 	mutex_unlock(&balloon_mutex);
 }

@@ -689,7 +684,7 @@ static void __init balloon_add_region(unsigned long start_pfn,
 		   include the boot-time balloon extension, so
 		   don't subtract from it. */
 		__SetPageOffline(page);
-		__balloon_append(page);
+		balloon_append(page);
 	}
 
 	balloon_stats.total_pages += extra_pfn_end - start_pfn;
Signature · 1 line
2.21.0
author
#3 lore raw

Re: [PATCH v1 1/3] xen/balloon: Drop __balloon_append()

Quoted 11 lines · Boris Ostrovsky
Quoted 6 lines · David Hildenbrand

Let's simply use balloon_append() directly.

Cc Boris Ostrovsky <[email protected]>
Cc Juergen Gross <[email protected]>
Cc Stefano Stabellini <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>

For the series (and your earlier patch)

Reviewed-by Boris Ostrovsky <[email protected]>

Thanks! Who's the lucky winner to eventually pick the four patches up? :)

Signature · 3 lines
Thanks,

David / dhildenb
maintainer
#4 lore raw

Re: [PATCH v1 1/3] xen/balloon: Drop __balloon_append()

Quoted 9 lines · David Hildenbrand
Quoted 8 lines · Boris Ostrovsky
Quoted 6 lines · David Hildenbrand

Let's simply use balloon_append() directly.

Cc Boris Ostrovsky <[email protected]>
Cc Juergen Gross <[email protected]>
Cc Stefano Stabellini <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
Quoted 5 lines
Quoted 3 lines

For the series (and your earlier patch)

Reviewed-by Boris Ostrovsky <[email protected]>

Thanks! Who's the lucky winner to eventually pick the four patches up? :)

They will be taken through the Xen tree.

Juergen

author
#5 lore raw

PATCH 2/3 xen/balloon: Mark pages PG_offline in balloon_append()

In reply to David Hildenbrand

Let's move the __SetPageOffline() call which all callers perform into balloon_append().

In bp_state decrease_reservation(), pages are now marked PG_offline a little later than before, however, this should not matter for XEN.

Suggested-by Boris Ostrovsky <[email protected]>
Cc Boris Ostrovsky <[email protected]>
Cc Juergen Gross <[email protected]>
Cc Stefano Stabellini <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
---
 drivers/xen/balloon.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)
drivers/xen/balloon.c +4 -9
index 37443c5fda99..8c245e99bb06 100644
@@ -158,6 +158,8 @@ static DECLARE_DELAYED_WORK(balloon_worker, balloon_process);
 /* balloon_append: add the given page to the balloon. */
 static void balloon_append(struct page *page)
 {
+	__SetPageOffline(page);
+
 	/* Lowmem is re-populated first, so highmem pages go at list tail. */
 	if (PageHighMem(page)) {
 		list_add_tail(&page->lru, &ballooned_pages);

@@ -372,7 +374,6 @@ static void xen_online_page(struct page *page, 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);

@@ -466,7 +467,6 @@ 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);

@@ -648,10 +648,8 @@ void free_xenballooned_pages(int nr_pages, struct page **pages)
 	mutex_lock(&balloon_mutex);
 
 	for (i = 0; i < nr_pages; i++) {
-		if (pages[i]) {
-			__SetPageOffline(pages[i]);
+		if (pages[i])
 			balloon_append(pages[i]);
-		}
 	}
 
 	balloon_stats.target_unpopulated -= nr_pages;

@@ -669,7 +667,6 @@ static void __init balloon_add_region(unsigned long start_pfn,
 				      unsigned long pages)
 {
 	unsigned long pfn, extra_pfn_end;
-	struct page *page;
 
 	/*
 	 * If the amount of usable memory has been limited (e.g., with

@@ -679,12 +676,10 @@ static void __init balloon_add_region(unsigned long start_pfn,
 	extra_pfn_end = min(max_pfn, start_pfn + pages);
 
 	for (pfn = start_pfn; pfn < extra_pfn_end; pfn++) {
-		page = pfn_to_page(pfn);
 		/* totalram_pages and totalhigh_pages do not
 		   include the boot-time balloon extension, so
 		   don't subtract from it. */
-		__SetPageOffline(page);
-		balloon_append(page);
+		balloon_append(pfn_to_page(pfn));
 	}
 
 	balloon_stats.total_pages += extra_pfn_end - start_pfn;
Signature · 1 line
2.21.0
author
#6 lore raw

PATCH 3/3 xen/balloon: Clear PG_offline in balloon_retrieve()

In reply to David Hildenbrand

Let's move the clearing to balloon_retrieve(). In
bp_state increase_reservation(), we now clear the flag a little earlier than before, however, this should not matter for XEN.

Suggested-by Boris Ostrovsky <[email protected]>
Cc Boris Ostrovsky <[email protected]>
Cc Juergen Gross <[email protected]>
Cc Stefano Stabellini <[email protected]>
Signed-off-by David Hildenbrand <[email protected]>
---
 drivers/xen/balloon.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/xen/balloon.c +1 -2
index 8c245e99bb06..5bae515c8e25 100644
@@ -189,6 +189,7 @@ static struct page *balloon_retrieve(bool require_lowmem)
 	else
 		balloon_stats.balloon_low--;
 
+	__ClearPageOffline(page);
 	return page;
 }
 

@@ -440,7 +441,6 @@ 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);
 	}
 

@@ -606,7 +606,6 @@ int alloc_xenballooned_pages(int nr_pages, struct page **pages)
 	while (pgno < nr_pages) {
 		page = balloon_retrieve(true);
 		if (page) {
-			__ClearPageOffline(page);
 			pages[pgno++] = page;
 #ifdef CONFIG_XEN_HAVE_PVMMU
 			/*
Signature · 1 line
2.21.0