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

2012-08-15 Thread Mel Gorman
On Tue, Aug 14, 2012 at 05:00:49PM -0300, Rafael Aquini wrote:
> On Tue, Aug 14, 2012 at 10:35:25PM +0300, Michael S. Tsirkin wrote:
> > > > > +/* __isolate_lru_page() counterpart for a ballooned page */
> > > > > +bool isolate_balloon_page(struct page *page)
> > > > > +{
> > > > > + if (WARN_ON(!movable_balloon_page(page)))
> > > > 
> > > > Looks like this actually can happen if the page is leaked
> > > > between previous movable_balloon_page and here.
> > > > 
> > > > > + return false;
> > > 
> > > Yes, it surely can happen, and it does not harm to catch it here, print a 
> > > warn and
> > > return.
> > 
> > If it is legal, why warn? For that matter why test here at all?
> >
> 
> As this is a public symbol, and despite the usage we introduce is sane, the 
> warn
> was placed as an insurance policy to let us know about any insane attempt to 
> use
> the procedure in the future. That was due to a nice review nitpick, actually.
> 
> Even though the code already had a test to properly avoid this race you
> mention, I thought that sustaining the warn was a good thing. As I told you,
> despite real, I've never got (un)lucky enough to stumble across that race 
> window
> while testing the patch.
> 
> If your concern is about being too much verbose on logging, under certain
> conditions, perhaps we can change that test to a WARN_ON_ONCE() ?
> 
> Mel, what are your thoughts here?
>  

I viewed it as being defensive programming. VM_BUG_ON would be less
useful as it can be compiled out. If the race can be routinely hit then
multiple warnings is instructive in itself. I have no strong feelings
about this though. I see little harm in making the check but in light of
this conversation add a short comment explaining that the check should
be redundant.

-- 
Mel Gorman
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-08-15 Thread Mel Gorman
On Tue, Aug 14, 2012 at 05:00:49PM -0300, Rafael Aquini wrote:
 On Tue, Aug 14, 2012 at 10:35:25PM +0300, Michael S. Tsirkin wrote:
 +/* __isolate_lru_page() counterpart for a ballooned page */
 +bool isolate_balloon_page(struct page *page)
 +{
 + if (WARN_ON(!movable_balloon_page(page)))

Looks like this actually can happen if the page is leaked
between previous movable_balloon_page and here.

 + return false;
   
   Yes, it surely can happen, and it does not harm to catch it here, print a 
   warn and
   return.
  
  If it is legal, why warn? For that matter why test here at all?
 
 
 As this is a public symbol, and despite the usage we introduce is sane, the 
 warn
 was placed as an insurance policy to let us know about any insane attempt to 
 use
 the procedure in the future. That was due to a nice review nitpick, actually.
 
 Even though the code already had a test to properly avoid this race you
 mention, I thought that sustaining the warn was a good thing. As I told you,
 despite real, I've never got (un)lucky enough to stumble across that race 
 window
 while testing the patch.
 
 If your concern is about being too much verbose on logging, under certain
 conditions, perhaps we can change that test to a WARN_ON_ONCE() ?
 
 Mel, what are your thoughts here?
  

I viewed it as being defensive programming. VM_BUG_ON would be less
useful as it can be compiled out. If the race can be routinely hit then
multiple warnings is instructive in itself. I have no strong feelings
about this though. I see little harm in making the check but in light of
this conversation add a short comment explaining that the check should
be redundant.

-- 
Mel Gorman
SUSE Labs
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-08-14 Thread Michael S. Tsirkin
On Tue, Aug 14, 2012 at 05:00:49PM -0300, Rafael Aquini wrote:
> On Tue, Aug 14, 2012 at 10:35:25PM +0300, Michael S. Tsirkin wrote:
> > > > > +/* __isolate_lru_page() counterpart for a ballooned page */
> > > > > +bool isolate_balloon_page(struct page *page)
> > > > > +{
> > > > > + if (WARN_ON(!movable_balloon_page(page)))
> > > > 
> > > > Looks like this actually can happen if the page is leaked
> > > > between previous movable_balloon_page and here.
> > > > 
> > > > > + return false;
> > > 
> > > Yes, it surely can happen, and it does not harm to catch it here, print a 
> > > warn and
> > > return.
> > 
> > If it is legal, why warn? For that matter why test here at all?
> >
> 
> As this is a public symbol, and despite the usage we introduce is sane, the 
> warn
> was placed as an insurance policy to let us know about any insane attempt to 
> use
> the procedure in the future. That was due to a nice review nitpick, actually.
> 
> Even though the code already had a test to properly avoid this race you
> mention, I thought that sustaining the warn was a good thing. As I told you,
> despite real, I've never got (un)lucky enough to stumble across that race 
> window
> while testing the patch.
> 
> If your concern is about being too much verbose on logging, under certain
> conditions, perhaps we can change that test to a WARN_ON_ONCE() ?
> 
> Mel, what are your thoughts here?
>  
> > > While testing it, I wasn't lucky to see this small window opening, though.


think about it: you see it in log. so you know race triggered.
now what? why is it useful to know this?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-08-14 Thread Rafael Aquini
On Tue, Aug 14, 2012 at 10:48:37PM +0300, Michael S. Tsirkin wrote:
> > 
> > E.g. kvm can emulate hyperv so it could in theory have hyperv balloon.
> > This is mm stuff it is best not to tie it to specific drivers.
> 
> But of course I agree this is not top priority, no need
> to block submission on this, just nice to have.
>
This surely is interesting to look at, in the near future. 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-08-14 Thread Rafael Aquini
On Tue, Aug 14, 2012 at 10:35:25PM +0300, Michael S. Tsirkin wrote:
> > > > +/* __isolate_lru_page() counterpart for a ballooned page */
> > > > +bool isolate_balloon_page(struct page *page)
> > > > +{
> > > > +   if (WARN_ON(!movable_balloon_page(page)))
> > > 
> > > Looks like this actually can happen if the page is leaked
> > > between previous movable_balloon_page and here.
> > > 
> > > > +   return false;
> > 
> > Yes, it surely can happen, and it does not harm to catch it here, print a 
> > warn and
> > return.
> 
> If it is legal, why warn? For that matter why test here at all?
>

As this is a public symbol, and despite the usage we introduce is sane, the warn
was placed as an insurance policy to let us know about any insane attempt to use
the procedure in the future. That was due to a nice review nitpick, actually.

Even though the code already had a test to properly avoid this race you
mention, I thought that sustaining the warn was a good thing. As I told you,
despite real, I've never got (un)lucky enough to stumble across that race window
while testing the patch.

If your concern is about being too much verbose on logging, under certain
conditions, perhaps we can change that test to a WARN_ON_ONCE() ?

Mel, what are your thoughts here?
 
> > While testing it, I wasn't lucky to see this small window opening, though.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-08-14 Thread Michael S. Tsirkin
On Tue, Aug 14, 2012 at 02:44:05PM -0300, Rafael Aquini wrote:
> On Mon, Aug 13, 2012 at 11:26:19AM +0300, Michael S. Tsirkin wrote:
> > > +static inline bool movable_balloon_page(struct page *page)
> > > +{
> > > + return (page->mapping && page->mapping == balloon_mapping);
> > 
> > I am guessing this needs smp_read_barrier_depends, and maybe
> > ACCESS_ONCE ...
> > 
> 
> I'm curious about your guessing here. Could you ellaborate it further, please?

well balloon_mapping can change dynamically.


I think actually rcu is a good fit here.

> 
> > > +#else
> > > +static inline bool isolate_balloon_page(struct page *page) { return 
> > > false; }
> > > +static inline void putback_balloon_page(struct page *page) { return 
> > > false; }
> > > +static inline bool movable_balloon_page(struct page *page) { return 
> > > false; }
> > > +#endif /* (VIRTIO_BALLOON || VIRTIO_BALLOON_MODULE) && CONFIG_COMPACTION 
> > > */
> > > +
> > 
> > This does mean that only one type of balloon is useable at a time.
> > I wonder whether using a flag in address_space structure instead
> > is possible ...
> 
> This means we are only introducing this feature for virtio_balloon by now.
> Despite the flagging address_space stuff is something we surely can look in 
> the
> future, I quite didn't get how we could be using two different types of 
> balloon
> devices at the same time for the same system. Could you ellaborate it a little
> more, please?
> 
> 
> > > +/* __isolate_lru_page() counterpart for a ballooned page */
> > > +bool isolate_balloon_page(struct page *page)
> > > +{
> > > + if (WARN_ON(!movable_balloon_page(page)))
> > 
> > Looks like this actually can happen if the page is leaked
> > between previous movable_balloon_page and here.
> > 
> > > + return false;
> 
> Yes, it surely can happen, and it does not harm to catch it here, print a 
> warn and
> return. While testing it, I wasn't lucky to see this small window opening, 
> though.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-08-14 Thread Michael S. Tsirkin
On Tue, Aug 14, 2012 at 10:35:25PM +0300, Michael S. Tsirkin wrote:
> On Tue, Aug 14, 2012 at 02:44:05PM -0300, Rafael Aquini wrote:
> > On Mon, Aug 13, 2012 at 11:26:19AM +0300, Michael S. Tsirkin wrote:
> > > > +static inline bool movable_balloon_page(struct page *page)
> > > > +{
> > > > +   return (page->mapping && page->mapping == balloon_mapping);
> > > 
> > > I am guessing this needs smp_read_barrier_depends, and maybe
> > > ACCESS_ONCE ...
> > > 
> > 
> > I'm curious about your guessing here. Could you ellaborate it further, 
> > please?
> > 
> > 
> > > > +#else
> > > > +static inline bool isolate_balloon_page(struct page *page) { return 
> > > > false; }
> > > > +static inline void putback_balloon_page(struct page *page) { return 
> > > > false; }
> > > > +static inline bool movable_balloon_page(struct page *page) { return 
> > > > false; }
> > > > +#endif /* (VIRTIO_BALLOON || VIRTIO_BALLOON_MODULE) && 
> > > > CONFIG_COMPACTION */
> > > > +
> > > 
> > > This does mean that only one type of balloon is useable at a time.
> > > I wonder whether using a flag in address_space structure instead
> > > is possible ...
> > 
> > This means we are only introducing this feature for virtio_balloon by now.
> > Despite the flagging address_space stuff is something we surely can look in 
> > the
> > future, I quite didn't get how we could be using two different types of 
> > balloon
> > devices at the same time for the same system. Could you ellaborate it a 
> > little
> > more, please?
> > 
> 
> E.g. kvm can emulate hyperv so it could in theory have hyperv balloon.
> This is mm stuff it is best not to tie it to specific drivers.

But of course I agree this is not top priority, no need
to block submission on this, just nice to have.

> > > > +/* __isolate_lru_page() counterpart for a ballooned page */
> > > > +bool isolate_balloon_page(struct page *page)
> > > > +{
> > > > +   if (WARN_ON(!movable_balloon_page(page)))
> > > 
> > > Looks like this actually can happen if the page is leaked
> > > between previous movable_balloon_page and here.
> > > 
> > > > +   return false;
> > 
> > Yes, it surely can happen, and it does not harm to catch it here, print a 
> > warn and
> > return.
> 
> If it is legal, why warn? For that matter why test here at all?
> 
> > While testing it, I wasn't lucky to see this small window opening, though.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-08-14 Thread Michael S. Tsirkin
On Tue, Aug 14, 2012 at 02:44:05PM -0300, Rafael Aquini wrote:
> On Mon, Aug 13, 2012 at 11:26:19AM +0300, Michael S. Tsirkin wrote:
> > > +static inline bool movable_balloon_page(struct page *page)
> > > +{
> > > + return (page->mapping && page->mapping == balloon_mapping);
> > 
> > I am guessing this needs smp_read_barrier_depends, and maybe
> > ACCESS_ONCE ...
> > 
> 
> I'm curious about your guessing here. Could you ellaborate it further, please?
> 
> 
> > > +#else
> > > +static inline bool isolate_balloon_page(struct page *page) { return 
> > > false; }
> > > +static inline void putback_balloon_page(struct page *page) { return 
> > > false; }
> > > +static inline bool movable_balloon_page(struct page *page) { return 
> > > false; }
> > > +#endif /* (VIRTIO_BALLOON || VIRTIO_BALLOON_MODULE) && CONFIG_COMPACTION 
> > > */
> > > +
> > 
> > This does mean that only one type of balloon is useable at a time.
> > I wonder whether using a flag in address_space structure instead
> > is possible ...
> 
> This means we are only introducing this feature for virtio_balloon by now.
> Despite the flagging address_space stuff is something we surely can look in 
> the
> future, I quite didn't get how we could be using two different types of 
> balloon
> devices at the same time for the same system. Could you ellaborate it a little
> more, please?
> 

E.g. kvm can emulate hyperv so it could in theory have hyperv balloon.
This is mm stuff it is best not to tie it to specific drivers.

> > > +/* __isolate_lru_page() counterpart for a ballooned page */
> > > +bool isolate_balloon_page(struct page *page)
> > > +{
> > > + if (WARN_ON(!movable_balloon_page(page)))
> > 
> > Looks like this actually can happen if the page is leaked
> > between previous movable_balloon_page and here.
> > 
> > > + return false;
> 
> Yes, it surely can happen, and it does not harm to catch it here, print a 
> warn and
> return.

If it is legal, why warn? For that matter why test here at all?

> While testing it, I wasn't lucky to see this small window opening, though.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-08-14 Thread Rafael Aquini
On Mon, Aug 13, 2012 at 11:26:19AM +0300, Michael S. Tsirkin wrote:
> > +static inline bool movable_balloon_page(struct page *page)
> > +{
> > +   return (page->mapping && page->mapping == balloon_mapping);
> 
> I am guessing this needs smp_read_barrier_depends, and maybe
> ACCESS_ONCE ...
> 

I'm curious about your guessing here. Could you ellaborate it further, please?


> > +#else
> > +static inline bool isolate_balloon_page(struct page *page) { return false; 
> > }
> > +static inline void putback_balloon_page(struct page *page) { return false; 
> > }
> > +static inline bool movable_balloon_page(struct page *page) { return false; 
> > }
> > +#endif /* (VIRTIO_BALLOON || VIRTIO_BALLOON_MODULE) && CONFIG_COMPACTION */
> > +
> 
> This does mean that only one type of balloon is useable at a time.
> I wonder whether using a flag in address_space structure instead
> is possible ...

This means we are only introducing this feature for virtio_balloon by now.
Despite the flagging address_space stuff is something we surely can look in the
future, I quite didn't get how we could be using two different types of balloon
devices at the same time for the same system. Could you ellaborate it a little
more, please?


> > +/* __isolate_lru_page() counterpart for a ballooned page */
> > +bool isolate_balloon_page(struct page *page)
> > +{
> > +   if (WARN_ON(!movable_balloon_page(page)))
> 
> Looks like this actually can happen if the page is leaked
> between previous movable_balloon_page and here.
> 
> > +   return false;

Yes, it surely can happen, and it does not harm to catch it here, print a warn 
and
return. While testing it, I wasn't lucky to see this small window opening, 
though.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-08-14 Thread Rafael Aquini
On Mon, Aug 13, 2012 at 11:26:19AM +0300, Michael S. Tsirkin wrote:
  +static inline bool movable_balloon_page(struct page *page)
  +{
  +   return (page-mapping  page-mapping == balloon_mapping);
 
 I am guessing this needs smp_read_barrier_depends, and maybe
 ACCESS_ONCE ...
 

I'm curious about your guessing here. Could you ellaborate it further, please?


  +#else
  +static inline bool isolate_balloon_page(struct page *page) { return false; 
  }
  +static inline void putback_balloon_page(struct page *page) { return false; 
  }
  +static inline bool movable_balloon_page(struct page *page) { return false; 
  }
  +#endif /* (VIRTIO_BALLOON || VIRTIO_BALLOON_MODULE)  CONFIG_COMPACTION */
  +
 
 This does mean that only one type of balloon is useable at a time.
 I wonder whether using a flag in address_space structure instead
 is possible ...

This means we are only introducing this feature for virtio_balloon by now.
Despite the flagging address_space stuff is something we surely can look in the
future, I quite didn't get how we could be using two different types of balloon
devices at the same time for the same system. Could you ellaborate it a little
more, please?


  +/* __isolate_lru_page() counterpart for a ballooned page */
  +bool isolate_balloon_page(struct page *page)
  +{
  +   if (WARN_ON(!movable_balloon_page(page)))
 
 Looks like this actually can happen if the page is leaked
 between previous movable_balloon_page and here.
 
  +   return false;

Yes, it surely can happen, and it does not harm to catch it here, print a warn 
and
return. While testing it, I wasn't lucky to see this small window opening, 
though.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-08-14 Thread Michael S. Tsirkin
On Tue, Aug 14, 2012 at 02:44:05PM -0300, Rafael Aquini wrote:
 On Mon, Aug 13, 2012 at 11:26:19AM +0300, Michael S. Tsirkin wrote:
   +static inline bool movable_balloon_page(struct page *page)
   +{
   + return (page-mapping  page-mapping == balloon_mapping);
  
  I am guessing this needs smp_read_barrier_depends, and maybe
  ACCESS_ONCE ...
  
 
 I'm curious about your guessing here. Could you ellaborate it further, please?
 
 
   +#else
   +static inline bool isolate_balloon_page(struct page *page) { return 
   false; }
   +static inline void putback_balloon_page(struct page *page) { return 
   false; }
   +static inline bool movable_balloon_page(struct page *page) { return 
   false; }
   +#endif /* (VIRTIO_BALLOON || VIRTIO_BALLOON_MODULE)  CONFIG_COMPACTION 
   */
   +
  
  This does mean that only one type of balloon is useable at a time.
  I wonder whether using a flag in address_space structure instead
  is possible ...
 
 This means we are only introducing this feature for virtio_balloon by now.
 Despite the flagging address_space stuff is something we surely can look in 
 the
 future, I quite didn't get how we could be using two different types of 
 balloon
 devices at the same time for the same system. Could you ellaborate it a little
 more, please?
 

E.g. kvm can emulate hyperv so it could in theory have hyperv balloon.
This is mm stuff it is best not to tie it to specific drivers.

   +/* __isolate_lru_page() counterpart for a ballooned page */
   +bool isolate_balloon_page(struct page *page)
   +{
   + if (WARN_ON(!movable_balloon_page(page)))
  
  Looks like this actually can happen if the page is leaked
  between previous movable_balloon_page and here.
  
   + return false;
 
 Yes, it surely can happen, and it does not harm to catch it here, print a 
 warn and
 return.

If it is legal, why warn? For that matter why test here at all?

 While testing it, I wasn't lucky to see this small window opening, though.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-08-14 Thread Michael S. Tsirkin
On Tue, Aug 14, 2012 at 10:35:25PM +0300, Michael S. Tsirkin wrote:
 On Tue, Aug 14, 2012 at 02:44:05PM -0300, Rafael Aquini wrote:
  On Mon, Aug 13, 2012 at 11:26:19AM +0300, Michael S. Tsirkin wrote:
+static inline bool movable_balloon_page(struct page *page)
+{
+   return (page-mapping  page-mapping == balloon_mapping);
   
   I am guessing this needs smp_read_barrier_depends, and maybe
   ACCESS_ONCE ...
   
  
  I'm curious about your guessing here. Could you ellaborate it further, 
  please?
  
  
+#else
+static inline bool isolate_balloon_page(struct page *page) { return 
false; }
+static inline void putback_balloon_page(struct page *page) { return 
false; }
+static inline bool movable_balloon_page(struct page *page) { return 
false; }
+#endif /* (VIRTIO_BALLOON || VIRTIO_BALLOON_MODULE)  
CONFIG_COMPACTION */
+
   
   This does mean that only one type of balloon is useable at a time.
   I wonder whether using a flag in address_space structure instead
   is possible ...
  
  This means we are only introducing this feature for virtio_balloon by now.
  Despite the flagging address_space stuff is something we surely can look in 
  the
  future, I quite didn't get how we could be using two different types of 
  balloon
  devices at the same time for the same system. Could you ellaborate it a 
  little
  more, please?
  
 
 E.g. kvm can emulate hyperv so it could in theory have hyperv balloon.
 This is mm stuff it is best not to tie it to specific drivers.

But of course I agree this is not top priority, no need
to block submission on this, just nice to have.

+/* __isolate_lru_page() counterpart for a ballooned page */
+bool isolate_balloon_page(struct page *page)
+{
+   if (WARN_ON(!movable_balloon_page(page)))
   
   Looks like this actually can happen if the page is leaked
   between previous movable_balloon_page and here.
   
+   return false;
  
  Yes, it surely can happen, and it does not harm to catch it here, print a 
  warn and
  return.
 
 If it is legal, why warn? For that matter why test here at all?
 
  While testing it, I wasn't lucky to see this small window opening, though.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-08-14 Thread Michael S. Tsirkin
On Tue, Aug 14, 2012 at 02:44:05PM -0300, Rafael Aquini wrote:
 On Mon, Aug 13, 2012 at 11:26:19AM +0300, Michael S. Tsirkin wrote:
   +static inline bool movable_balloon_page(struct page *page)
   +{
   + return (page-mapping  page-mapping == balloon_mapping);
  
  I am guessing this needs smp_read_barrier_depends, and maybe
  ACCESS_ONCE ...
  
 
 I'm curious about your guessing here. Could you ellaborate it further, please?

well balloon_mapping can change dynamically.


I think actually rcu is a good fit here.

 
   +#else
   +static inline bool isolate_balloon_page(struct page *page) { return 
   false; }
   +static inline void putback_balloon_page(struct page *page) { return 
   false; }
   +static inline bool movable_balloon_page(struct page *page) { return 
   false; }
   +#endif /* (VIRTIO_BALLOON || VIRTIO_BALLOON_MODULE)  CONFIG_COMPACTION 
   */
   +
  
  This does mean that only one type of balloon is useable at a time.
  I wonder whether using a flag in address_space structure instead
  is possible ...
 
 This means we are only introducing this feature for virtio_balloon by now.
 Despite the flagging address_space stuff is something we surely can look in 
 the
 future, I quite didn't get how we could be using two different types of 
 balloon
 devices at the same time for the same system. Could you ellaborate it a little
 more, please?
 
 
   +/* __isolate_lru_page() counterpart for a ballooned page */
   +bool isolate_balloon_page(struct page *page)
   +{
   + if (WARN_ON(!movable_balloon_page(page)))
  
  Looks like this actually can happen if the page is leaked
  between previous movable_balloon_page and here.
  
   + return false;
 
 Yes, it surely can happen, and it does not harm to catch it here, print a 
 warn and
 return. While testing it, I wasn't lucky to see this small window opening, 
 though.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-08-14 Thread Rafael Aquini
On Tue, Aug 14, 2012 at 10:35:25PM +0300, Michael S. Tsirkin wrote:
+/* __isolate_lru_page() counterpart for a ballooned page */
+bool isolate_balloon_page(struct page *page)
+{
+   if (WARN_ON(!movable_balloon_page(page)))
   
   Looks like this actually can happen if the page is leaked
   between previous movable_balloon_page and here.
   
+   return false;
  
  Yes, it surely can happen, and it does not harm to catch it here, print a 
  warn and
  return.
 
 If it is legal, why warn? For that matter why test here at all?


As this is a public symbol, and despite the usage we introduce is sane, the warn
was placed as an insurance policy to let us know about any insane attempt to use
the procedure in the future. That was due to a nice review nitpick, actually.

Even though the code already had a test to properly avoid this race you
mention, I thought that sustaining the warn was a good thing. As I told you,
despite real, I've never got (un)lucky enough to stumble across that race window
while testing the patch.

If your concern is about being too much verbose on logging, under certain
conditions, perhaps we can change that test to a WARN_ON_ONCE() ?

Mel, what are your thoughts here?
 
  While testing it, I wasn't lucky to see this small window opening, though.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-08-14 Thread Rafael Aquini
On Tue, Aug 14, 2012 at 10:48:37PM +0300, Michael S. Tsirkin wrote:
  
  E.g. kvm can emulate hyperv so it could in theory have hyperv balloon.
  This is mm stuff it is best not to tie it to specific drivers.
 
 But of course I agree this is not top priority, no need
 to block submission on this, just nice to have.

This surely is interesting to look at, in the near future. 
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-08-14 Thread Michael S. Tsirkin
On Tue, Aug 14, 2012 at 05:00:49PM -0300, Rafael Aquini wrote:
 On Tue, Aug 14, 2012 at 10:35:25PM +0300, Michael S. Tsirkin wrote:
 +/* __isolate_lru_page() counterpart for a ballooned page */
 +bool isolate_balloon_page(struct page *page)
 +{
 + if (WARN_ON(!movable_balloon_page(page)))

Looks like this actually can happen if the page is leaked
between previous movable_balloon_page and here.

 + return false;
   
   Yes, it surely can happen, and it does not harm to catch it here, print a 
   warn and
   return.
  
  If it is legal, why warn? For that matter why test here at all?
 
 
 As this is a public symbol, and despite the usage we introduce is sane, the 
 warn
 was placed as an insurance policy to let us know about any insane attempt to 
 use
 the procedure in the future. That was due to a nice review nitpick, actually.
 
 Even though the code already had a test to properly avoid this race you
 mention, I thought that sustaining the warn was a good thing. As I told you,
 despite real, I've never got (un)lucky enough to stumble across that race 
 window
 while testing the patch.
 
 If your concern is about being too much verbose on logging, under certain
 conditions, perhaps we can change that test to a WARN_ON_ONCE() ?
 
 Mel, what are your thoughts here?
  
   While testing it, I wasn't lucky to see this small window opening, though.


think about it: you see it in log. so you know race triggered.
now what? why is it useful to know this?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-08-13 Thread Michael S. Tsirkin
On Fri, Aug 10, 2012 at 02:55:14PM -0300, Rafael Aquini wrote:
> 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 
> ---
>  include/linux/mm.h |  17 
>  mm/compaction.c| 125 
> +
>  mm/migrate.c   |  30 -
>  3 files changed, 152 insertions(+), 20 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 311be90..56cc553 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1662,5 +1662,22 @@ 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)) && defined(CONFIG_COMPACTION)
> +extern bool isolate_balloon_page(struct page *);
> +extern void putback_balloon_page(struct page *);
> +extern struct address_space *balloon_mapping;
> +
> +static inline bool movable_balloon_page(struct page *page)
> +{
> + return (page->mapping && page->mapping == balloon_mapping);

I am guessing this needs smp_read_barrier_depends, and maybe
ACCESS_ONCE ...

> +}
> +
> +#else
> +static inline bool isolate_balloon_page(struct page *page) { return false; }
> +static inline void putback_balloon_page(struct page *page) { return false; }
> +static inline bool movable_balloon_page(struct page *page) { return false; }
> +#endif /* (VIRTIO_BALLOON || VIRTIO_BALLOON_MODULE) && CONFIG_COMPACTION */
> +

This does mean that only one type of balloon is useable at a time.
I wonder whether using a flag in address_space structure instead
is possible ...

>  #endif /* __KERNEL__ */
>  #endif /* _LINUX_MM_H */
> diff --git a/mm/compaction.c b/mm/compaction.c
> index e78cb96..e4e871b 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -14,6 +14,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include "internal.h"
>  
>  #if defined CONFIG_COMPACTION || defined CONFIG_CMA
> @@ -21,6 +22,84 @@
>  #define CREATE_TRACE_POINTS
>  #include 
>  
> +#if defined(CONFIG_VIRTIO_BALLOON) || defined(CONFIG_VIRTIO_BALLOON_MODULE)
> +/*
> + * Balloon pages special page->mapping.
> + * Users must properly allocate and initialize an instance of 
> balloon_mapping,
> + * and set it as the page->mapping for balloon enlisted page instances.
> + * There is no need on utilizing struct address_space locking schemes for
> + * balloon_mapping as, once it gets initialized at balloon driver, it will
> + * remain just like a static reference that helps us on identifying a guest
> + * ballooned page by its mapping, as well as it will keep the 'a_ops' 
> callback
> + * pointers to the functions that will execute the balloon page mobility 
> tasks.
> + *
> + * address_space_operations necessary methods for ballooned pages:
> + *   .migratepage- used to perform balloon's page migration (as is)
> + *   .invalidatepage - used to isolate a page from balloon's page list
> + *   .freepage   - used to reinsert an isolated page to balloon's page 
> list
> + */
> +struct address_space *balloon_mapping;
> +EXPORT_SYMBOL_GPL(balloon_mapping);
> +
> +static inline void __isolate_balloon_page(struct page *page)
> +{
> + page->mapping->a_ops->invalidatepage(page, 0);
> +}
> +
> +static inline void __putback_balloon_page(struct page *page)
> +{
> + page->mapping->a_ops->freepage(page);
> +}
> +
> +/* __isolate_lru_page() counterpart for a ballooned page */
> +bool isolate_balloon_page(struct page *page)
> +{
> + if (WARN_ON(!movable_balloon_page(page)))

Looks like this actually can happen if the page is leaked
between previous movable_balloon_page and here.

> + return false;
> +
> + if (likely(get_page_unless_zero(page))) {
> + /*
> +  * As balloon pages are not isolated from LRU lists, concurrent
> +  * compaction threads can race against page migration functions
> +  * move_to_new_page() & __unmap_and_move().
> +  * In order to avoid having an already isolated balloon page
> +  * being (wrongly) re-isolated while it is under migration,
> +  * lets be sure we have the page lock before proceeding with
> +  * the balloon page isolation steps.
> +  */
> + if (likely(trylock_page(page))) {
> + /*
> +  * A ballooned 

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

2012-08-13 Thread Michael S. Tsirkin
On Fri, Aug 10, 2012 at 02:55:14PM -0300, Rafael Aquini wrote:
 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 aqu...@redhat.com
 ---
  include/linux/mm.h |  17 
  mm/compaction.c| 125 
 +
  mm/migrate.c   |  30 -
  3 files changed, 152 insertions(+), 20 deletions(-)
 
 diff --git a/include/linux/mm.h b/include/linux/mm.h
 index 311be90..56cc553 100644
 --- a/include/linux/mm.h
 +++ b/include/linux/mm.h
 @@ -1662,5 +1662,22 @@ 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))  defined(CONFIG_COMPACTION)
 +extern bool isolate_balloon_page(struct page *);
 +extern void putback_balloon_page(struct page *);
 +extern struct address_space *balloon_mapping;
 +
 +static inline bool movable_balloon_page(struct page *page)
 +{
 + return (page-mapping  page-mapping == balloon_mapping);

I am guessing this needs smp_read_barrier_depends, and maybe
ACCESS_ONCE ...

 +}
 +
 +#else
 +static inline bool isolate_balloon_page(struct page *page) { return false; }
 +static inline void putback_balloon_page(struct page *page) { return false; }
 +static inline bool movable_balloon_page(struct page *page) { return false; }
 +#endif /* (VIRTIO_BALLOON || VIRTIO_BALLOON_MODULE)  CONFIG_COMPACTION */
 +

This does mean that only one type of balloon is useable at a time.
I wonder whether using a flag in address_space structure instead
is possible ...

  #endif /* __KERNEL__ */
  #endif /* _LINUX_MM_H */
 diff --git a/mm/compaction.c b/mm/compaction.c
 index e78cb96..e4e871b 100644
 --- a/mm/compaction.c
 +++ b/mm/compaction.c
 @@ -14,6 +14,7 @@
  #include linux/backing-dev.h
  #include linux/sysctl.h
  #include linux/sysfs.h
 +#include linux/export.h
  #include internal.h
  
  #if defined CONFIG_COMPACTION || defined CONFIG_CMA
 @@ -21,6 +22,84 @@
  #define CREATE_TRACE_POINTS
  #include trace/events/compaction.h
  
 +#if defined(CONFIG_VIRTIO_BALLOON) || defined(CONFIG_VIRTIO_BALLOON_MODULE)
 +/*
 + * Balloon pages special page-mapping.
 + * Users must properly allocate and initialize an instance of 
 balloon_mapping,
 + * and set it as the page-mapping for balloon enlisted page instances.
 + * There is no need on utilizing struct address_space locking schemes for
 + * balloon_mapping as, once it gets initialized at balloon driver, it will
 + * remain just like a static reference that helps us on identifying a guest
 + * ballooned page by its mapping, as well as it will keep the 'a_ops' 
 callback
 + * pointers to the functions that will execute the balloon page mobility 
 tasks.
 + *
 + * address_space_operations necessary methods for ballooned pages:
 + *   .migratepage- used to perform balloon's page migration (as is)
 + *   .invalidatepage - used to isolate a page from balloon's page list
 + *   .freepage   - used to reinsert an isolated page to balloon's page 
 list
 + */
 +struct address_space *balloon_mapping;
 +EXPORT_SYMBOL_GPL(balloon_mapping);
 +
 +static inline void __isolate_balloon_page(struct page *page)
 +{
 + page-mapping-a_ops-invalidatepage(page, 0);
 +}
 +
 +static inline void __putback_balloon_page(struct page *page)
 +{
 + page-mapping-a_ops-freepage(page);
 +}
 +
 +/* __isolate_lru_page() counterpart for a ballooned page */
 +bool isolate_balloon_page(struct page *page)
 +{
 + if (WARN_ON(!movable_balloon_page(page)))

Looks like this actually can happen if the page is leaked
between previous movable_balloon_page and here.

 + return false;
 +
 + if (likely(get_page_unless_zero(page))) {
 + /*
 +  * As balloon pages are not isolated from LRU lists, concurrent
 +  * compaction threads can race against page migration functions
 +  * move_to_new_page()  __unmap_and_move().
 +  * In order to avoid having an already isolated balloon page
 +  * being (wrongly) re-isolated while it is under migration,
 +  * lets be sure we have the page lock before proceeding with
 +  * the balloon page isolation steps.
 +  */
 + if (likely(trylock_page(page))) {
 + /*
 +  * A ballooned page, by default, has just one 

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

2012-08-12 Thread Minchan Kim
On Fri, Aug 10, 2012 at 02:55:14PM -0300, Rafael Aquini wrote:
> 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 
Reviewed-by: Minchan Kim 

-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2012-08-12 Thread Minchan Kim
On Fri, Aug 10, 2012 at 02:55:14PM -0300, Rafael Aquini wrote:
 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 aqu...@redhat.com
Reviewed-by: Minchan Kim minc...@kernel.org

-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v7 1/4] mm: introduce compaction and migration for virtio ballooned pages

2012-08-10 Thread Rafael Aquini
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 
---
 include/linux/mm.h |  17 
 mm/compaction.c| 125 +
 mm/migrate.c   |  30 -
 3 files changed, 152 insertions(+), 20 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 311be90..56cc553 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1662,5 +1662,22 @@ 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)) && defined(CONFIG_COMPACTION)
+extern bool isolate_balloon_page(struct page *);
+extern void putback_balloon_page(struct page *);
+extern struct address_space *balloon_mapping;
+
+static inline bool movable_balloon_page(struct page *page)
+{
+   return (page->mapping && page->mapping == balloon_mapping);
+}
+
+#else
+static inline bool isolate_balloon_page(struct page *page) { return false; }
+static inline void putback_balloon_page(struct page *page) { return false; }
+static inline bool movable_balloon_page(struct page *page) { return false; }
+#endif /* (VIRTIO_BALLOON || VIRTIO_BALLOON_MODULE) && CONFIG_COMPACTION */
+
 #endif /* __KERNEL__ */
 #endif /* _LINUX_MM_H */
diff --git a/mm/compaction.c b/mm/compaction.c
index e78cb96..e4e871b 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "internal.h"
 
 #if defined CONFIG_COMPACTION || defined CONFIG_CMA
@@ -21,6 +22,84 @@
 #define CREATE_TRACE_POINTS
 #include 
 
+#if defined(CONFIG_VIRTIO_BALLOON) || defined(CONFIG_VIRTIO_BALLOON_MODULE)
+/*
+ * Balloon pages special page->mapping.
+ * Users must properly allocate and initialize an instance of balloon_mapping,
+ * and set it as the page->mapping for balloon enlisted page instances.
+ * There is no need on utilizing struct address_space locking schemes for
+ * balloon_mapping as, once it gets initialized at balloon driver, it will
+ * remain just like a static reference that helps us on identifying a guest
+ * ballooned page by its mapping, as well as it will keep the 'a_ops' callback
+ * pointers to the functions that will execute the balloon page mobility tasks.
+ *
+ * address_space_operations necessary methods for ballooned pages:
+ *   .migratepage- used to perform balloon's page migration (as is)
+ *   .invalidatepage - used to isolate a page from balloon's page list
+ *   .freepage   - used to reinsert an isolated page to balloon's page list
+ */
+struct address_space *balloon_mapping;
+EXPORT_SYMBOL_GPL(balloon_mapping);
+
+static inline void __isolate_balloon_page(struct page *page)
+{
+   page->mapping->a_ops->invalidatepage(page, 0);
+}
+
+static inline void __putback_balloon_page(struct page *page)
+{
+   page->mapping->a_ops->freepage(page);
+}
+
+/* __isolate_lru_page() counterpart for a ballooned page */
+bool isolate_balloon_page(struct page *page)
+{
+   if (WARN_ON(!movable_balloon_page(page)))
+   return false;
+
+   if (likely(get_page_unless_zero(page))) {
+   /*
+* As balloon pages are not isolated from LRU lists, concurrent
+* compaction threads can race against page migration functions
+* move_to_new_page() & __unmap_and_move().
+* In order to avoid having an already isolated balloon page
+* being (wrongly) re-isolated while it is under migration,
+* lets be sure we have the page lock before proceeding with
+* the balloon page isolation steps.
+*/
+   if (likely(trylock_page(page))) {
+   /*
+* A ballooned page, by default, has just one refcount.
+* Prevent concurrent compaction threads from isolating
+* an already isolated balloon page.
+*/
+   if (movable_balloon_page(page) &&
+   (page_count(page) == 2)) {
+   __isolate_balloon_page(page);
+   unlock_page(page);
+   return true;
+   }
+   unlock_page(page);
+   }
+   /* Drop 

[PATCH v7 1/4] mm: introduce compaction and migration for virtio ballooned pages

2012-08-10 Thread Rafael Aquini
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 aqu...@redhat.com
---
 include/linux/mm.h |  17 
 mm/compaction.c| 125 +
 mm/migrate.c   |  30 -
 3 files changed, 152 insertions(+), 20 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 311be90..56cc553 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1662,5 +1662,22 @@ 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))  defined(CONFIG_COMPACTION)
+extern bool isolate_balloon_page(struct page *);
+extern void putback_balloon_page(struct page *);
+extern struct address_space *balloon_mapping;
+
+static inline bool movable_balloon_page(struct page *page)
+{
+   return (page-mapping  page-mapping == balloon_mapping);
+}
+
+#else
+static inline bool isolate_balloon_page(struct page *page) { return false; }
+static inline void putback_balloon_page(struct page *page) { return false; }
+static inline bool movable_balloon_page(struct page *page) { return false; }
+#endif /* (VIRTIO_BALLOON || VIRTIO_BALLOON_MODULE)  CONFIG_COMPACTION */
+
 #endif /* __KERNEL__ */
 #endif /* _LINUX_MM_H */
diff --git a/mm/compaction.c b/mm/compaction.c
index e78cb96..e4e871b 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -14,6 +14,7 @@
 #include linux/backing-dev.h
 #include linux/sysctl.h
 #include linux/sysfs.h
+#include linux/export.h
 #include internal.h
 
 #if defined CONFIG_COMPACTION || defined CONFIG_CMA
@@ -21,6 +22,84 @@
 #define CREATE_TRACE_POINTS
 #include trace/events/compaction.h
 
+#if defined(CONFIG_VIRTIO_BALLOON) || defined(CONFIG_VIRTIO_BALLOON_MODULE)
+/*
+ * Balloon pages special page-mapping.
+ * Users must properly allocate and initialize an instance of balloon_mapping,
+ * and set it as the page-mapping for balloon enlisted page instances.
+ * There is no need on utilizing struct address_space locking schemes for
+ * balloon_mapping as, once it gets initialized at balloon driver, it will
+ * remain just like a static reference that helps us on identifying a guest
+ * ballooned page by its mapping, as well as it will keep the 'a_ops' callback
+ * pointers to the functions that will execute the balloon page mobility tasks.
+ *
+ * address_space_operations necessary methods for ballooned pages:
+ *   .migratepage- used to perform balloon's page migration (as is)
+ *   .invalidatepage - used to isolate a page from balloon's page list
+ *   .freepage   - used to reinsert an isolated page to balloon's page list
+ */
+struct address_space *balloon_mapping;
+EXPORT_SYMBOL_GPL(balloon_mapping);
+
+static inline void __isolate_balloon_page(struct page *page)
+{
+   page-mapping-a_ops-invalidatepage(page, 0);
+}
+
+static inline void __putback_balloon_page(struct page *page)
+{
+   page-mapping-a_ops-freepage(page);
+}
+
+/* __isolate_lru_page() counterpart for a ballooned page */
+bool isolate_balloon_page(struct page *page)
+{
+   if (WARN_ON(!movable_balloon_page(page)))
+   return false;
+
+   if (likely(get_page_unless_zero(page))) {
+   /*
+* As balloon pages are not isolated from LRU lists, concurrent
+* compaction threads can race against page migration functions
+* move_to_new_page()  __unmap_and_move().
+* In order to avoid having an already isolated balloon page
+* being (wrongly) re-isolated while it is under migration,
+* lets be sure we have the page lock before proceeding with
+* the balloon page isolation steps.
+*/
+   if (likely(trylock_page(page))) {
+   /*
+* A ballooned page, by default, has just one refcount.
+* Prevent concurrent compaction threads from isolating
+* an already isolated balloon page.
+*/
+   if (movable_balloon_page(page) 
+   (page_count(page) == 2)) {
+   __isolate_balloon_page(page);
+   unlock_page(page);
+   return true;
+   }
+