Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Vlastimil Babka <vbabka@suse.cz>,
	linux-mm@kvack.org, Michael Ellerman <mpe@ellerman.id.au>,
	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: Thu, 26 Oct 2017 09:41:28 +0200
Message-ID: <20171026074128.ip66zog7ar2bjbb6@dhcp22.suse.cz> (raw)
In-Reply-To: <20171026024707.GA11791@js1304-P5Q-DELUXE>

On Thu 26-10-17 11:47:07, Joonsoo Kim wrote:
> On Tue, Oct 24, 2017 at 10:12:58AM +0200, 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
> 
> This potential problem exists there before Michal's series if the
> migratetype of the target pageblock isn't MIGRATE_MOVABLE or MIGRATE_CMA.
> However, his series enlarges this potential problem surface. It
> would be the problem now even if the migratetype of the target
> pageblock is MIGRATE_MOVABLE.
> 
> > 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
> 
> I'm not very sensitive that which patch is applied first. I can do
> rebase. But, IMHO, correct applying order is my patch first and then
> Michal's series.
> 
> Anyway, Michal, feel free to do what you think correct.

If you do not mind I would rather go with the simple patch first and
then build on top of that. For two reasons 1) it documents the CMA
requirement and 2) there do not seem to be any real users affected by
the issue you are seeing right now. And 3) I really believe
alloc_contig_range needs a deeper thought to be usable in more general
contexts.

> > 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?
> 
> I noticed the problem you mentioned now and, yes, it should be fixed.
> My patch will naturally fixes this issue, too.

I really do not see how.
-- 
Michal Hocko
SUSE Labs

--
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
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 [this message]
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=20171026074128.ip66zog7ar2bjbb6@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=arbab@linux.vnet.ibm.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=imammedo@redhat.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.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