linux-kernel - Re: [PATCH v2 2/4] mm: introduce cma_alloc

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201203082810.GX17338@dhcp22.suse.cz>
Date:   Thu, 3 Dec 2020 09:28:10 +0100
From:   Michal Hocko <mhocko@...e.com>
To:     David Hildenbrand <david@...hat.com>
Cc:     Minchan Kim <minchan@...nel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-mm <linux-mm@...ck.org>, hyesoo.yu@...sung.com,
        willy@...radead.org, iamjoonsoo.kim@....com, vbabka@...e.cz,
        surenb@...gle.com, pullip.cho@...sung.com, joaodias@...gle.com,
        hridya@...gle.com, sumit.semwal@...aro.org, john.stultz@...aro.org,
        Brian.Starkey@....com, linux-media@...r.kernel.org,
        devicetree@...r.kernel.org, robh@...nel.org,
        christian.koenig@....com, linaro-mm-sig@...ts.linaro.org
Subject: Re: [PATCH v2 2/4] mm: introduce cma_alloc_bulk API

On Wed 02-12-20 21:22:36, David Hildenbrand wrote:
> On 02.12.20 20:26, Minchan Kim wrote:
> > On Wed, Dec 02, 2020 at 07:51:07PM +0100, Michal Hocko wrote:
[...]
> >> I am still not sure a specific flag is a good interface. Really can this
> >> be gfp_mask instead?
> > 
> > I am not strong(even, I did it with GFP_NORETRY) but David wanted to
> > have special mode and I agreed when he mentioned ALLOC_CONTIG_HARD as
> > one of options in future(it would be hard to indicate that mode with
> > gfp flags).
> 
> I can't tell regarding the CMA interface, but for the alloc_contig()
> interface I think modes make sense. Yes, it's different to other
> allocaters, but the contig range allocater is different already. E.g.,
> the CMA allocater mostly hides "which exact PFNs you try to allocate".

Yes, alloc_contig_range is a low level API but it already has a gfp_mask
parameter. Adding yet another allocation mode sounds like API
convolution to me.

> In the contig range allocater, gfp flags are currently used to express
> how to allocate pages used as migration targets. I don't think mangling
> in other gfp flags (or even overloading them) makes things a lot
> clearer. E.g., GFP_NORETRY: don't retry to allocate migration targets?
> don't retry to migrate pages? both?
>
> As I said, other aspects might be harder to model (e.g., don't drain
> LRU) and hiding them behind generic gfp flags (e.g., GFP_NORETRY) feels
> wrong.
> 
> With the mode, we're expressing details for the necessary page
> migration. Suggestions on how to model that are welcome.

The question is whether the caller should really have such an intimate
knowledge and control of the alloc_contig_range implementation. This all
are implementation details. Should really anybody think about how many
times migration retries or control LRU draining? Those can change in the
future and I do not think we really want to go over all users grown over
that time and try to deduce what was the intention behind.

I think we should aim at easy and very highlevel behavior:
- GFP_NOWAIT - unsupported currently IIRC but something that something
  that should be possible to implement. Isolation is non blocking,
  migration could be skipped
- GFP_KERNEL - default behavior whatever that means
- GFP_NORETRY - opportunistic allocation as lightweight as we can get.
  Failures to be expected also for transient reasons.
- GFP_RETRY_MAYFAIL - try hard but not as hard as to trigger disruption
  (e.g. via oom killer).

- __GFP_THIS_NODE - stick to a node without fallback
- we can support zone modifiers although there is no existing user.
- __GFP_NOWARN - obvious

And that is it. Or maybe I am seeing that oversimplified.
-- 
Michal Hocko
SUSE Labs