From: Michal Hocko <mhocko@kernel.org>
To: Mike Kravetz <mike.kravetz@oracle.com>
Cc: linux-mm@kvack.org, Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH RFC 2/2] mm, hugetlb: do not rely on overcommit limit during migration
Date: Thu, 30 Nov 2017 08:57:42 +0100 [thread overview]
Message-ID: <20171130075742.3exagxg6y4j427ut@dhcp22.suse.cz> (raw)
In-Reply-To: <425a8947-d32a-d6bb-3a0a-2e30275c64c9@oracle.com>
On Wed 29-11-17 11:52:53, Mike Kravetz wrote:
> On 11/29/2017 01:22 AM, Michal Hocko wrote:
> > What about this on top. I haven't tested this yet though.
>
> Yes, this would work.
>
> However, I think a simple modification to your previous free_huge_page
> changes would make this unnecessary. I was confused in your previous
> patch because you decremented the per-node surplus page count, but not
> the global count. I think it would have been correct (and made this
> patch unnecessary) if you decremented the global counter there as well.
We cannot really increment the global counter because the over number of
surplus pages during migration doesn't increase.
> Of course, this patch makes the surplus accounting more explicit.
>
> If we move forward with this patch, one issue below.
>
> > ---
> > diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> > index 1b6d7783c717..f5fcd4e355dc 100644
> > --- a/include/linux/hugetlb.h
> > +++ b/include/linux/hugetlb.h
> > @@ -119,6 +119,7 @@ long hugetlb_unreserve_pages(struct inode *inode, long start, long end,
> > long freed);
> > bool isolate_huge_page(struct page *page, struct list_head *list);
> > void putback_active_hugepage(struct page *page);
> > +void move_hugetlb_state(struct page *oldpage, struct page *newpage, int reason);
> > void free_huge_page(struct page *page);
> > void hugetlb_fix_reserve_counts(struct inode *inode);
> > extern struct mutex *hugetlb_fault_mutex_table;
> > @@ -232,6 +233,7 @@ static inline bool isolate_huge_page(struct page *page, struct list_head *list)
> > return false;
> > }
> > #define putback_active_hugepage(p) do {} while (0)
> > +#define move_hugetlb_state(old, new, reason) do {} while (0)
> >
> > static inline unsigned long hugetlb_change_protection(struct vm_area_struct *vma,
> > unsigned long address, unsigned long end, pgprot_t newprot)
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index 037bf0f89463..30601c1c62f3 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -34,6 +34,7 @@
> > #include <linux/hugetlb_cgroup.h>
> > #include <linux/node.h>
> > #include <linux/userfaultfd_k.h>
> > +#include <linux/page_owner.h>
> > #include "internal.h"
> >
> > int hugetlb_max_hstate __read_mostly;
> > @@ -4830,3 +4831,34 @@ void putback_active_hugepage(struct page *page)
> > spin_unlock(&hugetlb_lock);
> > put_page(page);
> > }
> > +
> > +void move_hugetlb_state(struct page *oldpage, struct page *newpage, int reason)
> > +{
> > + struct hstate *h = page_hstate(oldpage);
> > +
> > + hugetlb_cgroup_migrate(oldpage, newpage);
> > + set_page_owner_migrate_reason(newpage, reason);
> > +
> > + /*
> > + * transfer temporary state of the new huge page. This is
> > + * reverse to other transitions because the newpage is going to
> > + * be final while the old one will be freed so it takes over
> > + * the temporary status.
> > + *
> > + * Also note that we have to transfer the per-node surplus state
> > + * here as well otherwise the global surplus count will not match
> > + * the per-node's.
> > + */
> > + if (PageHugeTemporary(newpage)) {
> > + int old_nid = page_to_nid(oldpage);
> > + int new_nid = page_to_nid(newpage);
> > +
> > + SetPageHugeTemporary(oldpage);
> > + ClearPageHugeTemporary(newpage);
> > +
> > + if (h->surplus_huge_pages_node[old_nid]) {
> > + h->surplus_huge_pages_node[old_nid]--;
> > + h->surplus_huge_pages_node[new_nid]++;
> > + }
>
> You need to take hugetlb_lock before adjusting the surplus counts.
You are right. Actually moving the code to hugetlb.c was exactly because
I didn't want to take the lock outside of the hugetlb proper. I just
forgot to add it here. Thanks for spotting.
--
Michal Hocko
SUSE Labs
next prev parent reply other threads:[~2017-11-30 7:57 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-22 15:28 hugetlb page migration vs. overcommit Michal Hocko
2017-11-22 19:11 ` Mike Kravetz
2017-11-23 9:21 ` Michal Hocko
2017-11-27 6:27 ` Naoya Horiguchi
2017-11-28 10:19 ` Michal Hocko
2017-11-28 14:12 ` Michal Hocko
2017-11-28 14:12 ` [PATCH RFC 1/2] mm, hugetlb: unify core page allocation accounting and initialization Michal Hocko
2017-11-28 21:34 ` Mike Kravetz
2017-11-29 6:57 ` Michal Hocko
2017-11-29 19:09 ` Mike Kravetz
2017-11-28 14:12 ` [PATCH RFC 2/2] mm, hugetlb: do not rely on overcommit limit during migration Michal Hocko
2017-11-29 1:39 ` Mike Kravetz
2017-11-29 7:17 ` Michal Hocko
2017-11-29 9:22 ` Michal Hocko
2017-11-29 9:40 ` Michal Hocko
2017-11-29 11:23 ` Michal Hocko
2017-11-29 19:52 ` Mike Kravetz
2017-11-30 7:57 ` Michal Hocko [this message]
2017-11-30 19:35 ` Mike Kravetz
2017-11-30 19:57 ` Michal Hocko
2017-11-30 20:06 ` Michal Hocko
2017-11-29 9:51 ` Michal Hocko
2017-11-29 11:33 ` [PATCH RFC v2 " 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=20171130075742.3exagxg6y4j427ut@dhcp22.suse.cz \
--to=mhocko@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mike.kravetz@oracle.com \
--cc=n-horiguchi@ah.jp.nec.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).