linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Sharath Kumar Bhat <sharath.k.bhat@linux.intel.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: Sharath Kumar Bhat <sharath.k.bhat@linux.intel.com>,
	Dave Hansen <dave.hansen@intel.com>,
	linux-mm@kvack.org, akpm@linux-foundation.org
Subject: Re: [PATCH] mm: fix movable_node kernel command-line
Date: Wed, 25 Oct 2017 15:01:32 -0700	[thread overview]
Message-ID: <20171025220132.GA2614@linux.intel.com> (raw)
In-Reply-To: <20171025063852.nunaquo5wevayejf@dhcp22.suse.cz>

On Wed, Oct 25, 2017 at 08:38:52AM +0200, Michal Hocko wrote:
> On Tue 24-10-17 17:53:14, Sharath Kumar Bhat wrote:
> > On Tue, Oct 24, 2017 at 09:19:06AM +0200, Michal Hocko wrote:
> > > On Mon 23-10-17 18:06:33, Sharath Kumar Bhat wrote:
> [...]
> > > > And moreover
> > > > 'movable_node' is implemented with an assumption to provide the entire
> > > > hotpluggable memory as movable zone. This ACPI override would be against
> > > > that assumption.
> > > 
> > > This is true and in fact movable_node should become movable_memory over
> > > time and only ranges marked as movable would become really movable. This
> > > is a rather non-trivial change to do and there is not a great demand for
> > > the feature so it is low on my TODO list.
> > 
> > Do you mean to have a single kernel command-line 'movable_memory=' for this
> > purpose and remove all other kernel command-line parameters such as
> > 'kernelcore=', 'movablecore=' and 'movable_node'?
> 
> yes.

Ok then I believe it will let user to specify multiple memory ranges so
that admin can explicitly choose to have movable zones in either
hotpluggable or non-hotpluggable memories. Because in this use case the
requirement is to have the movable zones in both hotpluggable and
non-hotpluggable memories.

> 
> > because after the kernel
> > boots up we can not gurantee that a contig memory range can be made zone
> > movable since any kernel allocations could pre-exist.
> 
> No, I meant that the zone association would be done _only_ based by
> memory attributes exported by ACPI or whatever is used to configure
> memory ranges on the particular platform. So an early init code.
> 
> > > > Also ACPI override would introduce additional topology
> > > > changes. Again this would have to change every time the total movable
> > > > memory requirement changes and the whole system and apps have to be
> > > > re-tuned (for job launch ex: numactl etc) to comphrehend this change.
> > > 
> > > This is something you have to do anyway when the topology of the system
> > > changes each boot.
> > 
> > No, this is a manual tuning for job-launch, mem policy handling code etc.
> > which would be done once for a platform. But in this case based on the
> > application need the amount of movable memory will change so it is really
> > unfair to ask user to re-work their job launch and apps for every such
> > changes.
> 
> I am still confused. Why does the application even care about
> movability?

Right its not about movability, since 'movable_node' assumes that the entire
memory node is hotpluggable, to stay compatible with it the memory ranges of
non-hotpluggable memory that we want to be movable zone should be exposed as
a complete node. This increases the number of NUMA nodes and the total
no.of such nodes changes as the movable memory requirement changes.

>  
> > > That being said, I would really prefer to actually _remove_ kernel_core
> > > parameter altogether. It is messy (just look at find_zone_movable_pfns_for_nodes
> > > at al.) and the original usecase it has been added for [1] does not hold
> > > anymore. Adding more stuff to workaround issues which can be handled
> > > more cleanly is definitely not a right way to go.
> > 
> > I agree that kernelcore handling is non-trivial in that function. But the
> > changes introduced by this patch are under 'movable_node' case handling in
> > find_zone_movable_pfns_for_nodes() and it does not cause any change to the
> > existing kernelcore behavior of the code. Also this enables all
> > multi-kernel users to make use of this functionality untill later when
> > new interface would be available for the same purpose.
> 
> The point is to not build on top and rather get rid of it completely.

I thought you mentioned its a low priority on the TODO list and you
dont expect to see it in the near future. So till then there is no
existing solution that one case use.

>  
> > > [1] note that MOVABLE_ZONE has been originally added to help the
> > > fragmentation avoidance.
> > 
> > Isn't this true even now since ZONE_MOVABLE will populate only
> > MIGRATE_MOVABLE free list of pages? and other zones could have
> > MIGRATE_UNMOVABLE pages?
> 
> My point was that the original motivation is gone because our compaction
> code doesn't really depend on movable zone. So the movable zone is more
> about making sure that the specific memory is migratable and so
> offlineable.
> 
> -- 
> 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	other threads:[~2017-10-25 22:01 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-20 23:32 Sharath Kumar Bhat
2017-10-23 12:52 ` Michal Hocko
2017-10-23 16:03   ` Sharath Kumar Bhat
2017-10-23 16:15     ` Michal Hocko
2017-10-23 17:14       ` Sharath Kumar Bhat
2017-10-23 17:20         ` Michal Hocko
2017-10-23 17:35           ` Sharath Kumar Bhat
2017-10-23 17:49             ` Michal Hocko
2017-10-23 18:48               ` Sharath Kumar Bhat
2017-10-23 19:04                 ` Michal Hocko
2017-10-23 19:25                   ` Sharath Kumar Bhat
2017-10-23 19:35                     ` Michal Hocko
2017-10-23 19:56                       ` Sharath Kumar Bhat
2017-10-23 21:52                         ` Dave Hansen
2017-10-24  1:06                           ` Sharath Kumar Bhat
2017-10-24  7:19                             ` Michal Hocko
2017-10-25  0:53                               ` Sharath Kumar Bhat
2017-10-25  6:38                                 ` Michal Hocko
2017-10-25 22:01                                   ` Sharath Kumar Bhat [this message]
2017-10-26  7:36                                     ` Michal Hocko

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=20171025220132.GA2614@linux.intel.com \
    --to=sharath.k.bhat@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=dave.hansen@intel.com \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox