linux-kernel - Re: [RFC PATCH v2 2/4] mm/__free_one_page: skip merge for order-0 page unless compaction failed

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]

Message-ID: <5b1f7ef5-0dca-c35f-aba9-3b55f81740b2@suse.cz>
Date:   Wed, 21 Mar 2018 08:53:27 +0100
From:   Vlastimil Babka <vbabka@...e.cz>
To:     Aaron Lu <aaron.lu@...el.com>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Huang Ying <ying.huang@...el.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Kemi Wang <kemi.wang@...el.com>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Michal Hocko <mhocko@...e.com>,
        Mel Gorman <mgorman@...hsingularity.net>,
        Matthew Wilcox <willy@...radead.org>,
        Daniel Jordan <daniel.m.jordan@...cle.com>
Subject: Re: [RFC PATCH v2 2/4] mm/__free_one_page: skip merge for order-0
 page unless compaction failed

On 03/20/2018 03:11 PM, Aaron Lu wrote:
> On Tue, Mar 20, 2018 at 12:45:50PM +0100, Vlastimil Babka wrote:
>> But why, with all the prefetching in place?
> 
> The prefetch is just for its order 0 buddy, if merge happens, then its
> order 1 buddy will also be checked and on and on, so the cache misses
> are much more in merge mode.

I see.

>> Not thrilled about such disruptive change in the name of a
>> microbenchmark :/ Shouldn't normally the pcplists hide the overhead?
> 
> Sadly, with the default pcp count, it didn't avoid the lock contention.
> We can of course increase pcp->count to a large enough value to avoid
> entering buddy and thus avoid zone->lock contention, but that would
> require admin to manually change the value on a per-machine per-workload
> basis I believe.

Well, anyone who really cares about performance has to invest some time
to tuning anyway, I believe?

>> If not, wouldn't it make more sense to turn zone->lock into a range lock?
> 
> Not familiar with range lock, will need to take a look at it, thanks for
> the pointer.

The suggestion was rather quick and not well thought-out. Range lock
itself is insufficient - for merging/splitting buddies it's ok for
working with struct pages because the candidate buddies are within a
MAX_ORDER range. But the freelists contain pages from the whole zone.

>>
>>> A new document file called "struct_page_filed" is added to explain
>>> the newly reused field in "struct page".
>>
>> Sounds rather ad-hoc for a single field, I'd rather document it via
>> comments.
> 
> Dave would like to have a document to explain all those "struct page"
> fields that are repurposed under different scenarios and this is the
> very start of the document :-)

Oh, I see.

> I probably should have explained the intent of the document more.
> 
> Thanks for taking a look at this.
> 
>>> Suggested-by: Dave Hansen <dave.hansen@...el.com>
>>> Signed-off-by: Aaron Lu <aaron.lu@...el.com>
>>> ---
>>>  Documentation/vm/struct_page_field |  5 +++
>>>  include/linux/mm_types.h           |  1 +
>>>  mm/compaction.c                    | 13 +++++-
>>>  mm/internal.h                      | 27 ++++++++++++
>>>  mm/page_alloc.c                    | 89 +++++++++++++++++++++++++++++++++-----
>>>  5 files changed, 122 insertions(+), 13 deletions(-)
>>>  create mode 100644 Documentation/vm/struct_page_field
>>>