* [PATCH] mm/hotplug: fix a drain pcp bug when offline pages
@ 2013-08-01 9:18 Xishi Qiu
2013-08-01 18:59 ` Cody P Schafer
0 siblings, 1 reply; 3+ messages in thread
From: Xishi Qiu @ 2013-08-01 9:18 UTC (permalink / raw)
To: Andrew Morton, Mel Gorman, cody, Liujiang, Minchan Kim,
Yasuaki Ishimatsu, b.zolnierkie
Cc: linux-mm, LKML
__offline_pages()
start_isolate_page_range()
set_migratetype_isolate()
set_pageblock_migratetype() -> this pageblock will be marked as MIGRATE_ISOLATE
move_freepages_block() -> pages in PageBuddy will be moved into MIGRATE_ISOLATE list
drain_all_pages() -> drain PCP
free_pcppages_bulk()
mt = get_freepage_migratetype(page); -> PCP's migratetype is not MIGRATE_ISOLATE
__free_one_page(page, zone, 0, mt); -> so PCP will not be freed into into MIGRATE_ISOLATE list
In this case, the PCP may be allocated again, because they are not in
PageBuddy's MIGRATE_ISOLATE list. This will cause offline_pages failed.
Signed-off-by: Xishi Qiu <qiuxishi@huawei.com>
---
mm/page_alloc.c | 10 ++++++----
mm/page_isolation.c | 15 ++++++++++++++-
2 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index b100255..d873471 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -965,11 +965,13 @@ int move_freepages(struct zone *zone,
}
order = page_order(page);
- list_move(&page->lru,
- &zone->free_area[order].free_list[migratetype]);
- set_freepage_migratetype(page, migratetype);
+ if (get_freepage_migratetype(page) != migratetype) {
+ list_move(&page->lru,
+ &zone->free_area[order].free_list[migratetype]);
+ set_freepage_migratetype(page, migratetype);
+ pages_moved += 1 << order;
+ }
page += 1 << order;
- pages_moved += 1 << order;
}
return pages_moved;
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 383bdbb..ba1afc9 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -65,8 +65,21 @@ out:
}
spin_unlock_irqrestore(&zone->lock, flags);
- if (!ret)
+
+ if (!ret) {
drain_all_pages();
+ /*
+ * When drain_all_pages() frees cached pages into the buddy
+ * system, it uses the stale migratetype cached in the
+ * page->index field, so try to move free pages to ISOLATE
+ * list again.
+ */
+ spin_lock_irqsave(&zone->lock, flags);
+ nr_pages = move_freepages_block(zone, page, MIGRATE_ISOLATE);
+ __mod_zone_freepage_state(zone, -nr_pages, migratetype);
+ spin_unlock_irqrestore(&zone->lock, flags);
+ }
+
return ret;
}
--
1.7.1
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] mm/hotplug: fix a drain pcp bug when offline pages
2013-08-01 9:18 [PATCH] mm/hotplug: fix a drain pcp bug when offline pages Xishi Qiu
@ 2013-08-01 18:59 ` Cody P Schafer
2013-08-02 2:30 ` Xishi Qiu
0 siblings, 1 reply; 3+ messages in thread
From: Cody P Schafer @ 2013-08-01 18:59 UTC (permalink / raw)
To: Xishi Qiu
Cc: Andrew Morton, Mel Gorman, Liujiang, Minchan Kim,
Yasuaki Ishimatsu, b.zolnierkie, linux-mm, LKML
On 08/01/2013 02:18 AM, Xishi Qiu wrote:
> __offline_pages()
> start_isolate_page_range()
> set_migratetype_isolate()
> set_pageblock_migratetype() -> this pageblock will be marked as MIGRATE_ISOLATE
> move_freepages_block() -> pages in PageBuddy will be moved into MIGRATE_ISOLATE list
> drain_all_pages() -> drain PCP
> free_pcppages_bulk()
> mt = get_freepage_migratetype(page); -> PCP's migratetype is not MIGRATE_ISOLATE
> __free_one_page(page, zone, 0, mt); -> so PCP will not be freed into into MIGRATE_ISOLATE list
>
> In this case, the PCP may be allocated again, because they are not in
> PageBuddy's MIGRATE_ISOLATE list. This will cause offline_pages failed.
>
> Signed-off-by: Xishi Qiu <qiuxishi@huawei.com>
> ---
> mm/page_alloc.c | 10 ++++++----
> mm/page_isolation.c | 15 ++++++++++++++-
> 2 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index b100255..d873471 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -965,11 +965,13 @@ int move_freepages(struct zone *zone,
> }
>
> order = page_order(page);
> - list_move(&page->lru,
> - &zone->free_area[order].free_list[migratetype]);
> - set_freepage_migratetype(page, migratetype);
> + if (get_freepage_migratetype(page) != migratetype) {
> + list_move(&page->lru,
> + &zone->free_area[order].free_list[migratetype]);
> + set_freepage_migratetype(page, migratetype);
> + pages_moved += 1 << order;
> + }
> page += 1 << order;
> - pages_moved += 1 << order;
So this looks like it changes the return from move_freepages() to be the
"pages moved" from "the pages now belonging to the passed migrate type".
The user of move_freepages_block()'s return value (and thus the return
value of move_freepages()) in mm/page_alloc.c expects that it is the
original meaning. The users in page_isolation.c expect it is the new
meaning. Those need to be reconciled.
> }
>
> return pages_moved;
> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> index 383bdbb..ba1afc9 100644
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -65,8 +65,21 @@ out:
> }
>
> spin_unlock_irqrestore(&zone->lock, flags);
> - if (!ret)
> +
> + if (!ret) {
> drain_all_pages();
> + /*
> + * When drain_all_pages() frees cached pages into the buddy
> + * system, it uses the stale migratetype cached in the
> + * page->index field, so try to move free pages to ISOLATE
> + * list again.
> + */
> + spin_lock_irqsave(&zone->lock, flags);
> + nr_pages = move_freepages_block(zone, page, MIGRATE_ISOLATE);
> + __mod_zone_freepage_state(zone, -nr_pages, migratetype);
> + spin_unlock_irqrestore(&zone->lock, flags);
> + }
> +
Could we teach drain_all_pages() to use the right migrate type instead
(or add something similar that does)? (pages could be reallocated
between the drain_all_pages() and move_freepages_block()).
> return ret;
> }
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mm/hotplug: fix a drain pcp bug when offline pages
2013-08-01 18:59 ` Cody P Schafer
@ 2013-08-02 2:30 ` Xishi Qiu
0 siblings, 0 replies; 3+ messages in thread
From: Xishi Qiu @ 2013-08-02 2:30 UTC (permalink / raw)
To: Cody P Schafer
Cc: Andrew Morton, Mel Gorman, Liujiang, Minchan Kim,
Yasuaki Ishimatsu, b.zolnierkie, linux-mm, LKML
On 2013/8/2 2:59, Cody P Schafer wrote:
> On 08/01/2013 02:18 AM, Xishi Qiu wrote:
>> __offline_pages()
>> start_isolate_page_range()
>> set_migratetype_isolate()
>> set_pageblock_migratetype() -> this pageblock will be marked as MIGRATE_ISOLATE
>> move_freepages_block() -> pages in PageBuddy will be moved into MIGRATE_ISOLATE list
>> drain_all_pages() -> drain PCP
>> free_pcppages_bulk()
>> mt = get_freepage_migratetype(page); -> PCP's migratetype is not MIGRATE_ISOLATE
>> __free_one_page(page, zone, 0, mt); -> so PCP will not be freed into into MIGRATE_ISOLATE list
>>
>> In this case, the PCP may be allocated again, because they are not in
>> PageBuddy's MIGRATE_ISOLATE list. This will cause offline_pages failed.
>>
>> Signed-off-by: Xishi Qiu <qiuxishi@huawei.com>
>> ---
>> mm/page_alloc.c | 10 ++++++----
>> mm/page_isolation.c | 15 ++++++++++++++-
>> 2 files changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index b100255..d873471 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -965,11 +965,13 @@ int move_freepages(struct zone *zone,
>> }
>>
>> order = page_order(page);
>> - list_move(&page->lru,
>> - &zone->free_area[order].free_list[migratetype]);
>> - set_freepage_migratetype(page, migratetype);
>> + if (get_freepage_migratetype(page) != migratetype) {
>> + list_move(&page->lru,
>> + &zone->free_area[order].free_list[migratetype]);
>> + set_freepage_migratetype(page, migratetype);
>> + pages_moved += 1 << order;
>> + }
>> page += 1 << order;
>> - pages_moved += 1 << order;
>
> So this looks like it changes the return from move_freepages() to be the "pages moved" from "the pages now belonging to the passed migrate type".
>
> The user of move_freepages_block()'s return value (and thus the return value of move_freepages()) in mm/page_alloc.c expects that it is the original meaning. The users in page_isolation.c expect it is the new meaning. Those need to be reconciled.
>
>> }
>>
>> return pages_moved;
>> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
>> index 383bdbb..ba1afc9 100644
>> --- a/mm/page_isolation.c
>> +++ b/mm/page_isolation.c
>> @@ -65,8 +65,21 @@ out:
>> }
>>
>> spin_unlock_irqrestore(&zone->lock, flags);
>> - if (!ret)
>> +
>> + if (!ret) {
>> drain_all_pages();
>> + /*
>> + * When drain_all_pages() frees cached pages into the buddy
>> + * system, it uses the stale migratetype cached in the
>> + * page->index field, so try to move free pages to ISOLATE
>> + * list again.
>> + */
>> + spin_lock_irqsave(&zone->lock, flags);
>> + nr_pages = move_freepages_block(zone, page, MIGRATE_ISOLATE);
>> + __mod_zone_freepage_state(zone, -nr_pages, migratetype);
>> + spin_unlock_irqrestore(&zone->lock, flags);
>> + }
>> +
>
> Could we teach drain_all_pages() to use the right migrate type instead (or add something similar that does)? (pages could be reallocated between the drain_all_pages() and move_freepages_block()).
>
Sorry, I made a mistake. __test_page_isolated_in_pageblock() will catch the pages and move
them into MIGRATE_ISOLATE list. So the code is right.
Thanks,
Xishi Qiu
>> return ret;
>> }
>>
>
> .
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-08-02 2:31 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-01 9:18 [PATCH] mm/hotplug: fix a drain pcp bug when offline pages Xishi Qiu
2013-08-01 18:59 ` Cody P Schafer
2013-08-02 2:30 ` Xishi Qiu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).