From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: akpm@linux-foundation.org, linux-mm@kvack.org,
sparclinux@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-nvdimm@lists.01.org, davem@davemloft.net,
pavel.tatashin@microsoft.com, mingo@kernel.org,
kirill.shutemov@linux.intel.com, dan.j.williams@intel.com,
dave.jiang@intel.com, rppt@linux.vnet.ibm.com,
willy@infradead.org, vbabka@suse.cz, khalid.aziz@oracle.com,
ldufour@linux.vnet.ibm.com, mgorman@techsingularity.net,
yi.z.zhang@linux.intel.com
Subject: Re: [mm PATCH v5 0/7] Deferred page init improvements
Date: Wed, 14 Nov 2018 16:50:23 -0800 [thread overview]
Message-ID: <9e8218eb-80bf-fc02-ae56-42ccfddb572e@linux.intel.com> (raw)
In-Reply-To: <20181114150742.GZ23419@dhcp22.suse.cz>
On 11/14/2018 7:07 AM, Michal Hocko wrote:
> On Mon 05-11-18 13:19:25, Alexander Duyck wrote:
>> This patchset is essentially a refactor of the page initialization logic
>> that is meant to provide for better code reuse while providing a
>> significant improvement in deferred page initialization performance.
>>
>> In my testing on an x86_64 system with 384GB of RAM and 3TB of persistent
>> memory per node I have seen the following. In the case of regular memory
>> initialization the deferred init time was decreased from 3.75s to 1.06s on
>> average. For the persistent memory the initialization time dropped from
>> 24.17s to 19.12s on average. This amounts to a 253% improvement for the
>> deferred memory initialization performance, and a 26% improvement in the
>> persistent memory initialization performance.
>>
>> I have called out the improvement observed with each patch.
>
> I have only glanced through the code (there is a lot of the code to look
> at here). And I do not like the code duplication and the way how you
> make the hotplug special. There shouldn't be any real reason for that
> IMHO (e.g. why do we init pfn-at-a-time in early init while we do
> pageblock-at-a-time for hotplug). I might be wrong here and the code
> reuse might be really hard to achieve though.
Actually it isn't so much that hotplug is special. The issue is more
that the non-hotplug case is special in that you have to perform a
number of extra checks for things that just aren't necessary for the
hotplug case.
If anything I would probably need a new iterator that would be able to
take into account all the checks for the non-hotplug case and then
provide ranges of PFNs to initialize.
> I am also not impressed by new iterators because this api is quite
> complex already. But this is mostly a detail.
Yeah, the iterators were mostly an attempt at hiding some of the
complexity. Being able to break a loop down to just an iterator provding
the start of the range and the number of elements to initialize is
pretty easy to visualize, or at least I thought so.
> Thing I do not like is that you keep microptimizing PageReserved part
> while there shouldn't be anything fundamental about it. We should just
> remove it rather than make the code more complex. I fell more and more
> guilty to add there actually.
I plan to remove it, but don't think I can get to it in this patch set.
I was planning to submit one more iteration of this patch set early next
week, and then start focusing more on the removal of the PageReserved
bit for hotplug. I figure it is probably going to be a full patch set
onto itself and as you pointed out at the start of this email there is
already enough code to review without adding that.
next prev parent reply other threads:[~2018-11-15 0:50 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-05 21:19 [mm PATCH v5 0/7] Deferred page init improvements Alexander Duyck
2018-11-05 21:19 ` [mm PATCH v5 1/7] mm: Use mm_zero_struct_page from SPARC on all 64b architectures Alexander Duyck
2018-11-05 21:19 ` [mm PATCH v5 2/7] mm: Drop meminit_pfn_in_nid as it is redundant Alexander Duyck
2018-11-05 21:19 ` [mm PATCH v5 3/7] mm: Implement new zone specific memblock iterator Alexander Duyck
2018-11-09 23:26 ` Pavel Tatashin
2018-11-09 23:58 ` Alexander Duyck
2018-11-10 0:11 ` Pavel Tatashin
2018-11-05 21:19 ` [mm PATCH v5 4/7] mm: Initialize MAX_ORDER_NR_PAGES at a time instead of doing larger sections Alexander Duyck
2018-11-10 1:02 ` Pavel Tatashin
2018-11-19 18:53 ` Alexander Duyck
2018-11-05 21:19 ` [mm PATCH v5 5/7] mm: Move hot-plug specific memory init into separate functions and optimize Alexander Duyck
2018-11-10 2:07 ` Pavel Tatashin
2018-11-05 21:19 ` [mm PATCH v5 6/7] mm: Add reserved flag setting to set_page_links Alexander Duyck
2018-11-10 2:11 ` Pavel Tatashin
2018-11-05 21:20 ` [mm PATCH v5 7/7] mm: Use common iterator for deferred_init_pages and deferred_free_pages Alexander Duyck
2018-11-10 4:13 ` Pavel Tatashin
2018-11-12 15:12 ` Alexander Duyck
2018-11-09 21:15 ` [mm PATCH v5 0/7] Deferred page init improvements Pavel Tatashin
2018-11-09 23:14 ` Alexander Duyck
2018-11-10 0:00 ` Pavel Tatashin
2018-11-10 0:46 ` Alexander Duyck
2018-11-10 1:16 ` Pavel Tatashin
2018-11-12 19:10 ` Alexander Duyck
2018-11-12 20:37 ` Pavel Tatashin
2018-11-12 16:25 ` Daniel Jordan
2018-11-14 15:07 ` Michal Hocko
2018-11-14 19:12 ` Pavel Tatashin
2018-11-14 21:35 ` Michal Hocko
2018-11-15 0:50 ` Alexander Duyck [this message]
2018-11-15 1:55 ` Mike Rapoport
2018-11-15 19:09 ` Mike Rapoport
2018-11-15 8:10 ` Michal Hocko
2018-11-15 16:02 ` Alexander Duyck
2018-11-15 16:40 ` 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=9e8218eb-80bf-fc02-ae56-42ccfddb572e@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=linux-nvdimm@lists.01.org \
--cc=mgorman@techsingularity.net \
--cc=mhocko@kernel.org \
--cc=mingo@kernel.org \
--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 \
/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;
as well as URLs for NNTP newsgroup(s).