Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] mm/page_alloc: Add a reason for reserved pages in
 has_unmovable_pages()
To:     Qian Cai <cai@lca.pw>
Cc:     linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>,
        Michal Hocko <mhocko@suse.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        Oscar Salvador <osalvador@suse.de>,
        Mel Gorman <mgorman@techsingularity.net>,
        Mike Rapoport <rppt@linux.ibm.com>,
        Dan Williams <dan.j.williams@intel.com>,
        Pavel Tatashin <pavel.tatashin@microsoft.com>,
        linux-kernel@vger.kernel.org
References: <d3a88afd-63c6-1091-cf4c-75cd10b7f547@arm.com>
 <983E7EA4-A022-448C-B11D-8C10441A2E07@lca.pw>
From:   Anshuman Khandual <anshuman.khandual@arm.com>
Message-ID: <49fa7dea-00ac-155f-e7b7-eeca206556b5@arm.com>
Date:   Thu, 3 Oct 2019 17:32:02 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: <983E7EA4-A022-448C-B11D-8C10441A2E07@lca.pw>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 10/03/2019 05:20 PM, Qian Cai wrote:
> 
> 
>> On Oct 3, 2019, at 7:31 AM, Anshuman Khandual <Anshuman.Khandual@arm.com> wrote:
>>
>> Ohh, never meant that the 'Reserved' bit is anything special here but it
>> is a reason to make a page unmovable unlike many other flags.
> 
> But dump_page() is used everywhere, and it is better to reserve “reason” to indicate something more important rather than duplicating the page flags.
> 
> Especially, it is trivial enough right now for developers look in the page flags dumping from has_unmovable_pages(), and figure out the exact branching in the code.
> 

Will something like this be better ? hugepage_migration_supported() has got
uncertainty depending on platform and huge page size.

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 15c2050c629b..8dbc86696515 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -8175,7 +8175,7 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
        unsigned long found;
        unsigned long iter = 0;
        unsigned long pfn = page_to_pfn(page);
-       const char *reason = "unmovable page";
+       const char *reason;

        /*
         * TODO we could make this much more efficient by not checking every
@@ -8194,7 +8194,7 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
                if (is_migrate_cma(migratetype))
                        return false;

-               reason = "CMA page";
+               reason = "Unmovable CMA page";
                goto unmovable;
        }

@@ -8206,8 +8206,10 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int count,

                page = pfn_to_page(check);

-               if (PageReserved(page))
+               if (PageReserved(page)) {
+                       reason = "Unmovable reserved page";
                        goto unmovable;
+               }

                /*
                 * If the zone is movable and we have ruled out all reserved
@@ -8226,8 +8228,10 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
                        struct page *head = compound_head(page);
                        unsigned int skip_pages;

-                       if (!hugepage_migration_supported(page_hstate(head)))
+                       if (!hugepage_migration_supported(page_hstate(head))) {
+                               reason = "Unmovable HugeTLB page";
                                goto unmovable;
+                       }

                        skip_pages = compound_nr(head) - (page - head);
                        iter += skip_pages - 1;
@@ -8271,8 +8275,10 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int count,
                 * is set to both of a memory hole page and a _used_ kernel
                 * page at boot.
                 */
-               if (found > count)
+               if (found > count) {
+                       reason = "Unmovable non-LRU page";
                        goto unmovable;
+               }
        }
        return false;
 unmovable: