Re: Accounting problem of MIGRATE_ISOLATED freed page
From: Kamezawa Hiroyuki
Date: Thu Jun 21 2012 - 06:55:06 EST
(2012/06/21 13:55), Minchan Kim wrote:
On 06/21/2012 11:45 AM, KOSAKI Motohiro wrote:
On Wed, Jun 20, 2012 at 9:55 PM, Minchan Kim<minchan@xxxxxxxxxx> wrote:
On 06/21/2012 10:39 AM, KOSAKI Motohiro wrote:
number of isolate page block is almost always 0. then if we have such counter,
we almost always can avoid zone->lock. Just idea.
Yeb. I thought about it but unfortunately we can't have a counter for MIGRATE_ISOLATE.
Because we have to tweak in page free path for pages which are going to free later after we
mark pageblock type to MIGRATE_ISOLATE.
I mean,
if (nr_isolate_pageblock != 0)
free_pages -= nr_isolated_free_pages(); // your counting logic
return __zone_watermark_ok(z, alloc_order, mark,
classzone_idx, alloc_flags, free_pages);
I don't think this logic affect your race. zone_watermark_ok() is already
racy. then new little race is no big matter.
It seems my explanation wasn't enough. :(
I already understand your intention but we can't make nr_isolate_pageblock.
Because we should count two type of free pages.
I mean, move_freepages_block increment number of page *block*, not pages.
number of free *pages* are counted by zone_watermark_ok_safe().
1. Already freed page so they are already in buddy list.
Of course, we can count it with return value of move_freepages_block(zone, page, MIGRATE_ISOLATE) easily.
2. Will be FREEed page by do_migrate_range.
It's a _PROBLEM_. For it, we should tweak free path. No?
No.
If All of pages are PageLRU when hot-plug happens(ie, 2), nr_isolate_pagblock is zero and
zone_watermk_ok_safe can't do his role.
number of isolate pageblock don't depend on number of free pages. It's
a concept of
an attribute of PFN range.
It seems you mean is_migrate_isolate as a just flag, NOT nr_isolate_pageblock.
So do you mean this?
diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index 3bdcab3..7f4d19c 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -1,6 +1,7 @@
#ifndef __LINUX_PAGEISOLATION_H
#define __LINUX_PAGEISOLATION_H
+extern bool is_migrate_isolate;
/*
* Changes migrate type in [start_pfn, end_pfn) to be MIGRATE_ISOLATE.
* If specified range includes migrate types other than MOVABLE or CMA,
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index d2a515d..b997cb3 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1756,6 +1756,27 @@ bool zone_watermark_ok_safe(struct zone *z, int order, unsigned long ma
if (z->percpu_drift_mark&& free_pages< z->percpu_drift_mark)
free_pages = zone_page_state_snapshot(z, NR_FREE_PAGES);
+#if defined CONFIG_CMA || CONFIG_MEMORY_HOTPLUG
+ if (unlikely(is_migrate_isolate)) {
+ unsigned long flags;
+ spin_lock_irqsave(&z->lock, flags);
+ for (order = MAX_ORDER - 1; order>= 0; order--) {
+ struct free_area *area =&z->free_area[order];
+ long count = 0;
+ struct list_head *curr;
+
+ list_for_each(curr,&area->free_list[MIGRATE_ISOLATE])
+ count++;
+
+ free_pages -= (count<< order);
+ if (free_pages< 0) {
+ free_pages = 0;
+ break;
+ }
+ }
+ spin_unlock_irqrestore(&z->lock, flags);
+ }
+#endif
return __zone_watermark_ok(z, order, mark, classzone_idx, alloc_flags,
free_pages);
}
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index c9f0477..212e526 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -19,6 +19,8 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
return pfn_to_page(pfn + i);
}
+bool is_migrate_isolate = false;
+
/*
* start_isolate_page_range() -- make page-allocation-type of range of pages
* to be MIGRATE_ISOLATE.
@@ -43,6 +45,8 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
BUG_ON((start_pfn)& (pageblock_nr_pages - 1));
BUG_ON((end_pfn)& (pageblock_nr_pages - 1));
+ is_migrate_isolate = true;
+
for (pfn = start_pfn;
pfn< end_pfn;
pfn += pageblock_nr_pages) {
@@ -59,6 +63,7 @@ undo:
pfn += pageblock_nr_pages)
unset_migratetype_isolate(pfn_to_page(pfn), migratetype);
+ is_migrate_isolate = false;
return -EBUSY;
}
@@ -80,6 +85,9 @@ int undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
continue;
unset_migratetype_isolate(page, migratetype);
}
+
+ is_migrate_isolate = false;
+
return 0;
}
/*
It is still racy as you already mentioned and I don't think it's trivial.
Direct reclaim can't wake up kswapd forever by current fragile zone->all_unreclaimable.
So it's a livelock.
Then, do you want to fix this problem by your patch[1]?
It could solve the livelock by OOM kill if we apply your patch[1] but still doesn't wake up
kswapd although it's not critical. Okay. Then, please write down this problem in detail
in your patch's changelog and resend, please.
Hm. I'm sorry if I couldn't chase the disucussion...Can I make summary ?
As you shown, it seems to be not difficult to counting free pages under MIGRATE_ISOLATE.
And we can know the zone contains MIGRATE_ISOLATE area or not by simple check.
for example.
==
set_pageblock_migratetype(page, MIGRATE_ISOLATE);
move_freepages_block(zone, page, MIGRATE_ISOLATE);
zone->nr_isolated_areas++;
=
Then, the solution will be adding a function like following
=
u64 zone_nr_free_pages(struct zone *zone) {
unsigned long free_pages;
free_pages = zone_page_state(NR_FREE_PAGES);
if (unlikely(z->nr_isolated_areas)) {
isolated = count_migrate_isolated_pages(zone);
free_pages -= isolated;
}
return free_pages;
}
=
Right ? and... zone->all_unreclaimable is a different problem ?
Thanks,
-Kame
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/