Re: [PATCH v11 3/7] mm: introduce a common interface for balloon pages mobility

2012-11-09 Thread Rafael Aquini 
On Fri, Nov 09, 2012 at 04:23:27PM +, Mel Gorman wrote:
 On Fri, Nov 09, 2012 at 12:53:22PM -0200, Rafael Aquini wrote:
   SNIP
   If you get the barrier issue sorted out then feel free to add
   
   Acked-by: Mel Gorman m...@csn.ul.ie
   
  
  I believe we can drop the barriers stuff, as the locking scheme is now 
  provinding
  enough protection against collisions between isolation page scanning and
  balloon_leak() page release (the major concern that has lead to the barriers
  originally)
  
  I'll refactor this patch with no barriers and ensure a better commentary on 
  the
  aforementioned locking scheme and resubmit, if it's OK to everyone
  
 
 Sounds good to me. When they are dropped again feel free to stick my ack
 on for the compaction and migration parts. The virtio aspects are up to
 someone else :)
 

Andrew,

If we get no further objections raised on dropping those barriers, would you
like me to resubmit the whole series rebased on the latest -next, 
or just this (new) refactored patch?

-- Rafael
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v11 3/7] mm: introduce a common interface for balloon pages mobility

2012-11-07 Thread Andrew Morton 
On Wed,  7 Nov 2012 01:05:50 -0200
Rafael Aquini aqu...@redhat.com 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 a common interface to help a balloon driver on
 making its page set movable to compaction, and thus allowing the system
 to better leverage the compation efforts on memory defragmentation.


mm/migrate.c: In function 'unmap_and_move':
mm/migrate.c:899: error: 'COMPACTBALLOONRELEASED' undeclared (first use in this 
function)
mm/migrate.c:899: error: (Each undeclared identifier is reported only once
mm/migrate.c:899: error: for each function it appears in.)

You've been bad - you didn't test with your feature disabled. 
Please do that.  And not just compilation testing.


We can fix this one with a sucky macro.  I think that's better than
unconditionally defining the enums.

--- 
a/include/linux/balloon_compaction.h~mm-introduce-a-common-interface-for-balloon-pages-mobility-fix
+++ a/include/linux/balloon_compaction.h
@@ -207,10 +207,8 @@ static inline gfp_t balloon_mapping_gfp_
return GFP_HIGHUSER;
 }
 
-static inline void balloon_event_count(enum vm_event_item item)
-{
-   return;
-}
+/* A macro, to avoid generating references to the undefined COMPACTBALLOON* */
+#define balloon_event_count(item) do { } while (0)
 
 static inline bool balloon_compaction_check(void)
 {

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v11 3/7] mm: introduce a common interface for balloon pages mobility

2012-11-07 Thread Rafael Aquini 
On Wed, Nov 07, 2012 at 01:02:07PM -0800, Andrew Morton wrote:
 On Wed,  7 Nov 2012 01:05:50 -0200
 Rafael Aquini aqu...@redhat.com 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 a common interface to help a balloon driver on
  making its page set movable to compaction, and thus allowing the system
  to better leverage the compation efforts on memory defragmentation.
 
 
 mm/migrate.c: In function 'unmap_and_move':
 mm/migrate.c:899: error: 'COMPACTBALLOONRELEASED' undeclared (first use in 
 this function)
 mm/migrate.c:899: error: (Each undeclared identifier is reported only once
 mm/migrate.c:899: error: for each function it appears in.)
 
 You've been bad - you didn't test with your feature disabled. 
 Please do that.  And not just compilation testing.


Gasp... Shame on me, indeed. balloon_event_count() was a macro and I had it
tested a couple of review rounds earlier. You requested me to get rid of
preprocessor pirotech, and I did but I miserably failed on re-testing it.
 
I'm pretty sure Santa is not going to visit me this year.

Do you want me to resubmit this?

 
 We can fix this one with a sucky macro.  I think that's better than
 unconditionally defining the enums.
 
 --- 
 a/include/linux/balloon_compaction.h~mm-introduce-a-common-interface-for-balloon-pages-mobility-fix
 +++ a/include/linux/balloon_compaction.h
 @@ -207,10 +207,8 @@ static inline gfp_t balloon_mapping_gfp_
   return GFP_HIGHUSER;
  }
  
 -static inline void balloon_event_count(enum vm_event_item item)
 -{
 - return;
 -}
 +/* A macro, to avoid generating references to the undefined COMPACTBALLOON* 
 */
 +#define balloon_event_count(item) do { } while (0)
  
  static inline bool balloon_compaction_check(void)
  {
 
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization