linux-kernel - Re: [PATCH] mm/page_alloc: Add a reason for reserved pages in has_unmovable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 3 Oct 2019 17:32:02 +0530
From:   Anshuman Khandual <anshuman.khandual@....com>
To:     Qian Cai <cai@....pw>
Cc:     linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
        Michal Hocko <mhocko@...e.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        Oscar Salvador <osalvador@...e.de>,
        Mel Gorman <mgorman@...hsingularity.net>,
        Mike Rapoport <rppt@...ux.ibm.com>,
        Dan Williams <dan.j.williams@...el.com>,
        Pavel Tatashin <pavel.tatashin@...rosoft.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm/page_alloc: Add a reason for reserved pages in
 has_unmovable_pages()



On 10/03/2019 05:20 PM, Qian Cai wrote:
> 
> 
>> On Oct 3, 2019, at 7:31 AM, Anshuman Khandual <Anshuman.Khandual@....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: