linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
To: Pavel Tatashin <pasha.tatashin@gmail.com>,
	linux-mm@kvack.org, akpm@linux-foundation.org
Cc: pavel.tatashin@microsoft.com, mhocko@suse.com,
	dave.jiang@intel.com, linux-kernel@vger.kernel.org,
	willy@infradead.org, davem@davemloft.net,
	yi.z.zhang@linux.intel.com, khalid.aziz@oracle.com,
	rppt@linux.vnet.ibm.com, vbabka@suse.cz,
	sparclinux@vger.kernel.org, dan.j.williams@intel.com,
	ldufour@linux.vnet.ibm.com, mgorman@techsingularity.net,
	mingo@kernel.org, kirill.shutemov@linux.intel.com
Subject: Re: [mm PATCH v3 2/6] mm: Drop meminit_pfn_in_nid as it is redundant
Date: Tue, 16 Oct 2018 13:49:14 -0700	[thread overview]
Message-ID: <e4f806d4-2527-07c2-56bc-9c41789d669c@linux.intel.com> (raw)
In-Reply-To: <a21c1827-10ad-8ff5-c8b6-e34a3f1e8b80@gmail.com>

On 10/16/2018 1:33 PM, Pavel Tatashin wrote:
> 
> 
> On 10/15/18 4:27 PM, Alexander Duyck wrote:
>> As best as I can tell the meminit_pfn_in_nid call is completely redundant.
>> The deferred memory initialization is already making use of
>> for_each_free_mem_range which in turn will call into __next_mem_range which
>> will only return a memory range if it matches the node ID provided assuming
>> it is not NUMA_NO_NODE.
>>
>> I am operating on the assumption that there are no zones or pgdata_t
>> structures that have a NUMA node of NUMA_NO_NODE associated with them. If
>> that is the case then __next_mem_range will never return a memory range
>> that doesn't match the zone's node ID and as such the check is redundant.
>>
>> So one piece I would like to verfy on this is if this works for ia64.
>> Technically it was using a different approach to get the node ID, but it
>> seems to have the node ID also encoded into the memblock. So I am
>> assuming this is okay, but would like to get confirmation on that.
>>
>> Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> 
> If I am not mistaken, this code is for systems with memory interleaving.
> Quick looks shows that x86, powerpc, s390, and sparc have it set.
> 
> I am not sure about other arches, but at least on SPARC, there are some
> processors with memory interleaving feature:
> 
> http://www.fujitsu.com/global/products/computing/servers/unix/sparc-enterprise/technology/performance/memory.html
> 
> Pavel

I get what it is for. However as best I can tell the check is actually 
redundant. In the case of the deferred page initialization we are 
already pulling the memory regions via "for_each_free_mem_range". That 
function is already passed a NUMA node ID. Because of that we are 
already checking the memory range to determine if it is in the node or 
not. As such it doesn't really make sense to go through for each PFN and 
then go back to the memory range and see if the node matches or not.

You can take a look at __next_mem_range which is called by 
for_each_free_mem_range and passed &memblock.memory and 
&memblock.reserved to avoid:
https://elixir.bootlin.com/linux/latest/source/mm/memblock.c#L899

Then you can work your way through:
meminit_pfn_in_nid(pfn, node, state)
  __early_pfn_to_nid(pfn, state)
   memblock_search_pfn_nid(pfn, &start_pfn, &end_pfn)
    memblock_search(&memblock.memory, pfn)

 From what I can tell the deferred init is going back through the 
memblock.memory list we pulled this range from and just validating it 
against itself. This makes sense for the standard init as that is just 
going from start_pfn->end_pfn, but for the deferred init we are pulling 
the memory ranges ahead of time so we shouldn't need to re-validate the 
memory that is contained within that range.

  reply	other threads:[~2018-10-16 20:49 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-15 20:26 [mm PATCH v3 0/6] Deferred page init improvements Alexander Duyck
2018-10-15 20:26 ` [mm PATCH v3 1/6] mm: Use mm_zero_struct_page from SPARC on all 64b architectures Alexander Duyck
2018-10-16 19:01   ` Pavel Tatashin
2018-10-17  7:30     ` Mike Rapoport
2018-10-17 14:52       ` Alexander Duyck
2018-10-17  8:47   ` Michal Hocko
2018-10-17 15:07     ` Alexander Duyck
2018-10-17 15:12       ` Pavel Tatashin
2018-10-17 15:40         ` David Laight
2018-10-17 16:31           ` Alexander Duyck
2018-10-17 17:08             ` Pavel Tatashin
2018-10-17 16:34       ` Michal Hocko
2018-10-15 20:27 ` [mm PATCH v3 2/6] mm: Drop meminit_pfn_in_nid as it is redundant Alexander Duyck
2018-10-16 20:33   ` Pavel Tatashin
2018-10-16 20:49     ` Alexander Duyck [this message]
2018-10-16 21:06       ` Pavel Tatashin
2018-10-17  9:04   ` Michal Hocko
2018-10-15 20:27 ` [mm PATCH v3 3/6] mm: Use memblock/zone specific iterator for handling deferred page init Alexander Duyck
2018-10-17  9:11   ` Michal Hocko
2018-10-17 15:17     ` Alexander Duyck
2018-10-17 16:42   ` Mike Rapoport
2018-10-15 20:27 ` [mm PATCH v3 4/6] mm: Move hot-plug specific memory init into separate functions and optimize Alexander Duyck
2018-10-17  9:18   ` Michal Hocko
2018-10-17 15:26     ` Alexander Duyck
2018-10-24 12:36       ` Michal Hocko
2018-10-24 15:08         ` Alexander Duyck
2018-10-24 15:27           ` Michal Hocko
2018-10-24 17:35             ` Alexander Duyck
2018-10-25 12:41               ` Michal Hocko
2018-10-15 20:27 ` [mm PATCH v3 5/6] mm: Use common iterator for deferred_init_pages and deferred_free_pages Alexander Duyck
2018-10-15 20:27 ` [mm PATCH v3 6/6] mm: Add reserved flag setting to set_page_links Alexander Duyck

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=e4f806d4-2527-07c2-56bc-9c41789d669c@linux.intel.com \
    --to=alexander.h.duyck@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=dan.j.williams@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=davem@davemloft.net \
    --cc=khalid.aziz@oracle.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=ldufour@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=mhocko@suse.com \
    --cc=mingo@kernel.org \
    --cc=pasha.tatashin@gmail.com \
    --cc=pavel.tatashin@microsoft.com \
    --cc=rppt@linux.vnet.ibm.com \
    --cc=sparclinux@vger.kernel.org \
    --cc=vbabka@suse.cz \
    --cc=willy@infradead.org \
    --cc=yi.z.zhang@linux.intel.com \
    --subject='Re: [mm PATCH v3 2/6] mm: Drop meminit_pfn_in_nid as it is redundant' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).