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: <87pobjhssq.fsf@notabene.neil.brown.name>
Date:   Sat, 26 Aug 2017 14:11:33 +1000
From:   NeilBrown <neilb@...e.com>
To:     Pavel Machek <pavel@....cz>, Michal Hocko <mhocko@...nel.org>
Cc:     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 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.

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.

NeilBrown

Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)