From: Cannon Matthews <cannonmatthews@google.com>
To: akpm@linux-foundation.org
Cc: mhocko@kernel.org, mike.kravetz@oracle.com, linux-mm@kvack.org,
linux-kernel@vger.kernel.org,
Andres Lagar-Cavilla <andreslc@google.com>,
sqazi@google.com, Paul Turner <pjt@google.com>,
David Matlack <dmatlack@google.com>,
Peter Feiner <pfeiner@google.com>,
nullptr@google.com
Subject: Re: [PATCH] RFC: clear 1G pages with streaming stores on x86
Date: Tue, 24 Jul 2018 19:50:25 -0700 [thread overview]
Message-ID: <CAJfu=UerK+cmgRcVOW_pLw+ADsSSksE1C0dgbGbbgX3DE_KCCg@mail.gmail.com> (raw)
In-Reply-To: <20180724135350.91a90f4f8742ec59c42721c3@linux-foundation.org>
On Tue, Jul 24, 2018 at 1:53 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Tue, 24 Jul 2018 13:46:39 -0700 Cannon Matthews <cannonmatthews@google.com> wrote:
>
> > Reimplement clear_gigantic_page() to clear gigabytes pages using the
> > non-temporal streaming store instructions that bypass the cache
> > (movnti), since an entire 1GiB region will not fit in the cache anyway.
> >
> > ...
> >
> > Tested:
> > Time to `mlock()` a 512GiB region on broadwell CPU
> > AVG time (s) % imp. ms/page
> > clear_page_erms 133.584 - 261
> > clear_page_nt 34.154 74.43% 67
>
> A gigantic improvement!
>
> > --- a/arch/x86/include/asm/page_64.h
> > +++ b/arch/x86/include/asm/page_64.h
> > @@ -56,6 +56,9 @@ static inline void clear_page(void *page)
> >
> > void copy_page(void *to, void *from);
> >
> > +#define __HAVE_ARCH_CLEAR_GIGANTIC_PAGE
> > +void __clear_page_nt(void *page, u64 page_size);
>
> Nit: the modern way is
>
> #ifndef __clear_page_nt
> void __clear_page_nt(void *page, u64 page_size);
> #define __clear_page_nt __clear_page_nt
> #endif
>
> Not sure why, really. I guess it avoids adding two symbols and
> having to remember and maintain the relationship between them.
>
That makes sense, changed to this style. Thanks.
> > --- /dev/null
> > +++ b/arch/x86/lib/clear_gigantic_page.c
> > @@ -0,0 +1,30 @@
> > +#include <asm/page.h>
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/mm.h>
> > +#include <linux/sched.h>
> > +
> > +#if defined(CONFIG_TRANSPARENT_HUGEPAGE) || defined(CONFIG_HUGETLBFS)
> > +#define PAGES_BETWEEN_RESCHED 64
> > +void clear_gigantic_page(struct page *page,
> > + unsigned long addr,
> > + unsigned int pages_per_huge_page)
> > +{
> > + int i;
> > + void *dest = page_to_virt(page);
> > + int resched_count = 0;
> > +
> > + BUG_ON(pages_per_huge_page % PAGES_BETWEEN_RESCHED != 0);
> > + BUG_ON(!dest);
> > +
> > + might_sleep();
>
> cond_resched() already does might_sleep() - it doesn't seem needed here.
Ah gotcha, removed it. The original implementation called both, which
does seem redundant.
>
> > + for (i = 0; i < pages_per_huge_page; i += PAGES_BETWEEN_RESCHED) {
> > + __clear_page_nt(dest + (i * PAGE_SIZE),
> > + PAGES_BETWEEN_RESCHED * PAGE_SIZE);
> > + resched_count += cond_resched();
> > + }
> > + /* __clear_page_nt requrires and `sfence` barrier. */
> > + wmb();
> > + pr_debug("clear_gigantic_page: rescheduled %d times\n", resched_count);
> > +}
> > +#endif
>
next prev parent reply other threads:[~2018-07-25 2:50 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-24 20:46 [PATCH] RFC: clear 1G pages with streaming stores on x86 Cannon Matthews
2018-07-24 20:53 ` Andrew Morton
2018-07-25 2:50 ` Cannon Matthews [this message]
2018-07-24 21:09 ` Matthew Wilcox
2018-07-25 2:37 ` [PATCH v2] " Cannon Matthews
2018-07-25 5:02 ` Elliott, Robert (Persistent Memory)
2018-07-25 14:38 ` Matthew Wilcox
2018-07-25 17:30 ` Cannon Matthews
2018-07-25 18:23 ` Matthew Wilcox
2018-07-25 18:48 ` Cannon Matthews
2018-07-25 12:57 ` Michal Hocko
2018-07-25 17:55 ` Cannon Matthews
2018-07-26 13:19 ` Michal Hocko
2018-07-27 0:05 ` Huang, Ying
2018-07-30 16:29 ` Borislav Petkov
2018-07-31 0:28 ` Cannon Matthews
2018-07-31 0:45 ` Matthew Wilcox
2018-07-25 2:46 ` [PATCH] " Cannon Matthews
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='CAJfu=UerK+cmgRcVOW_pLw+ADsSSksE1C0dgbGbbgX3DE_KCCg@mail.gmail.com' \
--to=cannonmatthews@google.com \
--cc=akpm@linux-foundation.org \
--cc=andreslc@google.com \
--cc=dmatlack@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=mike.kravetz@oracle.com \
--cc=nullptr@google.com \
--cc=pfeiner@google.com \
--cc=pjt@google.com \
--cc=sqazi@google.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).