Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, Mel Gorman <mgorman@suse.de>,
	Andrea Arcangeli <aarcange@redhat.com>,
	Jerome Glisse <jglisse@redhat.com>,
	Reza Arbab <arbab@linux.vnet.ibm.com>,
	Yasuaki Ishimatsu <yasu.isimatu@gmail.com>,
	qiuxishi@huawei.com, Kani Toshimitsu <toshi.kani@hpe.com>,
	slaoub@gmail.com, Joonsoo Kim <js1304@gmail.com>,
	Andi Kleen <ak@linux.intel.com>,
	David Rientjes <rientjes@google.com>,
	Daniel Kiper <daniel.kiper@oracle.com>,
	Igor Mammedov <imammedo@redhat.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 07/14] mm: consider zone which is not fully populated to have holes
Date: Thu, 18 May 2017 18:42:10 +0200
Message-ID: <20170518164210.GD18333@dhcp22.suse.cz> (raw)
In-Reply-To: <ae859e14-bf82-ae37-9c85-d4b31ce89b0a@suse.cz>

On Thu 18-05-17 18:14:39, Vlastimil Babka wrote:
> On 05/15/2017 10:58 AM, Michal Hocko wrote:
[...]
> >  #ifdef CONFIG_MEMORY_HOTPLUG
> > +/*
> > + * Return page for the valid pfn only if the page is online. All pfn
> > + * walkers which rely on the fully initialized page->flags and others
> > + * should use this rather than pfn_valid && pfn_to_page
> > + */
> > +#define pfn_to_online_page(pfn)				\
> > +({							\
> > +	struct page *___page = NULL;			\
> > +							\
> > +	if (online_section_nr(pfn_to_section_nr(pfn)))	\
> > +		___page = pfn_to_page(pfn);		\
> > +	___page;					\
> > +})
> 
> This seems to be already assuming pfn_valid() to be true. There's no
> "pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS" check and the comment
> suggests as such, but...

Yes, we should check the validity of the section number. We do not have
to check whether the section is valid because online sections are a
subset of those that are valid.

> > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> > index 05796ee974f7..c3a146028ba6 100644
> > --- a/mm/memory_hotplug.c
> > +++ b/mm/memory_hotplug.c
> > @@ -929,6 +929,9 @@ static int online_pages_range(unsigned long start_pfn, unsigned long nr_pages,
> >  	unsigned long i;
> >  	unsigned long onlined_pages = *(unsigned long *)arg;
> >  	struct page *page;
> > +
> > +	online_mem_sections(start_pfn, start_pfn + nr_pages);
> 
> Shouldn't this be moved *below* the loop that initializes struct pages?
> In the offline case you do mark sections offline before "tearing" struct
> pages, so that should be symmetric.

You are right! Andrew, could you fold the following intot the patch?
---

  reply index

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-15  8:58 [PATCH -v4 0/14] mm: make movable onlining suck less Michal Hocko
2017-05-15  8:58 ` [PATCH 01/14] mm: remove return value from init_currently_empty_zone Michal Hocko
2017-05-15  8:58 ` [PATCH 02/14] mm, memory_hotplug: use node instead of zone in can_online_high_movable Michal Hocko
2017-05-15  8:58 ` [PATCH 03/14] mm: drop page_initialized check from get_nid_for_pfn Michal Hocko
2017-05-15  8:58 ` [PATCH 04/14] mm, memory_hotplug: get rid of is_zone_device_section Michal Hocko
2017-06-10  9:56   ` Wei Yang
2017-06-10 14:58     ` Wei Yang
2017-06-12  6:49       ` Michal Hocko
2017-06-14  6:17         ` Wei Yang
2017-06-14  6:12     ` Wei Yang
2017-06-14  6:32       ` Michal Hocko
2017-06-14  9:12         ` Wei Yang
2017-06-14  9:24           ` Michal Hocko
2017-06-15  1:02             ` Wei Yang
2017-05-15  8:58 ` [PATCH 05/14] mm, memory_hotplug: split up register_one_node Michal Hocko
2017-05-15  8:58 ` [PATCH 06/14] mm, memory_hotplug: consider offline memblocks removable Michal Hocko
2017-05-15  8:58 ` [PATCH 07/14] mm: consider zone which is not fully populated to have holes Michal Hocko
2017-05-18 16:14   ` Vlastimil Babka
2017-05-18 16:42     ` Michal Hocko [this message]
2017-05-19  7:21       ` Vlastimil Babka
2017-05-15  8:58 ` [PATCH 08/14] mm, compaction: skip over holes in __reset_isolation_suitable Michal Hocko
2017-05-19  7:23   ` Vlastimil Babka
2017-05-15  8:58 ` [PATCH 09/14] mm: __first_valid_page skip over offline pages Michal Hocko
2017-05-19  8:05   ` Vlastimil Babka
2017-05-15  8:58 ` [PATCH 10/14] mm, vmstat: skip reporting offline pages in pagetypeinfo Michal Hocko
2017-05-19  8:17   ` Vlastimil Babka
2017-05-15  8:58 ` [PATCH 11/14] mm, memory_hotplug: do not associate hotadded memory to zones until online Michal Hocko
2017-05-19  8:56   ` Vlastimil Babka
2017-06-16  4:20   ` Wei Yang
2017-06-16  8:05     ` Michal Hocko
2017-06-16  8:11   ` Wei Yang
2017-06-16  8:45     ` Michal Hocko
2017-06-16  9:11       ` Wei Yang
2017-06-25  0:14   ` Wei Yang
2017-06-26  5:38     ` Michal Hocko
2017-05-15  8:58 ` [PATCH 12/14] mm, memory_hotplug: replace for_device by want_memblock in arch_add_memory Michal Hocko
2017-05-15  8:58 ` [PATCH 13/14] mm, memory_hotplug: fix the section mismatch warning Michal Hocko
2017-05-15  8:58 ` [PATCH 14/14] mm, memory_hotplug: remove unused cruft after memory hotplug rework Michal Hocko
2017-06-09  9:51 ` [PATCH -v4 0/14] mm: make movable onlining suck less Wei Yang
2017-06-09 10:49   ` Michal Hocko
2017-06-10  2:20     ` Wei Yang

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=20170518164210.GD18333@dhcp22.suse.cz \
    --to=mhocko@kernel.org \
    --cc=aarcange@redhat.com \
    --cc=ak@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=arbab@linux.vnet.ibm.com \
    --cc=daniel.kiper@oracle.com \
    --cc=imammedo@redhat.com \
    --cc=jglisse@redhat.com \
    --cc=js1304@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=qiuxishi@huawei.com \
    --cc=rientjes@google.com \
    --cc=slaoub@gmail.com \
    --cc=toshi.kani@hpe.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