Rafael Aquini | 6 Aug 2012 15:56
Picon
Favicon

[PATCH v5 0/3] make balloon pages movable by compaction

Memory fragmentation introduced by ballooning might reduce significantly
the number of 2MB contiguous memory blocks that can be used within a guest,
thus imposing performance penalties associated with the reduced number of
transparent huge pages that could be used by the guest workload.

This patch-set follows the main idea discussed at 2012 LSFMMS session:
"Ballooning for transparent huge pages" -- http://lwn.net/Articles/490114/
to introduce the required changes to the virtio_balloon driver, as well as
the changes to the core compaction & migration bits, in order to make those
subsystems aware of ballooned pages and allow memory balloon pages become
movable within a guest, thus avoiding the aforementioned fragmentation issue

Rafael Aquini (3):
  mm: introduce compaction and migration for virtio ballooned pages
  virtio_balloon: introduce migration primitives to balloon pages
  mm: add vm event counters for balloon pages compaction

 drivers/virtio/virtio_balloon.c | 139 +++++++++++++++++++++++++++++++++++++---
 include/linux/mm.h              |  26 ++++++++
 include/linux/virtio_balloon.h  |   4 ++
 include/linux/vm_event_item.h   |   2 +
 mm/compaction.c                 | 131 +++++++++++++++++++++++++++++++------
 mm/migrate.c                    |  34 +++++++++-
 mm/vmstat.c                     |   4 ++
 7 files changed, 312 insertions(+), 28 deletions(-)

Change log:
v5:
 * address Andrew Morton's review comments on the patch series;
 * address a couple extra nitpick suggestions on PATCH 01 (Minchan);
(Continue reading)

Rafael Aquini | 6 Aug 2012 15:56
Picon
Favicon

[PATCH v5 1/3] mm: introduce compaction and migration for virtio ballooned pages

Memory fragmentation introduced by ballooning might reduce significantly
the number of 2MB contiguous memory blocks that can be used within a guest,
thus imposing performance penalties associated with the reduced number of
transparent huge pages that could be used by the guest workload.

This patch introduces the helper functions as well as the necessary changes
to teach compaction and migration bits how to cope with pages which are
part of a guest memory balloon, in order to make them movable by memory
compaction procedures.

Signed-off-by: Rafael Aquini <aquini <at> redhat.com>
---
 include/linux/mm.h |  26 +++++++++++
 mm/compaction.c    | 130 +++++++++++++++++++++++++++++++++++++++++++++--------
 mm/migrate.c       |  32 ++++++++++++-
 3 files changed, 168 insertions(+), 20 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 311be90..01a2dc9 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
 <at>  <at>  -1662,5 +1662,31  <at>  <at>  static inline unsigned int debug_guardpage_minorder(void) { return 0; }
 static inline bool page_is_guard(struct page *page) { return false; }
 #endif /* CONFIG_DEBUG_PAGEALLOC */

+#if (defined(CONFIG_VIRTIO_BALLOON) || defined(CONFIG_VIRTIO_BALLOON_MODULE))
+extern bool isolate_balloon_page(struct page *);
+extern bool putback_balloon_page(struct page *);
+extern struct address_space *balloon_mapping;
+
(Continue reading)

Rik van Riel | 6 Aug 2012 20:38
Picon
Favicon

Re: [PATCH v5 1/3] mm: introduce compaction and migration for virtio ballooned pages

On 08/06/2012 09:56 AM, Rafael Aquini wrote:

