On 07/04/2014 09:57 AM, Joonsoo Kim wrote:
Currently, when we free the page from pcp list to buddy, we check
pageblock of the page in order to isolate the page on isolated
pageblock. Although this could rarely happen and to check migratetype of
pageblock is somewhat expensive, we check it on free fast path. I think
that this is undesirable. To prevent this situation, I introduce new
variable, nr_isolate_pageblock on struct zone and use it to determine
if we should check pageblock migratetype. Isolation on pageblock rarely
happens so we can mostly avoid this pageblock migratetype check.
Better, but still there's a zone flag check and maintenance. So if it
could be avoided, it would be better.
Additionally, unify freepage counting code, because it can be done in
common part, __free_one_page(). This unifying provides extra guarantee
that the page on isolate pageblock don't go into non-isolate buddy list.
This is similar situation describing in previous patch so refer it
if you need more explanation.
You should make it clearer that you are solving misplacement of the type
"page should be placed on isolated freelist but it's not" through
free_one_page(), which was solved only for free_pcppages_bulk() in patch
03/10. Mentioning patch 04/10 here, which solves the opposite problem
"page shouldn't be placed on isolated freelist, but it is", only
confuses the situation. Also this patch undoes everything of 04/10 and
moves it elsewhere, so that would make it harder to git blame etc. I
would reorder 04 and 05.
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
---
include/linux/mmzone.h | 5 +++++
include/linux/page-isolation.h | 8 ++++++++
mm/page_alloc.c | 40 +++++++++++++++++++---------------------
mm/page_isolation.c | 2 ++
4 files changed, 34 insertions(+), 21 deletions(-)
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index fd48890..e9c194f 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -374,6 +374,11 @@ struct zone {
/* pfn where async and sync compaction migration scanner should start */
unsigned long compact_cached_migrate_pfn[2];
#endif
+
+#ifdef CONFIG_MEMORY_ISOLATION
+ unsigned long nr_isolate_pageblock;
+#endif
+
#ifdef CONFIG_MEMORY_HOTPLUG
/* see spanned/present_pages for more description */
seqlock_t span_seqlock;
diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index 3fff8e7..2dc1e16 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -2,6 +2,10 @@
#define __LINUX_PAGEISOLATION_H
#ifdef CONFIG_MEMORY_ISOLATION
+static inline bool has_isolate_pageblock(struct zone *zone)
+{
+ return zone->nr_isolate_pageblock;
+}
static inline bool is_migrate_isolate_page(struct page *page)
{
return get_pageblock_migratetype(page) == MIGRATE_ISOLATE;
@@ -11,6 +15,10 @@ static inline bool is_migrate_isolate(int migratetype)
return migratetype == MIGRATE_ISOLATE;
}
#else
+static inline bool has_isolate_pageblock(struct zone *zone)
+{
+ return false;
+}
static inline bool is_migrate_isolate_page(struct page *page)
{
return false;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index d8feedc..dcc2f08 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -594,6 +594,24 @@ static inline void __free_one_page(struct page *page,
VM_BUG_ON_PAGE(page_idx & ((1 << order) - 1), page);
VM_BUG_ON_PAGE(bad_range(zone, page), page);
+ /*
+ * pcp pages could go normal buddy list due to stale pageblock
+ * migratetype so re-check it if there is isolate pageblock.
+ *
+ * And, we got migratetype without holding the lock so it could be
+ * racy. If some pages go on the isolate migratetype buddy list
+ * by this race, we can't allocate this page anymore until next
+ * isolation attempt on this pageblock. To prevent this
+ * possibility, re-check migratetype with holding the lock.
+ */
+ if (unlikely(has_isolate_pageblock(zone) ||
+ is_migrate_isolate(migratetype))) {
+ migratetype = get_pfnblock_migratetype(page, pfn);
+ }
+
+ if (!is_migrate_isolate(migratetype))
+ __mod_zone_freepage_state(zone, 1 << order, migratetype);
+
while (order < MAX_ORDER-1) {
buddy_idx = __find_buddy_index(page_idx, order);
buddy = page + (buddy_idx - page_idx);
@@ -719,13 +737,7 @@ static void free_pcppages_bulk(struct zone *zone, int count,
page = list_entry(list->prev, struct page, lru);
/* must delete as __free_one_page list manipulates */
list_del(&page->lru);
-
- if (unlikely(is_migrate_isolate_page(page))) {
- mt = MIGRATE_ISOLATE;
- } else {
- mt = get_freepage_migratetype(page);
- __mod_zone_freepage_state(zone, 1, mt);
- }
+ mt = get_freepage_migratetype(page);
/* MIGRATE_MOVABLE list may include MIGRATE_RESERVEs */
__free_one_page(page, page_to_pfn(page), zone, 0, mt);
@@ -742,21 +754,7 @@ static void free_one_page(struct zone *zone,
{
spin_lock(&zone->lock);
zone->pages_scanned = 0;
-
- if (unlikely(is_migrate_isolate(migratetype))) {
- /*
- * We got migratetype without holding the lock so it could be
- * racy. If some pages go on the isolate migratetype buddy list
- * by this race, we can't allocate this page anymore until next
- * isolation attempt on this pageblock. To prevent this
- * possibility, re-check migratetype with holding the lock.
- */
- migratetype = get_pfnblock_migratetype(page, pfn);
- }
-
__free_one_page(page, pfn, zone, order, migratetype);
- if (!is_migrate_isolate(migratetype))
- __mod_zone_freepage_state(zone, 1 << order, migratetype);
spin_unlock(&zone->lock);
}
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index d1473b2..1fa4a4d 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -60,6 +60,7 @@ out:
int migratetype = get_pageblock_migratetype(page);
set_pageblock_migratetype(page, MIGRATE_ISOLATE);
+ zone->nr_isolate_pageblock++;
nr_pages = move_freepages_block(zone, page, MIGRATE_ISOLATE);
__mod_zone_freepage_state(zone, -nr_pages, migratetype);
@@ -83,6 +84,7 @@ void unset_migratetype_isolate(struct page *page, unsigned migratetype)
nr_pages = move_freepages_block(zone, page, migratetype);
__mod_zone_freepage_state(zone, nr_pages, migratetype);
set_pageblock_migratetype(page, migratetype);
+ zone->nr_isolate_pageblock--;
out:
spin_unlock_irqrestore(&zone->lock, flags);
}
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>