linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Michal Hocko <mhocko@kernel.org>
To: linux-mm@kvack.org
Cc: Zi Yan <zi.yan@cs.rutgers.edu>,
	Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
	"Kirill A. Shutemov" <kirill@shutemov.name>,
	Vlastimil Babka <vbabka@suse.cz>,
	Andrew Morton <akpm@linux-foundation.org>,
	Andrea Reale <ar@linux.vnet.ibm.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: [RFC PATCH 0/3] mm: unclutter THP migration
Date: Fri,  8 Dec 2017 17:15:56 +0100	[thread overview]
Message-ID: <20171208161559.27313-1-mhocko@kernel.org> (raw)
In-Reply-To: <20171207143401.GK20234@dhcp22.suse.cz>

On Thu 07-12-17 15:34:01, Michal Hocko wrote:
> On Thu 07-12-17 22:10:47, Zi Yan wrote:
[...]
> > I agree with you that we should try to migrate all tail pages if the THP
> > needs to be split. But this might not be compatible with "getting
> > migration results" in unmap_and_move(), since a caller of
> > migrate_pages() may want to know the status of each page in the
> > migration list via int **result in get_new_page() (e.g.
> > new_page_node()). The caller has no idea whether a THP in its migration
> > list will be split or not, thus, storing migration results might be
> > quite tricky if tail pages are added into the migration list.
> 
> Ouch. I wasn't aware of this "beauty". I will try to wrap my head around
> this code and think about what to do about it. Thanks for point me to
> it.

OK, so was staring at this yesterday and concluded that the current
implementation of do_move_page_to_node_array is unfixable to work with
split_thp_page_list in migrate_pages. So I've reimplemented it to not
use the quite ugly fixed sized batching. Instead I am using dynamic
batching based on the same node request. See the patch 1 for more
details about implementation. This will allow us to remove the quite
ugly 'int **reason' from the allocation callback as well. This is patch
2 and patch 3 is finally the thp migration code.

Diffstat is quite supportive for this cleanup.
 include/linux/migrate.h        |   7 +-
 include/linux/page-isolation.h |   3 +-
 mm/compaction.c                |   3 +-
 mm/huge_memory.c               |   6 +
 mm/internal.h                  |   1 +
 mm/memory_hotplug.c            |   5 +-
 mm/mempolicy.c                 |  40 +----
 mm/migrate.c                   | 350 ++++++++++++++++++-----------------------
 mm/page_isolation.c            |   3 +-
 9 files changed, 177 insertions(+), 241 deletions(-)

Does anybody see any issues with this approach?

On a side note:
There are some semantic issues I have encountered on the way but I am
not addressing them here. E.g. trying to move THP tail pages will result
in either success or EBUSY (the later one more likely once we isolate
head from the LRU list). Hugetlb reports EACCESS on tail pages.
Some errors are reported via status parameter but migration failures are
not even though the original `reason' argument suggests there was an
intention to do so. From a quick look into git history this never
worked. I have tried to keep the semantic unchanged.

Then there is a relatively minor thing that the page isolation might
fail because of pages not being on the LRU - e.g. because they are
sitting on the per-cpu LRU caches. Easily fixable.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2017-12-08 16:16 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-07 12:48 [RFC PATCH] mm: unclutter THP migration Michal Hocko
2017-12-07 14:10 ` Zi Yan
2017-12-07 14:34   ` Michal Hocko
2017-12-08 16:15     ` Michal Hocko [this message]
2017-12-08 16:15       ` [RFC PATCH 1/3] mm, numa: rework do_pages_move Michal Hocko
2017-12-13 12:07         ` Kirill A. Shutemov
2017-12-13 12:17           ` Michal Hocko
2017-12-13 12:47             ` Kirill A. Shutemov
2017-12-13 14:10               ` Michal Hocko
2017-12-13 14:27                 ` Kirill A. Shutemov
2017-12-13 14:39         ` Michal Hocko
2017-12-14 15:35           ` Kirill A. Shutemov
2017-12-15  9:28             ` Michal Hocko
2017-12-15  9:51               ` Kirill A. Shutemov
2017-12-15  9:57                 ` Michal Hocko
2018-01-02 11:25         ` Anshuman Khandual
2018-01-02 12:12           ` Michal Hocko
2018-01-03  3:11             ` Anshuman Khandual
2018-01-03  8:42         ` Anshuman Khandual
2018-01-03  8:58           ` Michal Hocko
2018-01-03  9:36             ` Anshuman Khandual
2018-01-03  9:52               ` Michal Hocko
2017-12-08 16:15       ` [RFC PATCH 2/3] mm, migrate: remove reason argument from new_page_t Michal Hocko
2017-12-27  2:12         ` Zi Yan
2017-12-29 11:32           ` Michal Hocko
2017-12-08 16:15       ` [RFC PATCH 3/3] mm: unclutter THP migration Michal Hocko
2017-12-13 12:20         ` Kirill A. Shutemov
2017-12-27  2:19         ` Zi Yan
2017-12-29 11:36           ` Michal Hocko
2017-12-29 15:45             ` Zi Yan
2017-12-31  9:07               ` Michal Hocko
2017-12-31 13:09                 ` Zi Yan
2017-12-19 12:07       ` [RFC PATCH 0/3] " 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=20171208161559.27313-1-mhocko@kernel.org \
    --to=mhocko@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=ar@linux.vnet.ibm.com \
    --cc=kirill@shutemov.name \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=n-horiguchi@ah.jp.nec.com \
    --cc=vbabka@suse.cz \
    --cc=zi.yan@cs.rutgers.edu \
    /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).