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  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]
Date:   Tue, 24 Oct 2017 14:25:26 +0200
From:   Michal Hocko <>
To:     Vlastimil Babka <>
Cc:     Joonsoo Kim <>,,
        Michael Ellerman <>,
        Andrew Morton <>,
        KAMEZAWA Hiroyuki <>,
        Reza Arbab <>,
        Yasuaki Ishimatsu <>,, Igor Mammedov <>,
        Vitaly Kuznetsov <>,
        LKML <>
Subject: Re: [PATCH 1/2] mm: drop migrate type checks from has_unmovable_pages

On Tue 24-10-17 10:12:58, Vlastimil Babka wrote:
> On 10/24/2017 06:44 AM, Joonsoo Kim wrote:
> >>> I'm not sure what is the confusing semantic you mentioned. I think
> >>> that set_migratetype_isolate() has confusing semantic and should be
> >>> fixed since making the pageblock isolated doesn't need to check if
> >>> there is unmovable page or not. Do you think that
> >>> set_migratetype_isolate() need to check it? If so, why?
> >>
> >> My intuitive understanding of set_migratetype_isolate is that it either
> >> suceeds and that means that the given pfn range can be isolated for the
> >> given type of allocation (be it movable or cma). No new pages will be
> >> allocated from this range to allow converging into a free range in a
> >> finit amount of time. At least this is how the hotplug code would like
> >> to use it and I suppose that the alloc_contig_range would like to
> >> guarantee the same to not rely on a fixed amount of migration attempts.
> > 
> > Yes, alloc_contig_range() also want to guarantee the similar thing.
> > Major difference between them is 'given pfn range'. memory hotplug
> > works by pageblock unit but alloc_contig_range() doesn't.
> > alloc_contig_range() works by the page unit. However, there is no easy
> > way to isolate individual page so it uses pageblock isolation
> > regardless of 'given pfn range'. In this case, checking movability of
> > all pages on the pageblock would cause the problem as I mentioned
> > before.
> I couldn't look too closely yet, but do I understand correctly that the
> *potential* problem (because as you say there are no such
> alloc_contig_range callers) you are describing is not newly introduced
> by Michal's series? Then his patch fixing the introduced regression
> should be enough for now, and further improvements could be posted on
> top, and not vice versa? Please don't take it wrong, I agree the current
> state is a bit of a mess and improvements are welcome. Also it seems to
> me that Michal is right, and there's nothing preventing
> alloc_contig_range() to allocate from CMA pageblocks for non-CMA
> purposes (likely not movable), and that should be also fixed?

OK, it seems I understand Joonsoo's concern more now. And I agree with
Vlastimil, that it is better to plug the immediate regression with a
minimal patch and discuss general improvements of the pfn based
allocator separatelly. There are more things to clear up there,
including the proper API (alloc_contig_range is just too low level for
anybody to use) as well as the MIGRATE_* flags usage (e.g. I am not
really sure GB pages usage of MIGRATE_MOVABLE is really correct).
alloc_contig_range looks like an internal CMA function which has been
(ab)used for a different purpose to me rather than a well thought
through interface. MAP_CONTIG discussion has shown some interest in
an API for large allocations so I _believe_ we should think that through
befire we grow more unexpected users.

I am definitely willing to help there.

Is that something you would agree with Joonsoo?
Michal Hocko

Powered by blists - more mailing lists