linux-kernel - Re: [RFC PATCH] treewide: remove GFP

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: <20170828123657.GK17097@dhcp22.suse.cz>
Date:   Mon, 28 Aug 2017 14:36:57 +0200
From:   Michal Hocko <mhocko@...nel.org>
To:     NeilBrown <neilb@...e.com>
Cc:     Pavel Machek <pavel@....cz>, linux-mm@...ck.org,
        Mel Gorman <mgorman@...e.de>,
        Matthew Wilcox <willy@...radead.org>,
        Vlastimil Babka <vbabka@...e.cz>, Neil Brown <neilb@...e.de>,
        Theodore Ts'o <tytso@....edu>,
        Andrew Morton <akpm@...ux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH] treewide: remove GFP_TEMPORARY allocation flag

On Sat 26-08-17 14:11:33, NeilBrown wrote:
> On Fri, Aug 25 2017, Pavel Machek wrote:
> 
> > On Fri 2017-08-25 10:04:42, Michal Hocko wrote:
> >> On Fri 25-08-17 09:28:19, Pavel Machek wrote:
> >> > On Fri 2017-08-25 08:35:46, Michal Hocko wrote:
> >> > > On Wed 23-08-17 19:57:09, Pavel Machek wrote:
> >> [...]
> >> > > > Dunno. < 1msec probably is temporary, 1 hour probably is not. If it causes
> >> > > > problems, can you just #define GFP_TEMPORARY GFP_KERNEL ? Treewide replace,
> >> > > > and then starting again goes not look attractive to me.
> >> > > 
> >> > > I do not think we want a highlevel GFP_TEMPORARY without any meaning.
> >> > > This just supports spreading the flag usage without a clear semantic
> >> > > and it will lead to even bigger mess. Once we can actually define what
> >> > > the flag means we can also add its users based on that new semantic.
> >> > 
> >> > It has real meaning.
> >> 
> >> Which is?
> >
> > "This allocation is temporary. It lasts milliseconds, not hours."
> 
> It isn't sufficient to give a rule for when GFP_TEMPORARY will be used,
> you also need to explain (at least in general terms) how the information
> will be used.  Also you need to give guidelines on whether the flag
> should be set for allocation that will last seconds or minutes.
> 
> If we have a flag that doesn't have a well defined meaning that actually
> affects behavior, it will not be used consistently, and if we ever
> change exactly how it behaves we can expect things to break.  So it is
> better not to have a flag, than to have a poorly defined flag.

Absolutely agreed!

> My current thoughts is that the important criteria is not how long the
> allocation will be used for, but whether it is reclaimable.  Allocations
> that will only last 5 msecs are reclaimable by calling "usleep(5000)".
> Other allocations might be reclaimable in other ways.  Allocations that
> are not reclaimable may well be directed to a more restricted pool of
> memory, and might be more likely to fail.  If we grew a strong
> "reclaimable" concept, this 'temporary' concept that you want to hold on
> to would become a burden.

... and here again. The whole motivation for the flag was to gather
these objects together and reduce chances of internal fragmentation
due to long lived objects mixed with short term ones. Without an
explicit way to reclaim those objects or having a clear checkpoint to
wait for it is not really helping us to reach desired outcome (less
fragmented memory).

-- 
Michal Hocko
SUSE Labs