All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michal Hocko <mhocko@suse.com>
To: Oscar Salvador <osalvador@suse.de>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	David Hildenbrand <david@redhat.com>,
	Anshuman Khandual <anshuman.khandual@arm.com>,
	Pavel Tatashin <pasha.tatashin@soleen.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v9 3/8] mm,memory_hotplug: Factor out adjusting present pages into adjust_present_page_count()
Date: Wed, 21 Apr 2021 10:31:30 +0200	[thread overview]
Message-ID: <YH/i4nfrqt2k0mzZ@dhcp22.suse.cz> (raw)
In-Reply-To: <20210421080036.GC22456@linux>

On Wed 21-04-21 10:00:36, Oscar Salvador wrote:
> On Tue, Apr 20, 2021 at 11:45:55AM +0200, Michal Hocko wrote:
> > On Fri 16-04-21 13:24:06, Oscar Salvador wrote:
> > > From: David Hildenbrand <david@redhat.com>
> > > 
> > > Let's have a single place (inspired by adjust_managed_page_count()) where
> > > we adjust present pages.
> > > In contrast to adjust_managed_page_count(), only memory onlining/offlining
> > > is allowed to modify the number of present pages.
> > > 
> > > Signed-off-by: David Hildenbrand <david@redhat.com>
> > > Signed-off-by: Oscar Salvador <osalvador@suse.de>
> > > Reviewed-by: Oscar Salvador <osalvador@suse.de>
> > 
> > Not sure self review counts ;)
> 
> Uhm, the original author is David, I just added my signed-off-by as a deliverer.
> I thought that in that case was ok to stick my Reviewed-by.
> Or maybe my signed-off-by carries that implicitly.

Yeah I do expect that one should review own changes but this is not
really anything to lose sleep over.

> > Acked-by: Michal Hocko <mhocko@suse.com>
> > 
> > Btw. I strongly suspect the resize lock is quite pointless here.
> > Something for a follow up patch.
> 
> What makes you think that?

         * Write access to present_pages at runtime should be protected by
         * mem_hotplug_begin/end(). Any reader who can't tolerant drift of
         * present_pages should get_online_mems() to get a stable value.

> I have been thinking about this, let us ignore this patch for a moment.
> 
> If I poked the code correctly, node_size_lock is taken in:
> 
> remove_pfn_range_from_zone()
> move_pfn_range_to_zone()
> 
> both of them handling {zone,node}->spanned_pages
> 
> Then we take it in {offline,online}_pages() for {zone,node}->present_pages.
> 
> The other places where we take it are __init functions, so not of interest.
> 
> Given that {offline,online}_pages() is serialized by the memory_hotplug lock,
> I would say that {node,zone}->{spanned,present}_pages is, at any time, stable?
> So, no need for the lock even without considering this patch?

Yes. The resize lock is really only relevant to the parallel struct page
initialization during early boot. The hotplug usage seems just a left
over from the past or maybe it has never been really relevant in that
context.

> Now, getting back to this patch.
> adjust_present_page_count() will be called from memory_block_online(), which
> is not holding the memory_hotplug lock yet.
> But, we only fiddle with present pages out of {online,offline}_pages() if
> we have vmemmap pages, and since that operates on the same memory block,
> its lock should serialize that.

Memory hotplug is always synchronized on the device level.
-- 
Michal Hocko
SUSE Labs

  parent reply	other threads:[~2021-04-21  8:31 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-16 11:24 [PATCH v9 0/8] Allocate memmap from hotadded memory (per device) Oscar Salvador
2021-04-16 11:24 ` [PATCH v9 1/8] drivers/base/memory: Introduce memory_block_{online,offline} Oscar Salvador
2021-04-20  9:23   ` Michal Hocko
2021-04-16 11:24 ` [PATCH v9 2/8] mm,memory_hotplug: Relax fully spanned sections check Oscar Salvador
2021-04-20  9:40   ` Michal Hocko
2021-04-21  7:37     ` Oscar Salvador
2021-04-16 11:24 ` [PATCH v9 3/8] mm,memory_hotplug: Factor out adjusting present pages into adjust_present_page_count() Oscar Salvador
2021-04-20  9:45   ` Michal Hocko
2021-04-21  8:00     ` Oscar Salvador
2021-04-21  8:06       ` David Hildenbrand
2021-04-21  8:31       ` Michal Hocko [this message]
2021-04-21  8:35         ` Oscar Salvador
2021-04-16 11:24 ` [PATCH v9 4/8] mm,memory_hotplug: Allocate memmap from the added memory range Oscar Salvador
2021-04-20 10:56   ` Michal Hocko
2021-04-21  8:15     ` Oscar Salvador
2021-04-21  8:39       ` Michal Hocko
2021-04-21  8:44         ` David Hildenbrand
2021-04-21  8:49           ` Michal Hocko
2021-04-21  8:52             ` David Hildenbrand
2021-04-21  8:46         ` Oscar Salvador
2021-04-16 11:24 ` [PATCH v9 5/8] acpi,memhotplug: Enable MHP_MEMMAP_ON_MEMORY when supported Oscar Salvador
2021-04-16 11:24 ` [PATCH v9 6/8] mm,memory_hotplug: Add kernel boot option to enable memmap_on_memory Oscar Salvador
2021-04-16 11:24 ` [PATCH v9 7/8] x86/Kconfig: Introduce ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE Oscar Salvador
2021-04-20 10:56   ` Michal Hocko
2021-04-16 11:24 ` [PATCH v9 8/8] arm64/Kconfig: " Oscar Salvador

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=YH/i4nfrqt2k0mzZ@dhcp22.suse.cz \
    --to=mhocko@suse.com \
    --cc=akpm@linux-foundation.org \
    --cc=anshuman.khandual@arm.com \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=osalvador@suse.de \
    --cc=pasha.tatashin@soleen.com \
    --cc=vbabka@suse.cz \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.