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 <vba...@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 <vba...@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
