Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: linux-mm@kvack.org, Michael Ellerman <mpe@ellerman.id.au>,
	Vlastimil Babka <vbabka@suse.cz>,
	Andrew Morton <akpm@linux-foundation.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Reza Arbab <arbab@linux.vnet.ibm.com>,
	Yasuaki Ishimatsu <yasu.isimatu@gmail.com>,
	qiuxishi@huawei.com, Igor Mammedov <imammedo@redhat.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] mm: drop migrate type checks from has_unmovable_pages
Date: Fri, 20 Oct 2017 15:50:14 +0900
Message-ID: <20171020065014.GA11145@js1304-P5Q-DELUXE> (raw)
In-Reply-To: <20171020055922.x2mj6j66obmp52da@dhcp22.suse.cz>

On Fri, Oct 20, 2017 at 07:59:22AM +0200, Michal Hocko wrote:
> On Fri 20-10-17 11:13:29, Joonsoo Kim wrote:
> > On Thu, Oct 19, 2017 at 02:21:18PM +0200, Michal Hocko wrote:
> > > On Thu 19-10-17 10:20:41, Michal Hocko wrote:
> > > > On Thu 19-10-17 16:33:56, Joonsoo Kim wrote:
> > > > > On Thu, Oct 19, 2017 at 09:15:03AM +0200, Michal Hocko wrote:
> > > > > > On Thu 19-10-17 11:51:11, Joonsoo Kim wrote:
> > > > [...]
> > > > > > > Hello,
> > > > > > > 
> > > > > > > This patch will break the CMA user. As you mentioned, CMA allocation
> > > > > > > itself isn't migrateable. So, after a single page is allocated through
> > > > > > > CMA allocation, has_unmovable_pages() will return true for this
> > > > > > > pageblock. Then, futher CMA allocation request to this pageblock will
> > > > > > > fail because it requires isolating the pageblock.
> > > > > > 
> > > > > > Hmm, does this mean that the CMA allocation path depends on
> > > > > > has_unmovable_pages to return false here even though the memory is not
> > > > > > movable? This sounds really strange to me and kind of abuse of this
> > > > > 
> > > > > Your understanding is correct. Perhaps, abuse or wrong function name.
> > > > >
> > > > > > function. Which path is that? Can we do the migrate type test theres?
> > > > > 
> > > > > alloc_contig_range() -> start_isolate_page_range() ->
> > > > > set_migratetype_isolate() -> has_unmovable_pages()
> > > > 
> > > > I see. It seems that the CMA and memory hotplug have a very different
> > > > view on what should happen during isolation.
> > > >  
> > > > > We can add one argument, 'XXX' to set_migratetype_isolate() and change
> > > > > it to check migrate type rather than has_unmovable_pages() if 'XXX' is
> > > > > specified.
> > > > 
> > > > Can we use the migratetype argument and do the special thing for
> > > > MIGRATE_CMA? Like the following diff?
> > > 
> > > And with the full changelog.
> > > ---
> > > >From 8cbd811d741f5dd93d1b21bb3ef94482a4d0bd32 Mon Sep 17 00:00:00 2001
> > > From: Michal Hocko <mhocko@suse.com>
> > > Date: Thu, 19 Oct 2017 14:14:02 +0200
> > > Subject: [PATCH] mm: distinguish CMA and MOVABLE isolation in
> > >  has_unmovable_pages
> > > 
> > > Joonsoo has noticed that "mm: drop migrate type checks from
> > > has_unmovable_pages" would break CMA allocator because it relies on
> > > has_unmovable_pages returning false even for CMA pageblocks which in
> > > fact don't have to be movable:
> > > alloc_contig_range
> > >   start_isolate_page_range
> > >     set_migratetype_isolate
> > >       has_unmovable_pages
> > > 
> > > This is a result of the code sharing between CMA and memory hotplug
> > > while each one has a different idea of what has_unmovable_pages should
> > > return. This is unfortunate but fixing it properly would require a lot
> > > of code duplication.
> > > 
> > > Fix the issue by introducing the requested migrate type argument
> > > and special case MIGRATE_CMA case where CMA page blocks are handled
> > > properly. This will work for memory hotplug because it requires
> > > MIGRATE_MOVABLE.
> > 
> > Unfortunately, alloc_contig_range() can be called with
> > MIGRATE_MOVABLE so this patch cannot perfectly fix the problem.
> 
> Yes, alloc_contig_range can be called with MIGRATE_MOVABLE but my
> understanding is that only CMA allocator really depends on this weird
> semantic and that does MIGRATE_CMA unconditionally.

alloc_contig_range() could be called for partial pages in the
pageblock. With your patch, this case also fails unnecessarilly if the
other pages in the pageblock is pinned.

