linux-kernel - Re: [PATCH 1/1] mm, compaction: correct the bounds of __fragmentation

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: <E718672A-91A0-4A5A-91B5-A6CF1E9BD544@oracle.com>
Date:   Mon, 19 Feb 2018 12:14:26 +0000
From:   Robert Harris <robert.m.harris@...cle.com>
To:     Michal Hocko <mhocko@...nel.org>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        linux-doc@...r.kernel.org, Jonathan Corbet <corbet@....net>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Vlastimil Babka <vbabka@...e.cz>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Johannes Weiner <hannes@...xchg.org>,
        Kemi Wang <kemi.wang@...el.com>,
        David Rientjes <rientjes@...gle.com>,
        Yafang Shao <laoar.shao@...il.com>,
        Kangmin Park <l4stpr0gr4m@...il.com>,
        Mel Gorman <mgorman@...e.de>,
        Yisheng Xie <xieyisheng1@...wei.com>,
        Davidlohr Bueso <dave@...olabs.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Huang Ying <ying.huang@...el.com>,
        Vinayak Menon <vinmenon@...eaurora.org>
Subject: Re: [PATCH 1/1] mm, compaction: correct the bounds of
 __fragmentation_index()

> On 19 Feb 2018, at 08:26, Michal Hocko <mhocko@...nel.org> wrote:
> 
> On Sun 18-02-18 16:47:55, robert.m.harris@...cle.com wrote:
>> From: "Robert M. Harris" <robert.m.harris@...cle.com>
>> 
>> __fragmentation_index() calculates a value used to determine whether
>> compaction should be favoured over page reclaim in the event of allocation
>> failure.  The calculation itself is opaque and, on inspection, does not
>> match its existing description.  The function purports to return a value
>> between 0 and 1000, representing units of 1/1000.  Barring the case of a
>> pathological shortfall of memory, the lower bound is instead 500.  This is
>> significant because it is the default value of sysctl_extfrag_threshold,
>> i.e. the value below which compaction should be avoided in favour of page
>> reclaim for costly pages.
>> 
>> This patch implements and documents a modified version of the original
>> expression that returns a value in the range 0 <= index < 1000.  It amends
>> the default value of sysctl_extfrag_threshold to preserve the existing
>> behaviour.
> 
> It is not really clear to me what is the actual problem you are trying
> to solve by this patch. Is there any bug or are you just trying to
> improve the current implementation to be more effective?

There is not a significant bug.

The first problem is that the mathematical expression in
__fragmentation_index() is opaque, particularly given the lack of
description in the comments or the original commit message.  This patch
provides such a description.

Simply annotating the expression did not make sense since the formula
doesn't work as advertised.  The fragmentation index is described as
being in the range 0 to 1000 but the bounds of the formula are instead
500 to 1000.  This patch changes the formula so that its lower bound is
0.

The fragmentation index is compared to the tuneable
sysctl_extfrag_threshold, which defaults to 500.  If the index is above
this value then compaction is preferred over page reclaim in the event
of allocation failure.  Given the issue above, the index will almost
always exceed the default threshold and compaction will occur even if
there is low fragmentation.  This patch changes the default value of the
tuneable to 0, meaning that the existing behaviour will be unchanged.
Changing sysctl_extfrag_threshold back to something non-zero in a future
patch would effect the behaviour intended by the original code but would
require more comprehensive testing since it would modify the kernel's
performance under memory pressure.

Robert Harris