Messages in this thread Patch in this message | | | Subject | Re: [PATCH v6 10/11] mm, compaction: require only min watermarks for non-costly orders | From | Vlastimil Babka <> | Date | Thu, 18 Aug 2016 14:20:10 +0200 |
| |
On 08/16/2016 08:46 AM, Joonsoo Kim wrote: > On Tue, Aug 16, 2016 at 08:36:12AM +0200, Vlastimil Babka wrote: >> On 08/16/2016 08:16 AM, Joonsoo Kim wrote: >>> On Wed, Aug 10, 2016 at 11:12:25AM +0200, Vlastimil Babka wrote: >>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >>>> index 621e4211ce16..a5c0f914ec00 100644 >>>> --- a/mm/page_alloc.c >>>> +++ b/mm/page_alloc.c >>>> @@ -2492,7 +2492,7 @@ int __isolate_free_page(struct page *page, unsigned int order) >>>> >>>> if (!is_migrate_isolate(mt)) { >>>> /* Obey watermarks as if the page was being allocated */ >>>> - watermark = low_wmark_pages(zone) + (1 << order); >>>> + watermark = min_wmark_pages(zone) + (1UL << order); >>> >>> This '1 << order' also needs some comment. Why can't we use >>> compact_gap() in this case? >> >> This is just short-cutting the high-order watermark check to check >> only order-0, because we already know the high-order page exists. >> We can't use compact_gap() as that's too high to use for a single >> allocation watermark, since we can be already holding some free >> pages on the list. So it would defeat the gap purpose. > > Oops. I missed that. Thanks for clarifying it.
So let's expand the comment?
----8<---- From 5d060f4222a637e1005ff32ae0fd4330625b6675 Mon Sep 17 00:00:00 2001 From: Vlastimil Babka <vbabka@suse.cz> Date: Thu, 18 Aug 2016 14:18:08 +0200 Subject: [PATCH] mm, compaction: require only min watermarks for non-costly orders-fix
Clarify why __isolate_free_page() does a order-0 watermark check with apparent (1UL << order) gap, per Joonsoo.
Signed-off-by: Vlastimil Babka <vbabka@suse.cz> --- mm/page_alloc.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c index a5c0f914ec00..216715504fb4 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -2491,7 +2491,12 @@ int __isolate_free_page(struct page *page, unsigned int order) mt = get_pageblock_migratetype(page); if (!is_migrate_isolate(mt)) { - /* Obey watermarks as if the page was being allocated */ + /* + * Obey watermarks as if the page was being allocated. We can + * emulate a high-order watermark check with a raised order-0 + * watermark, because we already know our high-order page + * exists. + */ watermark = min_wmark_pages(zone) + (1UL << order); if (!zone_watermark_ok(zone, 0, watermark, 0, ALLOC_CMA)) return 0; -- 2.9.2
| |