Until now, there is no user calling alloc_contig_range() with partial
pages except CMA allocator but API could support it.

> 
> > I did a more thinking and found that it's strange to check if there is
> > unmovable page in the pageblock during the set_migratetype_isolate().
> > set_migratetype_isolate() should be just for setting the migratetype
> > of the pageblock. Checking other things should be done by another
> > place, for example, before calling the start_isolate_page_range() in
> > __offline_pages().
> 
> How do we guarantee the atomicity?

What atomicity do you mean?

Thanks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply index

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-18  7:08 [PATCH v2 0/2] mm, memory_hotplug: redefine memory offline retry logic Michal Hocko
2017-09-18  7:08 ` [PATCH 1/2] mm, memory_hotplug: do not fail offlining too early Michal Hocko
2017-10-10 12:05   ` Michael Ellerman
2017-10-10 12:27     ` Michal Hocko
2017-10-11  2:37       ` Michael Ellerman
2017-10-11  5:19         ` Michael Ellerman
2017-10-11 14:05           ` Anshuman Khandual
2017-10-11 14:16             ` Michal Hocko
2017-10-11  6:51         ` Michal Hocko
2017-10-11  8:04           ` Vlastimil Babka
2017-10-11  8:13             ` Michal Hocko
2017-10-11 11:17               ` Vlastimil Babka
2017-10-11 11:24                 ` Michal Hocko
2017-10-13 11:42             ` Michael Ellerman
2017-10-13 11:58               ` Michal Hocko
2017-10-13 12:00                 ` [PATCH 1/2] mm: drop migrate type checks from has_unmovable_pages Michal Hocko
2017-10-13 12:00                   ` [PATCH 2/2] mm, page_alloc: fail has_unmovable_pages when seeing reserved pages Michal Hocko
2017-10-13 12:04                     ` Vlastimil Babka
2017-10-13 12:07                       ` Michal Hocko
2017-10-17 13:03                         ` Vlastimil Babka
2017-10-17 11:41                   ` [PATCH 1/2] mm: drop migrate type checks from has_unmovable_pages Michael Ellerman
2017-10-17 12:03                     ` Michal Hocko
2017-10-17 13:02                   ` Vlastimil Babka
2017-10-19  2:51                   ` Joonsoo Kim
2017-10-19  7:15                     ` Michal Hocko
2017-10-19  7:33                       ` Joonsoo Kim
2017-10-19  8:20                         ` Michal Hocko
2017-10-19 12:21                           ` Michal Hocko
2017-10-20  2:13                             ` Joonsoo Kim
2017-10-20  5:59                               ` Michal Hocko
2017-10-20  6:50                                 ` Joonsoo Kim [this message]
2017-10-20  7:02                                   ` Michal Hocko
2017-10-23  5:23                                     ` Joonsoo Kim
2017-10-23  8:10                                       ` Michal Hocko
2017-10-24  4:44                                         ` Joonsoo Kim
2017-10-24  7:44                                           ` Michal Hocko
2017-10-24  8:12                                           ` Vlastimil Babka
2017-10-24 12:25                                             ` Michal Hocko
2017-10-26  2:47                                             ` Joonsoo Kim
2017-10-26  7:41                                               ` Michal Hocko
2017-10-20  7:22                               ` Xishi Qiu
2017-10-20  8:17                                 ` Michal Hocko
2017-10-23  5:26                                   ` Joonsoo Kim
2017-10-26 13:04                             ` Vlastimil Babka
2017-10-26 13:59                             ` Michal Hocko
2017-09-18  7:08 ` [PATCH 2/2] mm, memory_hotplug: remove timeout from __offline_memory Michal Hocko
     [not found] <AM3PR04MB14892A9D6D2FBCE21B8C1F0FF12B0@AM3PR04MB1489.eurprd04.prod.outlook.com>
2017-11-13  7:33 ` [PATCH 1/2] mm: drop migrate type checks from has_unmovable_pages Ran Wang
2017-11-13 11:02   ` Michal Hocko
2017-11-14  6:10     ` Ran Wang
2017-11-14  7:06       ` Michal Hocko
2017-11-14  7:45         ` Ran Wang

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=20171020065014.GA11145@js1304-P5Q-DELUXE \
    --to=iamjoonsoo.kim@lge.com \
    --cc=akpm@linux-foundation.org \
    --cc=arbab@linux.vnet.ibm.com \
    --cc=imammedo@redhat.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=qiuxishi@huawei.com \
    --cc=vbabka@suse.cz \
    --cc=vkuznets@redhat.com \
    --cc=yasu.isimatu@gmail.com \
    /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

Linux-mm Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mm linux-mm/ https://lore.kernel.org/linux-mm \
		linux-mm@kvack.org
	public-inbox-index linux-mm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kvack.linux-mm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git