>  <at>  <at>  -846,6 +861,21  <at>  <at>  static int unmap_and_move(new_page_t get_new_page, unsigned long private,
>   			goto out;
>
>   	rc = __unmap_and_move(page, newpage, force, offlining, mode);
> +
> +	if (unlikely(is_balloon_page(newpage)&&
> +		     balloon_compaction_enabled())) {

Could that be collapsed into one movable_balloon_page(newpage) function
call?

--

-- 
All rights reversed
Rafael Aquini | 6 Aug 2012 21:00
Picon
Favicon

Re: [PATCH v5 1/3] mm: introduce compaction and migration for virtio ballooned pages

On Mon, Aug 06, 2012 at 02:38:23PM -0400, Rik van Riel wrote:
> On 08/06/2012 09:56 AM, Rafael Aquini wrote:
> 
> > <at>  <at>  -846,6 +861,21  <at>  <at>  static int unmap_and_move(new_page_t get_new_page, unsigned long private,
> >  			goto out;
> >
> >  	rc = __unmap_and_move(page, newpage, force, offlining, mode);
> >+
> >+	if (unlikely(is_balloon_page(newpage)&&
> >+		     balloon_compaction_enabled())) {
> 
> Could that be collapsed into one movable_balloon_page(newpage) function
> call?
> 
Keeping is_balloon_page() as is, and itroducing this new movable_balloon_page()
function call, or just doing a plain rename, as Andrew has first suggested?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo <at> kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont <at> kvack.org"> email <at> kvack.org </a>

Rik van Riel | 6 Aug 2012 21:06
Picon
Favicon

Re: [PATCH v5 1/3] mm: introduce compaction and migration for virtio ballooned pages

On 08/06/2012 03:00 PM, Rafael Aquini wrote:
> On Mon, Aug 06, 2012 at 02:38:23PM -0400, Rik van Riel wrote:
>> On 08/06/2012 09:56 AM, Rafael Aquini wrote:
>>
>>>  <at>  <at>  -846,6 +861,21  <at>  <at>  static int unmap_and_move(new_page_t get_new_page, unsigned long private,
>>>   			goto out;
>>>
>>>   	rc = __unmap_and_move(page, newpage, force, offlining, mode);
>>> +
>>> +	if (unlikely(is_balloon_page(newpage)&&
>>> +		     balloon_compaction_enabled())) {
>>
>> Could that be collapsed into one movable_balloon_page(newpage) function
>> call?
>>
> Keeping is_balloon_page() as is, and itroducing this new movable_balloon_page()
> function call, or just doing a plain rename, as Andrew has first suggested?

Just a plain rename would work.

> +static inline bool is_balloon_page(struct page *page)
> +{
> +	return (page->mapping && page->mapping == balloon_mapping);
> +}

As an aside, since you are only comparing page->mapping and
not dereferencing it, it can be simplified to just:

	return (page->mapping == balloon_mapping);

(Continue reading)

Rafael Aquini | 6 Aug 2012 21:24
Picon
Favicon

Re: [PATCH v5 1/3] mm: introduce compaction and migration for virtio ballooned pages

On Mon, Aug 06, 2012 at 03:06:49PM -0400, Rik van Riel wrote:
> 
> Just a plain rename would work.
>
Ok, I will rename it.

> >+static inline bool is_balloon_page(struct page *page)
> >+{
> >+	return (page->mapping && page->mapping == balloon_mapping);
> >+}
> 
> As an aside, since you are only comparing page->mapping and
> not dereferencing it, it can be simplified to just:
> 
> 	return (page->mapping == balloon_mapping);
> 
We really need both comparisons to avoid potential NULL pointer dereferences at
__isolate_balloon_page() & __putback_balloon_page() while running at bare metal
with no balloon driver loaded, since balloon_mapping itself is a pointer which
each balloon driver can set to its own structure. 

Thanks, Rik, for taking the time to look at this patch and provide (always)
valuable feedback.

I'll shortly respin a v6 with your suggestions.

-- Rafael

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
(Continue reading)

Rafael Aquini | 6 Aug 2012 21:24
Picon
Favicon

Re: [PATCH v5 1/3] mm: introduce compaction and migration for virtio ballooned pages

On Mon, Aug 06, 2012 at 03:06:49PM -0400, Rik van Riel wrote:
> 
> Just a plain rename would work.
>
Ok, I will rename it.

> >+static inline bool is_balloon_page(struct page *page)
> >+{
> >+	return (page->mapping && page->mapping == balloon_mapping);
> >+}
> 
> As an aside, since you are only comparing page->mapping and
> not dereferencing it, it can be simplified to just:
> 
> 	return (page->mapping == balloon_mapping);
> 
We really need both comparisons to avoid potential NULL pointer dereferences at
__isolate_balloon_page() & __putback_balloon_page() while running at bare metal
with no balloon driver loaded, since balloon_mapping itself is a pointer which
each balloon driver can set to its own structure. 

Thanks, Rik, for taking the time to look at this patch and provide (always)
valuable feedback.

I'll shortly respin a v6 with your suggestions.

-- Rafael
Rik van Riel | 6 Aug 2012 21:06
Picon
Favicon

Re: [PATCH v5 1/3] mm: introduce compaction and migration for virtio ballooned pages

On 08/06/2012 03:00 PM, Rafael Aquini wrote:
> On Mon, Aug 06, 2012 at 02:38:23PM -0400, Rik van Riel wrote:
>> On 08/06/2012 09:56 AM, Rafael Aquini wrote:
>>
>>>  <at>  <at>  -846,6 +861,21  <at>  <at>  static int unmap_and_move(new_page_t get_new_page, unsigned long private,
>>>   			goto out;
>>>
>>>   	rc = __unmap_and_move(page, newpage, force, offlining, mode);
>>> +
>>> +	if (unlikely(is_balloon_page(newpage)&&
>>> +		     balloon_compaction_enabled())) {
>>
>> Could that be collapsed into one movable_balloon_page(newpage) function
>> call?
>>
> Keeping is_balloon_page() as is, and itroducing this new movable_balloon_page()
> function call, or just doing a plain rename, as Andrew has first suggested?

Just a plain rename would work.

> +static inline bool is_balloon_page(struct page *page)
> +{
> +	return (page->mapping && page->mapping == balloon_mapping);
> +}

As an aside, since you are only comparing page->mapping and
not dereferencing it, it can be simplified to just:

	return (page->mapping == balloon_mapping);

(Continue reading)

Rafael Aquini | 6 Aug 2012 21:00
Picon
Favicon

Re: [PATCH v5 1/3] mm: introduce compaction and migration for virtio ballooned pages

On Mon, Aug 06, 2012 at 02:38:23PM -0400, Rik van Riel wrote:
> On 08/06/2012 09:56 AM, Rafael Aquini wrote:
> 
> > <at>  <at>  -846,6 +861,21  <at>  <at>  static int unmap_and_move(new_page_t get_new_page, unsigned long private,
> >  			goto out;
> >
> >  	rc = __unmap_and_move(page, newpage, force, offlining, mode);
> >+
> >+	if (unlikely(is_balloon_page(newpage)&&
> >+		     balloon_compaction_enabled())) {
> 
> Could that be collapsed into one movable_balloon_page(newpage) function
> call?
> 
Keeping is_balloon_page() as is, and itroducing this new movable_balloon_page()
function call, or just doing a plain rename, as Andrew has first suggested?
Rafael Aquini | 6 Aug 2012 15:56
Picon
Favicon

[PATCH v5 2/3] virtio_balloon: introduce migration primitives to balloon pages

Memory fragmentation introduced by ballooning might reduce significantly
the number of 2MB contiguous memory blocks that can be used within a guest,
thus imposing performance penalties associated with the reduced number of
transparent huge pages that could be used by the guest workload.

Besides making balloon pages movable at allocation time and introducing
the necessary primitives to perform balloon page migration/compaction,
this patch also introduces the following locking scheme to provide the
proper synchronization and protection for struct virtio_balloon elements
against concurrent accesses due to parallel operations introduced by
memory compaction / page migration.
 - balloon_lock (mutex) : synchronizes the access demand to elements of
			  struct virtio_balloon and its queue operations;
 - pages_lock (spinlock): special protection to balloon pages list against
			  concurrent list handling operations;

Signed-off-by: Rafael Aquini <aquini <at> redhat.com>
---
 drivers/virtio/virtio_balloon.c | 138 +++++++++++++++++++++++++++++++++++++---
 include/linux/virtio_balloon.h  |   4 ++
 2 files changed, 134 insertions(+), 8 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 0908e60..7c937a0 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
 <at>  <at>  -27,6 +27,7  <at>  <at> 
 #include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/module.h>
(Continue reading)

Rafael Aquini | 6 Aug 2012 15:56
Picon
Favicon

[PATCH v5 3/3] mm: add vm event counters for balloon pages compaction

This patch is only for testing report purposes and shall be dropped in case of
the rest of this patchset getting accepted for merging.

Signed-off-by: Rafael Aquini <aquini <at> redhat.com>
---
 drivers/virtio/virtio_balloon.c | 1 +
 include/linux/vm_event_item.h   | 2 ++
 mm/compaction.c                 | 1 +
 mm/migrate.c                    | 6 ++++--
 mm/vmstat.c                     | 4 ++++
 5 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 7c937a0..b8f7ea5 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
 <at>  <at>  -414,6 +414,7  <at>  <at>  int virtballoon_migratepage(struct address_space *mapping,

 	mutex_unlock(&balloon_lock);

+	count_vm_event(COMPACTBALLOONMIGRATED);
 	return 0;
 }

diff --git a/include/linux/vm_event_item.h b/include/linux/vm_event_item.h
index 57f7b10..a632a5d 100644
--- a/include/linux/vm_event_item.h
+++ b/include/linux/vm_event_item.h
 <at>  <at>  -41,6 +41,8  <at>  <at>  enum vm_event_item { PGPGIN, PGPGOUT, PSWPIN, PSWPOUT,
 #ifdef CONFIG_COMPACTION
(Continue reading)

Rafael Aquini | 6 Aug 2012 15:56
Picon
Favicon

[PATCH v5 2/3] virtio_balloon: introduce migration primitives to balloon pages

Memory fragmentation introduced by ballooning might reduce significantly
the number of 2MB contiguous memory blocks that can be used within a guest,
thus imposing performance penalties associated with the reduced number of
transparent huge pages that could be used by the guest workload.

Besides making balloon pages movable at allocation time and introducing
the necessary primitives to perform balloon page migration/compaction,
this patch also introduces the following locking scheme to provide the
proper synchronization and protection for struct virtio_balloon elements
against concurrent accesses due to parallel operations introduced by
memory compaction / page migration.
 - balloon_lock (mutex) : synchronizes the access demand to elements of
			  struct virtio_balloon and its queue operations;
 - pages_lock (spinlock): special protection to balloon pages list against
			  concurrent list handling operations;

Signed-off-by: Rafael Aquini <aquini <at> redhat.com>
---
 drivers/virtio/virtio_balloon.c | 138 +++++++++++++++++++++++++++++++++++++---
 include/linux/virtio_balloon.h  |   4 ++
 2 files changed, 134 insertions(+), 8 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 0908e60..7c937a0 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
 <at>  <at>  -27,6 +27,7  <at>  <at> 
 #include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/module.h>
(Continue reading)

Rafael Aquini | 6 Aug 2012 15:56
Picon
Favicon

[PATCH v5 1/3] mm: introduce compaction and migration for virtio ballooned pages

Memory fragmentation introduced by ballooning might reduce significantly
the number of 2MB contiguous memory blocks that can be used within a guest,
thus imposing performance penalties associated with the reduced number of
transparent huge pages that could be used by the guest workload.

This patch introduces the helper functions as well as the necessary changes
to teach compaction and migration bits how to cope with pages which are
part of a guest memory balloon, in order to make them movable by memory
compaction procedures.

Signed-off-by: Rafael Aquini <aquini <at> redhat.com>
---
 include/linux/mm.h |  26 +++++++++++
 mm/compaction.c    | 130 +++++++++++++++++++++++++++++++++++++++++++++--------
 mm/migrate.c       |  32 ++++++++++++-
 3 files changed, 168 insertions(+), 20 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 311be90..01a2dc9 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
 <at>  <at>  -1662,5 +1662,31  <at>  <at>  static inline unsigned int debug_guardpage_minorder(void) { return 0; }
 static inline bool page_is_guard(struct page *page) { return false; }
 #endif /* CONFIG_DEBUG_PAGEALLOC */

+#if (defined(CONFIG_VIRTIO_BALLOON) || defined(CONFIG_VIRTIO_BALLOON_MODULE))
+extern bool isolate_balloon_page(struct page *);
+extern bool putback_balloon_page(struct page *);
+extern struct address_space *balloon_mapping;
+
(Continue reading)

Rafael Aquini | 6 Aug 2012 15:56
Picon
Favicon

[PATCH v5 3/3] mm: add vm event counters for balloon pages compaction

This patch is only for testing report purposes and shall be dropped in case of
the rest of this patchset getting accepted for merging.

Signed-off-by: Rafael Aquini <aquini <at> redhat.com>
---
 drivers/virtio/virtio_balloon.c | 1 +
 include/linux/vm_event_item.h   | 2 ++
 mm/compaction.c                 | 1 +
 mm/migrate.c                    | 6 ++++--
 mm/vmstat.c                     | 4 ++++
 5 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 7c937a0..b8f7ea5 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
 <at>  <at>  -414,6 +414,7  <at>  <at>  int virtballoon_migratepage(struct address_space *mapping,

 	mutex_unlock(&balloon_lock);

+	count_vm_event(COMPACTBALLOONMIGRATED);
 	return 0;
 }

diff --git a/include/linux/vm_event_item.h b/include/linux/vm_event_item.h
index 57f7b10..a632a5d 100644
--- a/include/linux/vm_event_item.h
+++ b/include/linux/vm_event_item.h
 <at>  <at>  -41,6 +41,8  <at>  <at>  enum vm_event_item { PGPGIN, PGPGOUT, PSWPIN, PSWPOUT,
 #ifdef CONFIG_COMPACTION
(Continue reading)


Gmane