Re: [PATCH v12 4/7] mm: introduce compaction and migration for ballooned pages

[Rafael Aquini <aquini@redhat.com>]

On Thu, Nov 22, 2012 at 09:19:15AM -0500, Sasha Levin wrote:
> And managed to reproduce it only once through last night, here is the dump I got
> before the oops:
> 
> [ 2760.356820] page:ffffea0000d00e00 count:1 mapcount:-2147287036 mapping:00000000000004f4 index:0xd00e00000003
> [ 2760.362354] page flags: 0x350000000001800(private|private_2)
> 

Thanks alot for following up this one Sasha.


We're stumbling across a private page -- seems something in your setup is doing
this particular usage, and that's probably why I'm not seeing the same here.

Regardless being a particular case or not, we shouldn't be poking at that
private page, so I figured the tests I'm doing at balloon_page_movable() are
incomplete and dumb.

Perhaps, a better way to proceed here would be assuring the NR_PAGEFLAGS
rightmost bits from page->flags are all cleared, as this is the state a page
coming from buddy to the balloon list will be, and this is the state the balloon
page flags will be kept as long as it lives as such (we don't play with any flag
at balloon level).


Here goes what I'll propose after you confirm it doesn't trigger your crash
anymore, as it simplifies the code and reduces the testing battery @
balloon_page_movable() -- ballooned pages have no flags set, 1 refcount and 0
mapcount, always.


Could you give this a try?

Thank you!

---
diff --git a/include/linux/balloon_compaction.h b/include/linux/balloon_compacti
index e339dd9..44ad50f 100644
--- a/include/linux/balloon_compaction.h
+++ b/include/linux/balloon_compaction.h
@@ -101,6 +101,12 @@ static inline bool __is_movable_balloon_page(struct page *p
        return mapping_balloon(mapping);
 }
 
+#define PAGE_FLAGS_MASK       ((1UL << NR_PAGEFLAGS) - 1)
+static inline bool __balloon_page_flags(struct page *page)
+{
+       return page->flags & PAGE_FLAGS_MASK ? false : true;
+}
+
 /*
  * balloon_page_movable - test page->mapping->flags to identify balloon pages
  *                       that can be moved by compaction/migration.
@@ -121,8 +127,8 @@ static inline bool balloon_page_movable(struct page *page)
         * Before dereferencing and testing mapping->flags, lets make sure
         * this is not a page that uses ->mapping in a different way
         */
-       if (!PageSlab(page) && !PageSwapCache(page) && !PageAnon(page) &&
-           !page_mapped(page))
+       if (__balloon_page_flags(page) && !page_mapped(page) &&
+           page_count(page) == 1)
                return __is_movable_balloon_page(page);
 
        return false;