linux-kernel - Re: [PATCH 1/4] mm, page_owner: make init_pages_in

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:   Wed, 6 Sep 2017 15:55:22 +0200
From:   Vlastimil Babka <vbabka@...e.cz>
To:     Michal Hocko <mhocko@...nel.org>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        Joonsoo Kim <iamjoonsoo.kim@....com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Mel Gorman <mgorman@...hsingularity.net>,
        Yang Shi <yang.shi@...aro.org>,
        Laura Abbott <labbott@...hat.com>,
        Vinayak Menon <vinmenon@...eaurora.org>,
        zhong jiang <zhongjiang@...wei.com>
Subject: Re: [PATCH 1/4] mm, page_owner: make init_pages_in_zone() faster

On 09/06/2017 03:49 PM, Michal Hocko wrote:
> On Thu 31-08-17 09:55:25, Vlastimil Babka wrote:
>> On 08/23/2017 08:47 AM, Vlastimil Babka wrote:
>>> On 07/24/2017 02:38 PM, Michal Hocko wrote:
>>>> On Thu 20-07-17 15:40:26, Vlastimil Babka wrote:
>>>>> In init_pages_in_zone() we currently use the generic set_page_owner() function
>>>>> to initialize page_owner info for early allocated pages. This means we
>>>>> needlessly do lookup_page_ext() twice for each page, and more importantly
>>>>> save_stack(), which has to unwind the stack and find the corresponding stack
>>>>> depot handle. Because the stack is always the same for the initialization,
>>>>> unwind it once in init_pages_in_zone() and reuse the handle. Also avoid the
>>>>> repeated lookup_page_ext().
>>>>
>>>> Yes this looks like an improvement but I have to admit that I do not
>>>> really get why we even do save_stack at all here. Those pages might
>>>> got allocated from anywhere so we could very well provide a statically
>>>> allocated "fake" stack trace, no?
>>>
>>> We could, but it's much simpler to do it this way than try to extend
>>> stack depot/stack saving to support creating such fakes. Would it be
>>> worth the effort?
>>
>> Ah, I've noticed we already do this for the dummy (prevent recursion)
>> stack and failure stack. So here you go. It will also make the fake
>> stack more obvious after "[PATCH 2/2] mm, page_owner: Skip unnecessary
>> stack_trace entries" is merged, which would otherwise remove
>> init_page_owner() from the stack.
> 
> Yes this is what I've had in mind.
> 
>> ----8<----
>> >From 9804a5e62fc768e12b86fd4a3184e692c59ebfd1 Mon Sep 17 00:00:00 2001
>> From: Vlastimil Babka <vbabka@...e.cz>
>> Date: Thu, 31 Aug 2017 09:46:46 +0200
>> Subject: [PATCH] mm, page_owner: make init_pages_in_zone() faster-fix2
>>
>> Create statically allocated fake stack trace for early allocated pages, per
>> Michal Hocko.
> 
> Yes this looks good to me. I am just wondering why we need 3 different
> fake stacks. I do not see any code that would special case them when
> dumping traces. Maybe this can be done on top?

It's so that the user can differentiate them in the output. That's why
the functions are noinline.

>> Signed-off-by: Vlastimil Babka <vbabka@...e.cz>
> 
> Anyway
> Acked-by: Michal Hocko <mhocko@...e.com>

Thanks!