[PATCH v1 0/3] xen/balloon: PG_offline cleanups

[PATCH v1 0/3] xen/balloon: PG_offline cleanups

[David Hildenbrand]

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(-)

-- 
2.21.0

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

[David Hildenbrand]

Let's simply use balloon_append() directly.

Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 drivers/xen/balloon.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 91cba70b69df..37443c5fda99 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -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;
-- 
2.21.0

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

[Boris Ostrovsky]

On 10/1/19 5:01 AM, David Hildenbrand wrote:
> Let's simply use balloon_append() directly.
>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Signed-off-by: David Hildenbrand <david@redhat.com>

For the series (and your earlier patch)

Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

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

[David Hildenbrand]

On 01.10.19 19:45, Boris Ostrovsky wrote:
> On 10/1/19 5:01 AM, David Hildenbrand wrote:
>> Let's simply use balloon_append() directly.
>>
>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> Cc: Juergen Gross <jgross@suse.com>
>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
> 
> For the series (and your earlier patch)
> 
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

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


-- 

Thanks,

David / dhildenb

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

[Jürgen Groß]

On 02.10.19 09:47, David Hildenbrand wrote:
> On 01.10.19 19:45, Boris Ostrovsky wrote:
>> On 10/1/19 5:01 AM, David Hildenbrand wrote:
>>> Let's simply use balloon_append() directly.
>>>
>>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>>> Cc: Juergen Gross <jgross@suse.com>
>>> Cc: Stefano Stabellini <sstabellini@kernel.org>
>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>
>> For the series (and your earlier patch)
>>
>> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> 
> Thanks! Who's the lucky winner to eventually pick the four patches up? :)

They will be taken through the Xen tree.


Juergen

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

[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 <boris.ostrovsky@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 drivers/xen/balloon.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 37443c5fda99..8c245e99bb06 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -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;
-- 
2.21.0

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

[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 <boris.ostrovsky@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 drivers/xen/balloon.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 8c245e99bb06..5bae515c8e25 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -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
 			/*
-- 
2.21.0