Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: osalvador@suse.de
Cc: akpm@linux-foundation.org, dan.j.williams@intel.com,
	pavel.tatashin@microsoft.com, jglisse@redhat.com,
	Jonathan.Cameron@huawei.com, rafael@kernel.org, david@redhat.com,
	linux-mm@kvack.org, owner-linux-mm@kvack.org
Subject: Re: [PATCH v2 5/5] mm, memory_hotplug: Refactor shrink_zone/pgdat_span
Date: Wed, 28 Nov 2018 13:31:20 +0100
Message-ID: <20181128123120.GJ6923@dhcp22.suse.cz> (raw)
In-Reply-To: <ddee6546c35aaada14b196c83f5205e0@suse.de>

On Wed 28-11-18 12:00:35, osalvador@suse.de wrote:
> 
> > OK, so what is the difference between memory hotremoving a range withing
> > a zone and on the zone boundary? There should be none, yet spanned pages
> > do get updated only when we do the later, IIRC? So spanned pages is not
> > really all that valuable information. It just tells the
> > zone_end-zone_start. Also not what is the semantic of
> > spanned_pages for interleaving zones.
> 
> Ok, I think I start getting your point.
> Yes, spanned_pages are only touched in case we remove the first or the last
> section of memory range.
> 
> So your point is to get rid of shrink_zone_span() and shrink_node_span(),
> and do not touch spanned_pages at all? (only when the zone is gone or the
> node
> goes offline?)

yep. Or when we extend a zone/node via hotplug.

> The only thing I am worried about is that by doing that, the system
> will account spanned_pages incorrectly.

As long as end_pfn - start_pfn matches then I do not see what would be
incorrect.

> So, if we remove pages on zone-boundary, neither zone_start_pfn nor
> spanned_pages will change.
> I did not check yet, but could it be that somewhere we use zone/node's
> spanned_pages
> information to compute something?

That is an obvious homework to do when posting such a patch ;)

> I mean, do not get me wrong, getting rid of all shrink stuff would be great,
> it will remove a __lot__ of code and some complexity, but I am not sure if
> it is totally safe.

Yes it is a lot of code and I do not see any strong justification for
it. In the past the zone boundary was really important becuase it
defined the memory zone for the new memory to hotplug. For quite some
time we have a much more flexible semantic and you can online memory to
normal/movable zones as you like. So I _believe_ the need for shrink
code is gone. Maybe I am missing something of course. All I want to say
is that it would be _so_ great to get rid of it rather than waste a lip
stick on a pig...
-- 
Michal Hocko
SUSE Labs

  reply index

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-27 16:20 [PATCH v2 0/5] Do not touch pages in hot-remove path Oscar Salvador
2018-11-27 16:20 ` [PATCH v2 1/5] mm, memory_hotplug: Add nid parameter to arch_remove_memory Oscar Salvador
2018-11-27 16:20 ` [PATCH v2 2/5] kernel, resource: Check for IORESOURCE_SYSRAM in release_mem_region_adjustable Oscar Salvador
2018-11-27 16:20 ` [PATCH v2 3/5] mm, memory_hotplug: Move zone/pages handling to offline stage Oscar Salvador
2018-11-28  7:52   ` Mike Rapoport
2018-11-28 14:25     ` osalvador
2018-11-28 14:15   ` osalvador
2018-11-27 16:20 ` [PATCH v2 4/5] mm, memory-hotplug: Rework unregister_mem_sect_under_nodes Oscar Salvador
2019-03-24  6:48   ` Anshuman Khandual
2019-03-25  7:40     ` Oscar Salvador
2019-03-25  8:04       ` Michal Hocko
2019-03-25  8:14         ` Oscar Salvador
2018-11-27 16:20 ` [PATCH v2 5/5] mm, memory_hotplug: Refactor shrink_zone/pgdat_span Oscar Salvador
2018-11-28  6:50   ` Michal Hocko
2018-11-28  7:07     ` Oscar Salvador
2018-11-28 10:03       ` David Hildenbrand
2018-11-28 10:14       ` Michal Hocko
2018-11-28 11:00         ` osalvador
2018-11-28 12:31           ` Michal Hocko [this message]
2018-11-28 12:51             ` osalvador
2018-11-28 13:08               ` Michal Hocko
2018-11-28 13:18                 ` osalvador
2018-11-28 15:50                   ` Michal Hocko
2018-11-28 16:02                     ` osalvador
2018-11-29  9:29                     ` osalvador
2018-11-28 13:09               ` osalvador

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=20181128123120.GJ6923@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=akpm@linux-foundation.org \
    --cc=dan.j.williams@intel.com \
    --cc=david@redhat.com \
    --cc=jglisse@redhat.com \
    --cc=linux-mm@kvack.org \
    --cc=osalvador@suse.de \
    --cc=owner-linux-mm@kvack.org \
    --cc=pavel.tatashin@microsoft.com \
    --cc=rafael@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

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