LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: Mike Kravetz <mike.kravetz@oracle.com>
Cc: Vlastimil Babka <vbabka@suse.cz>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	linux-api@vger.kernel.org,
	Reinette Chatre <reinette.chatre@intel.com>,
	Christopher Lameter <cl@linux.com>,
	Guy Shattah <sguy@mellanox.com>,
	Anshuman Khandual <khandual@linux.vnet.ibm.com>,
	Michal Nazarewicz <mina86@mina86.com>,
	David Nellans <dnellans@nvidia.com>,
	Laura Abbott <labbott@redhat.com>, Pavel Machek <pavel@ucw.cz>,
	Dave Hansen <dave.hansen@intel.com>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 2/3] mm: add find_alloc_contig_pages() interface
Date: Wed, 6 Jun 2018 11:32:14 +0200	[thread overview]
Message-ID: <20180606093214.GE32433@dhcp22.suse.cz> (raw)
In-Reply-To: <af56d281-fea1-2bd7-aff1-108fa1d9f3be@oracle.com>

On Wed 02-05-18 14:13:32, Mike Kravetz wrote:
> On 04/21/2018 09:16 AM, Vlastimil Babka wrote:
> > On 04/17/2018 04:09 AM, Mike Kravetz wrote:
> >> find_alloc_contig_pages() is a new interface that attempts to locate
> >> and allocate a contiguous range of pages.  It is provided as a more
> >> convenient interface than alloc_contig_range() which is currently
> >> used by CMA and gigantic huge pages.
> >>
> >> When attempting to allocate a range of pages, migration is employed
> >> if possible.  There is no guarantee that the routine will succeed.
> >> So, the user must be prepared for failure and have a fall back plan.
> >>
> >> Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
> > 
> > Hi, just two quick observations, maybe discussion pointers for the
> > LSF/MM session:
> > - it's weird that find_alloc_contig_pages() takes an order, and
> > free_contig_pages() takes a nr_pages. I suspect the interface would be
> > more future-proof with both using nr_pages? Perhaps also minimum
> > alignment for the allocation side? Order is fine for hugetlb, but what
> > about other potential users?
> 
> Agreed, and I am changing this to nr_pages and adding alignment.
> 
> > - contig_alloc_migratetype_ok() says that MIGRATE_CMA blocks are OK to
> > allocate from. This silently assumes that everything allocated by this
> > will be migratable itself, or it might eat CMA reserves. Is it the case?
> > Also you then call alloc_contig_range() with MIGRATE_MOVABLE, so it will
> > skip/fail on MIGRATE_CMA anyway IIRC.
> 
> When looking closer at the code, alloc_contig_range currently has comments
> saying migratetype must be MIGRATE_MOVABLE or MIGRATE_CMA.  However, this
> is not checked/enforced anywhere in the code (that I can see).  The
> migratetype passed to alloc_contig_range() will be used to set the migrate
> type of all pageblocks in the range.  If there is an error, one side effect
> is that some pageblocks may have their migrate type changed to migratetype.
> Depending on how far we got before hitting the error, the number of pageblocks
> changed is unknown.  This actually can happen at the lower level routine
> start_isolate_page_range().
> 
> My first thought was to make start_isolate_page_range/set_migratetype_isolate
> check that the migrate type of a pageblock was migratetype before isolating.
> This would work for CMA, and I could make it work for the new allocator.
> However, offline_pages also calls start_isolate_page_range and I believe we
> do not want to enforce such a rule (all pageblocks must be of the same migrate
> type) for memory hotplug/offline?
> 
> Should we be concerned at all about this potential changing of migrate type
> on error?  The only way I can think to avoid this is to save the original
> migrate type before isolation.

This is more a question to Vlastimil, Joonsoo. But my understanding is
that it doesn't matter. MIGRATE_MOVABLE will not block other
allocations. So we seem to need it only for MIGRATE_CMA. The later
should die sooner or later hopefully so this awful kludge should just
die with it.
-- 
Michal Hocko
SUSE Labs

  reply	other threads:[~2018-06-06  9:32 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-17  2:09 [PATCH 0/3] Interface for higher order contiguous allocations Mike Kravetz
2018-04-17  2:09 ` [PATCH 1/3] mm: change type of free_contig_range(nr_pages) to unsigned long Mike Kravetz
2018-04-17  2:09 ` [PATCH 2/3] mm: add find_alloc_contig_pages() interface Mike Kravetz
2018-04-17 12:10   ` kbuild test robot
2018-04-18  1:39     ` Mike Kravetz
2018-04-17 14:19   ` kbuild test robot
2018-04-21 16:16   ` Vlastimil Babka
2018-05-02 21:13     ` Mike Kravetz
2018-06-06  9:32       ` Michal Hocko [this message]
2018-04-23  0:09   ` Michal Hocko
2018-04-23  4:22     ` Mike Kravetz
2018-04-24 13:37       ` Michal Hocko
2018-04-17  2:09 ` [PATCH 3/3] mm/hugetlb: use find_alloc_contig_pages() to allocate gigantic pages Mike Kravetz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180606093214.GE32433@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=dave.hansen@intel.com \
    --cc=dnellans@nvidia.com \
    --cc=khandual@linux.vnet.ibm.com \
    --cc=labbott@redhat.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mike.kravetz@oracle.com \
    --cc=mina86@mina86.com \
    --cc=pavel@ucw.cz \
    --cc=reinette.chatre@intel.com \
    --cc=sguy@mellanox.com \
    --cc=vbabka@suse.cz \
    --subject='Re: [PATCH 2/3] mm: add find_alloc_contig_pages() interface' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).