Hi,
I noticed that after the recent "page-flags" patchset, there are an excessive
number of calls to compound_head() in certain places.
For example, the frequently executed mark_page_accessed() function already
starts out by calling compound_head(), but then each time it tests a page flag
afterwards, there is an extra, seemingly unnecessary, call to compound_head().
This causes a series of instructions like the following to appear no fewer than
10 times throughout the function:
ffffffff81119db4: 48 8b 53 20 mov 0x20(%rbx),%rdx
ffffffff81119db8: 48 8d 42 ff lea -0x1(%rdx),%rax
ffffffff81119dbc: 83 e2 01 and $0x1,%edx
ffffffff81119dbf: 48 0f 44 c3 cmove %rbx,%rax
ffffffff81119dc3: 48 8b 00 mov (%rax),%rax
Part of the problem, I suppose, is that the compiler doesn't know that the pages
can't be linked more than one level deep.
Is this a known tradeoff, and have any possible solutions been considered?
Eric
On Sat, Mar 26, 2016 at 01:50:49PM -0500, Eric Biggers wrote:
> Hi,
>
> I noticed that after the recent "page-flags" patchset, there are an excessive
> number of calls to compound_head() in certain places.
>
> For example, the frequently executed mark_page_accessed() function already
> starts out by calling compound_head(), but then each time it tests a page flag
> afterwards, there is an extra, seemingly unnecessary, call to compound_head().
> This causes a series of instructions like the following to appear no fewer than
> 10 times throughout the function:
>
> ffffffff81119db4: 48 8b 53 20 mov 0x20(%rbx),%rdx
> ffffffff81119db8: 48 8d 42 ff lea -0x1(%rdx),%rax
> ffffffff81119dbc: 83 e2 01 and $0x1,%edx
> ffffffff81119dbf: 48 0f 44 c3 cmove %rbx,%rax
> ffffffff81119dc3: 48 8b 00 mov (%rax),%rax
>
> Part of the problem, I suppose, is that the compiler doesn't know that the pages
> can't be linked more than one level deep.
>
> Is this a known tradeoff, and have any possible solutions been considered?
<I'm sick, so my judgment may be off>
Yes, it's known problem. And I've tried to approach it few times without
satisfying results.
Your mail made me try again.
The idea is to introduce new type to indicate head page --
'struct head_page' -- it's compatible with struct page on memory layout,
but distinct from C point of view. compound_head() should return pointer
of that type. For the proof-of-concept I've introduced new helper --
compound_head_t().
Then we can make page-flag helpers to accept both types, by converting
them to macros and use __builtin_types_compatible_p().
When a page-flag helper sees pointer to 'struct head_page' as an argument,
it can safely assume that it deals with head or non-compound page and therefore
can bypass all policy restrictions and get rid of compound_head() calls.
I'll send proof-of-concept patches in reply to this message. The code is
not pretty. I myself consider the idea rather ugly.
Any comments are welcome.
--
Kirill A. Shutemov
Just for example, convert one function to just created interface.
This way we cut size of the function by third:
function old new delta
mark_page_accessed 310 203 -107
Not-yet-signed-off-by: Kirill A. Shutemov <[email protected]>
---
mm/swap.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/mm/swap.c b/mm/swap.c
index 09fe5e97714a..1fe072ae6ee1 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -360,9 +360,10 @@ static void __lru_cache_activate_page(struct page *page)
*/
void mark_page_accessed(struct page *page)
{
- page = compound_head(page);
- if (!PageActive(page) && !PageUnevictable(page) &&
- PageReferenced(page)) {
+ struct head_page *head = compound_head_t(page);
+ page = &head->page;
+ if (!PageActive(head) && !PageUnevictable(head) &&
+ PageReferenced(head)) {
/*
* If the page is on the LRU, queue it for activation via
@@ -370,15 +371,15 @@ void mark_page_accessed(struct page *page)
* pagevec, mark it active and it'll be moved to the active
* LRU on the next drain.
*/
- if (PageLRU(page))
+ if (PageLRU(head))
activate_page(page);
else
__lru_cache_activate_page(page);
- ClearPageReferenced(page);
+ ClearPageReferenced(head);
if (page_is_file_cache(page))
workingset_activation(page);
- } else if (!PageReferenced(page)) {
- SetPageReferenced(page);
+ } else if (!PageReferenced(head)) {
+ SetPageReferenced(head);
}
if (page_is_idle(page))
clear_page_idle(page);
--
2.8.0.rc3
scripts/mkpageflags.sh generates include/generated/page-flags.h using
directive in mm/page-flags.tlb.
Format of the mm/page-flags.tlb: lines not started with capital letter,
just copied. Lines which starts with capital letter are rules to
generate page-flags helper:
<uname> <lname> <policy> <required-helpers>
Result of generation is set of helper macros, not functions. It's
required for future extension.
Type safety is controlled with __builtin_types_compatible_p() check.
Not-yet-signed-off-by: Kirill A. Shutemov <[email protected]>
---
Kbuild | 30 +++++--
include/linux/page-flags.h | 217 +--------------------------------------------
mm/page-flags.tbl | 102 +++++++++++++++++++++
scripts/mkpageflags.sh | 151 +++++++++++++++++++++++++++++++
4 files changed, 279 insertions(+), 221 deletions(-)
create mode 100644 mm/page-flags.tbl
create mode 100755 scripts/mkpageflags.sh
diff --git a/Kbuild b/Kbuild
index f55cefd9bf29..405c7145af95 100644
--- a/Kbuild
+++ b/Kbuild
@@ -3,8 +3,9 @@
# This file takes care of the following:
# 1) Generate bounds.h
# 2) Generate timeconst.h
-# 3) Generate asm-offsets.h (may need bounds.h and timeconst.h)
-# 4) Check for missing system calls
+# 3) Generate page-flags.h
+# 4) Generate asm-offsets.h (may need bounds.h, timeconst.h and page-flags.h)
+# 5) Check for missing system calls
# Default sed regexp - multiline due to syntax constraints
define sed-y
@@ -66,7 +67,25 @@ $(obj)/$(timeconst-file): kernel/time/timeconst.bc FORCE
$(call filechk,gentimeconst)
#####
-# 3) Generate asm-offsets.h
+# 3) Generate page-flags.h
+
+pageflags-file := include/generated/page-flags.h
+
+targets += $(pageflags-file)
+
+quiet_cmd_genpageflags = GEN $@
+define cmd_genpageflags
+ $(srctree)/scripts/mkpageflags.sh <$< >$@
+endef
+define filechk_genpageflags
+ $(srctree)/scripts/mkpageflags.sh <$<
+endef
+
+$(obj)/$(pageflags-file): mm/page-flags.tbl FORCE
+ $(call filechk,genpageflags)
+
+#####
+# 4) Generate asm-offsets.h
#
offsets-file := include/generated/asm-offsets.h
@@ -76,7 +95,8 @@ targets += arch/$(SRCARCH)/kernel/asm-offsets.s
# We use internal kbuild rules to avoid the "is up to date" message from make
arch/$(SRCARCH)/kernel/asm-offsets.s: arch/$(SRCARCH)/kernel/asm-offsets.c \
- $(obj)/$(timeconst-file) $(obj)/$(bounds-file) FORCE
+ $(obj)/$(timeconst-file) $(obj)/$(bounds-file) \
+ $(obj)/$(pageflags-file) FORCE
$(Q)mkdir -p $(dir $@)
$(call if_changed_dep,cc_s_c)
@@ -84,7 +104,7 @@ $(obj)/$(offsets-file): arch/$(SRCARCH)/kernel/asm-offsets.s FORCE
$(call filechk,offsets,__ASM_OFFSETS_H__)
#####
-# 4) Check for missing system calls
+# 5) Check for missing system calls
#
always += missing-syscalls
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index f4ed4f1b0c77..d111caad2a22 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -154,202 +154,7 @@ static __always_inline int PageCompound(struct page *page)
return test_bit(PG_head, &page->flags) || PageTail(page);
}
-/*
- * Page flags policies wrt compound pages
- *
- * PF_ANY:
- * the page flag is relevant for small, head and tail pages.
- *
- * PF_HEAD:
- * for compound page all operations related to the page flag applied to
- * head page.
- *
- * PF_NO_TAIL:
- * modifications of the page flag must be done on small or head pages,
- * checks can be done on tail pages too.
- *
- * PF_NO_COMPOUND:
- * the page flag is not relevant for compound pages.
- */
-#define PF_ANY(page, enforce) page
-#define PF_HEAD(page, enforce) compound_head(page)
-#define PF_NO_TAIL(page, enforce) ({ \
- VM_BUG_ON_PGFLAGS(enforce && PageTail(page), page); \
- compound_head(page);})
-#define PF_NO_COMPOUND(page, enforce) ({ \
- VM_BUG_ON_PGFLAGS(enforce && PageCompound(page), page); \
- page;})
-
-/*
- * Macros to create function definitions for page flags
- */
-#define TESTPAGEFLAG(uname, lname, policy) \
-static __always_inline int Page##uname(struct page *page) \
- { return test_bit(PG_##lname, &policy(page, 0)->flags); }
-
-#define SETPAGEFLAG(uname, lname, policy) \
-static __always_inline void SetPage##uname(struct page *page) \
- { set_bit(PG_##lname, &policy(page, 1)->flags); }
-
-#define CLEARPAGEFLAG(uname, lname, policy) \
-static __always_inline void ClearPage##uname(struct page *page) \
- { clear_bit(PG_##lname, &policy(page, 1)->flags); }
-
-#define __SETPAGEFLAG(uname, lname, policy) \
-static __always_inline void __SetPage##uname(struct page *page) \
- { __set_bit(PG_##lname, &policy(page, 1)->flags); }
-
-#define __CLEARPAGEFLAG(uname, lname, policy) \
-static __always_inline void __ClearPage##uname(struct page *page) \
- { __clear_bit(PG_##lname, &policy(page, 1)->flags); }
-
-#define TESTSETFLAG(uname, lname, policy) \
-static __always_inline int TestSetPage##uname(struct page *page) \
- { return test_and_set_bit(PG_##lname, &policy(page, 1)->flags); }
-
-#define TESTCLEARFLAG(uname, lname, policy) \
-static __always_inline int TestClearPage##uname(struct page *page) \
- { return test_and_clear_bit(PG_##lname, &policy(page, 1)->flags); }
-
-#define PAGEFLAG(uname, lname, policy) \
- TESTPAGEFLAG(uname, lname, policy) \
- SETPAGEFLAG(uname, lname, policy) \
- CLEARPAGEFLAG(uname, lname, policy)
-
-#define __PAGEFLAG(uname, lname, policy) \
- TESTPAGEFLAG(uname, lname, policy) \
- __SETPAGEFLAG(uname, lname, policy) \
- __CLEARPAGEFLAG(uname, lname, policy)
-
-#define TESTSCFLAG(uname, lname, policy) \
- TESTSETFLAG(uname, lname, policy) \
- TESTCLEARFLAG(uname, lname, policy)
-
-#define TESTPAGEFLAG_FALSE(uname) \
-static inline int Page##uname(const struct page *page) { return 0; }
-
-#define SETPAGEFLAG_NOOP(uname) \
-static inline void SetPage##uname(struct page *page) { }
-
-#define CLEARPAGEFLAG_NOOP(uname) \
-static inline void ClearPage##uname(struct page *page) { }
-
-#define __CLEARPAGEFLAG_NOOP(uname) \
-static inline void __ClearPage##uname(struct page *page) { }
-
-#define TESTSETFLAG_FALSE(uname) \
-static inline int TestSetPage##uname(struct page *page) { return 0; }
-
-#define TESTCLEARFLAG_FALSE(uname) \
-static inline int TestClearPage##uname(struct page *page) { return 0; }
-
-#define PAGEFLAG_FALSE(uname) TESTPAGEFLAG_FALSE(uname) \
- SETPAGEFLAG_NOOP(uname) CLEARPAGEFLAG_NOOP(uname)
-
-#define TESTSCFLAG_FALSE(uname) \
- TESTSETFLAG_FALSE(uname) TESTCLEARFLAG_FALSE(uname)
-
-__PAGEFLAG(Locked, locked, PF_NO_TAIL)
-PAGEFLAG(Error, error, PF_NO_COMPOUND) TESTCLEARFLAG(Error, error, PF_NO_COMPOUND)
-PAGEFLAG(Referenced, referenced, PF_HEAD)
- TESTCLEARFLAG(Referenced, referenced, PF_HEAD)
- __SETPAGEFLAG(Referenced, referenced, PF_HEAD)
-PAGEFLAG(Dirty, dirty, PF_HEAD) TESTSCFLAG(Dirty, dirty, PF_HEAD)
- __CLEARPAGEFLAG(Dirty, dirty, PF_HEAD)
-PAGEFLAG(LRU, lru, PF_HEAD) __CLEARPAGEFLAG(LRU, lru, PF_HEAD)
-PAGEFLAG(Active, active, PF_HEAD) __CLEARPAGEFLAG(Active, active, PF_HEAD)
- TESTCLEARFLAG(Active, active, PF_HEAD)
-__PAGEFLAG(Slab, slab, PF_NO_TAIL)
-__PAGEFLAG(SlobFree, slob_free, PF_NO_TAIL)
-PAGEFLAG(Checked, checked, PF_NO_COMPOUND) /* Used by some filesystems */
-
-/* Xen */
-PAGEFLAG(Pinned, pinned, PF_NO_COMPOUND)
- TESTSCFLAG(Pinned, pinned, PF_NO_COMPOUND)
-PAGEFLAG(SavePinned, savepinned, PF_NO_COMPOUND);
-PAGEFLAG(Foreign, foreign, PF_NO_COMPOUND);
-
-PAGEFLAG(Reserved, reserved, PF_NO_COMPOUND)
- __CLEARPAGEFLAG(Reserved, reserved, PF_NO_COMPOUND)
-PAGEFLAG(SwapBacked, swapbacked, PF_NO_TAIL)
- __CLEARPAGEFLAG(SwapBacked, swapbacked, PF_NO_TAIL)
- __SETPAGEFLAG(SwapBacked, swapbacked, PF_NO_TAIL)
-
-/*
- * Private page markings that may be used by the filesystem that owns the page
- * for its own purposes.
- * - PG_private and PG_private_2 cause releasepage() and co to be invoked
- */
-PAGEFLAG(Private, private, PF_ANY) __SETPAGEFLAG(Private, private, PF_ANY)
- __CLEARPAGEFLAG(Private, private, PF_ANY)
-PAGEFLAG(Private2, private_2, PF_ANY) TESTSCFLAG(Private2, private_2, PF_ANY)
-PAGEFLAG(OwnerPriv1, owner_priv_1, PF_ANY)
- TESTCLEARFLAG(OwnerPriv1, owner_priv_1, PF_ANY)
-
-/*
- * Only test-and-set exist for PG_writeback. The unconditional operators are
- * risky: they bypass page accounting.
- */
-TESTPAGEFLAG(Writeback, writeback, PF_NO_COMPOUND)
- TESTSCFLAG(Writeback, writeback, PF_NO_COMPOUND)
-PAGEFLAG(MappedToDisk, mappedtodisk, PF_NO_COMPOUND)
-
-/* PG_readahead is only used for reads; PG_reclaim is only for writes */
-PAGEFLAG(Reclaim, reclaim, PF_NO_COMPOUND)
- TESTCLEARFLAG(Reclaim, reclaim, PF_NO_COMPOUND)
-PAGEFLAG(Readahead, reclaim, PF_NO_COMPOUND)
- TESTCLEARFLAG(Readahead, reclaim, PF_NO_COMPOUND)
-
-#ifdef CONFIG_HIGHMEM
-/*
- * Must use a macro here due to header dependency issues. page_zone() is not
- * available at this point.
- */
-#define PageHighMem(__p) is_highmem_idx(page_zonenum(__p))
-#else
-PAGEFLAG_FALSE(HighMem)
-#endif
-
-#ifdef CONFIG_SWAP
-PAGEFLAG(SwapCache, swapcache, PF_NO_COMPOUND)
-#else
-PAGEFLAG_FALSE(SwapCache)
-#endif
-
-PAGEFLAG(Unevictable, unevictable, PF_HEAD)
- __CLEARPAGEFLAG(Unevictable, unevictable, PF_HEAD)
- TESTCLEARFLAG(Unevictable, unevictable, PF_HEAD)
-
-#ifdef CONFIG_MMU
-PAGEFLAG(Mlocked, mlocked, PF_NO_TAIL)
- __CLEARPAGEFLAG(Mlocked, mlocked, PF_NO_TAIL)
- TESTSCFLAG(Mlocked, mlocked, PF_NO_TAIL)
-#else
-PAGEFLAG_FALSE(Mlocked) __CLEARPAGEFLAG_NOOP(Mlocked)
- TESTSCFLAG_FALSE(Mlocked)
-#endif
-
-#ifdef CONFIG_ARCH_USES_PG_UNCACHED
-PAGEFLAG(Uncached, uncached, PF_NO_COMPOUND)
-#else
-PAGEFLAG_FALSE(Uncached)
-#endif
-
-#ifdef CONFIG_MEMORY_FAILURE
-PAGEFLAG(HWPoison, hwpoison, PF_ANY)
-TESTSCFLAG(HWPoison, hwpoison, PF_ANY)
-#define __PG_HWPOISON (1UL << PG_hwpoison)
-#else
-PAGEFLAG_FALSE(HWPoison)
-#define __PG_HWPOISON 0
-#endif
-
-#if defined(CONFIG_IDLE_PAGE_TRACKING) && defined(CONFIG_64BIT)
-TESTPAGEFLAG(Young, young, PF_ANY)
-SETPAGEFLAG(Young, young, PF_ANY)
-TESTCLEARFLAG(Young, young, PF_ANY)
-PAGEFLAG(Idle, idle, PF_ANY)
-#endif
+#include <generated/page-flags.h>
/*
* On an anonymous page mapped into a user virtual memory area,
@@ -390,8 +195,6 @@ static __always_inline int PageKsm(struct page *page)
return ((unsigned long)page->mapping & PAGE_MAPPING_FLAGS) ==
(PAGE_MAPPING_ANON | PAGE_MAPPING_KSM);
}
-#else
-TESTPAGEFLAG_FALSE(Ksm)
#endif
u64 stable_page_flags(struct page *page);
@@ -434,8 +237,6 @@ static __always_inline void SetPageUptodate(struct page *page)
set_bit(PG_uptodate, &page->flags);
}
-CLEARPAGEFLAG(Uptodate, uptodate, PF_NO_TAIL)
-
int test_clear_page_writeback(struct page *page);
int __test_set_page_writeback(struct page *page, bool keep_write);
@@ -454,8 +255,6 @@ static inline void set_page_writeback_keepwrite(struct page *page)
test_set_page_writeback_keepwrite(page);
}
-__PAGEFLAG(Head, head, PF_ANY) CLEARPAGEFLAG(Head, head, PF_ANY)
-
static __always_inline void set_compound_head(struct page *page, struct page *head)
{
WRITE_ONCE(page->compound_head, (unsigned long)head + 1);
@@ -481,8 +280,6 @@ int PageHuge(struct page *page);
int PageHeadHuge(struct page *page);
bool page_huge_active(struct page *page);
#else
-TESTPAGEFLAG_FALSE(Huge)
-TESTPAGEFLAG_FALSE(HeadHuge)
static inline bool page_huge_active(struct page *page)
{
@@ -555,14 +352,6 @@ static inline int TestClearPageDoubleMap(struct page *page)
VM_BUG_ON_PAGE(!PageHead(page), page);
return test_and_clear_bit(PG_double_map, &page[1].flags);
}
-
-#else
-TESTPAGEFLAG_FALSE(TransHuge)
-TESTPAGEFLAG_FALSE(TransCompound)
-TESTPAGEFLAG_FALSE(TransTail)
-TESTPAGEFLAG_FALSE(DoubleMap)
- TESTSETFLAG_FALSE(DoubleMap)
- TESTCLEARFLAG_FALSE(DoubleMap)
#endif
/*
@@ -684,10 +473,6 @@ static inline int page_has_private(struct page *page)
return !!(page->flags & PAGE_FLAGS_PRIVATE);
}
-#undef PF_ANY
-#undef PF_HEAD
-#undef PF_NO_TAIL
-#undef PF_NO_COMPOUND
#endif /* !__GENERATING_BOUNDS_H */
#endif /* PAGE_FLAGS_H */
diff --git a/mm/page-flags.tbl b/mm/page-flags.tbl
new file mode 100644
index 000000000000..1eb2f90f487a
--- /dev/null
+++ b/mm/page-flags.tbl
@@ -0,0 +1,102 @@
+Locked locked no_tail test __set __clear
+Error error no_compound test set clear testclear
+Referenced referenced head test set clear testclear __set
+Dirty dirty head test set clear testset testclear __clear
+LRU lru head test set clear __clear
+Active active head test set clear testclear __clear
+Slab slab no_tail test __set __clear
+SlobFree slob_free no_tail test __set __clear
+
+/* Used by some filesystems */
+Checked checked no_compound test set clear
+
+/* Xen */
+Pinned pinned no_compound test set clear testset testclear
+SavePinned savepinned no_compound test set clear
+Foreign foreign no_compound test set clear
+
+Reserved reserved no_compound test set clear __clear
+SwapBacked swapbacked no_tail test set clear __set __clear
+
+/*
+ * Private page markings that may be used by the filesystem that owns the page
+ * for its own purposes.
+ * - PG_private and PG_private_2 cause releasepage() and co to be invoked
+ */
+
+Private private any test set clear __set __clear
+Private2 private_2 any test set clear testset testclear
+
+/*
+ * Only test-and-set exist for PG_writeback. The unconditional operators are
+ * risky: they bypass page accounting.
+ */
+Writeback writeback no_compound test testset testclear
+MappedToDisk mappedtodisk no_compound test set clear
+
+/* PG_readahead is only used for reads; PG_reclaim is only for writes */
+Reclaim reclaim no_compound test set clear testclear
+Readahead reclaim no_compound test set clear testclear
+
+#ifdef CONFIG_HIGHMEM
+/*
+ * Must use a macro here due to header dependency issues. page_zone() is not
+ * available at this point.
+ */
+#define PageHighMem(__p) is_highmem_idx(page_zonenum(__p))
+#else
+HighMem null any testfalse setnoop clearnoop
+#endif
+
+#ifdef CONFIG_SWAP
+SwapCache swapcache no_compound test set clear
+#else
+SwapCache null any testfalse setnoop clearnoop
+#endif
+
+Unevictable unevictable head test set clear testclear __clear
+
+#ifdef CONFIG_MMU
+Mlocked mlocked no_tail test set clear testset testclear __clear
+#else
+Mlocked null any testfalse setnoop clearnoop testsetfalse testclearfalse __clearnoop
+#endif
+
+#ifdef CONFIG_ARCH_USES_PG_UNCACHED
+Uncached uncached no_compound test set clear
+#else
+Uncached uncached any testfalse setnoop clearnoop
+#endif
+
+#ifdef CONFIG_MEMORY_FAILURE
+HWPoison hwpoison any test set clear testset testclear
+#define __PG_HWPOISON (1UL << PG_hwpoison)
+#else
+HWPoison hwpoison any testfalse setnoop clearnoop
+#define __PG_HWPOISON 0
+#endif
+
+#if defined(CONFIG_IDLE_PAGE_TRACKING) && defined(CONFIG_64BIT)
+Young young any test set testclear
+Idle idle any test set clear
+#endif
+
+#ifndef CONFIG_KSM
+Ksm null any testfalse
+#endif
+
+Uptodate uptodate no_tail clear
+
+Head head any test clear __set __clear
+
+#ifndef CONFIG_HUGETLB_PAGE
+Huge null any testfalse
+HeadHuge null any testfalse
+#endif
+
+#ifndef CONFIG_TRANSPARENT_HUGEPAGE
+TransHuge null any testfalse
+TransCompound null any testfalse
+TransTail null any testfalse
+DoubleMap null any testfalse testsetfalse testclearfalse
+#endif
diff --git a/scripts/mkpageflags.sh b/scripts/mkpageflags.sh
new file mode 100755
index 000000000000..29d46bccaea4
--- /dev/null
+++ b/scripts/mkpageflags.sh
@@ -0,0 +1,151 @@
+#!/bin/sh -efu
+
+fatal() {
+ echo "$@" >&2
+ exit 1
+}
+
+any() {
+ echo "(__p)"
+}
+
+head() {
+ echo "compound_head(__p)"
+}
+
+no_tail() {
+ local enforce="${1:+VM_BUG_ON_PGFLAGS(PageTail(__p), __p);}"
+
+ echo "({$enforce compound_head(__p);})"
+}
+
+no_compound() {
+ local enforce="${1:+VM_BUG_ON_PGFLAGS(PageCompound(__p), __p);}"
+
+ echo "({$enforce __p;})"
+}
+
+generate_test() {
+ local op="$1"; shift
+ local uname="$1"; shift
+ local lname="$1"; shift
+ local page="$1"; shift
+
+ cat <<EOF
+#define $uname(__p) ({ \\
+ int ret; \\
+ if (__builtin_types_compatible_p(typeof(*(__p)), struct page)) \\
+ ret = $op(PG_$lname, &$page->flags); \\
+ else \\
+ BUILD_BUG(); \\
+ ret; \\
+})
+
+EOF
+}
+
+generate_mod() {
+ local op="$1"; shift
+ local uname="$1"; shift
+ local lname="$1"; shift
+ local page="$1"; shift
+
+ cat <<EOF
+#define $uname(__p) do { \\
+ if (__builtin_types_compatible_p(typeof(*(__p)), struct page)) \\
+ $op(PG_$lname, &$page->flags); \\
+ else \\
+ BUILD_BUG(); \\
+} while (0)
+
+EOF
+}
+
+generate_false() {
+ local uname="$1"; shift
+
+ cat <<EOF
+#define $uname(__p) 0
+
+EOF
+}
+
+generate_noop() {
+ local uname="$1"; shift
+
+ cat <<EOF
+#define $uname(__p) do { } while (0)
+
+EOF
+}
+
+generate_helper() {
+ local helper="$1"; shift
+ local uname="$1"; shift
+ local lname="$1"; shift
+ local policy="$1"; shift
+
+ case "$helper" in
+ test)
+ generate_test 'test_bit' "Page$uname" "$lname" "$($policy)"
+ ;;
+ set)
+ generate_mod 'set_bit' "SetPage$uname" "$lname" "$($policy)"
+ ;;
+ clear)
+ generate_mod 'clear_bit' "ClearPage$uname" "$lname" "$($policy)"
+ ;;
+ testset)
+ generate_test 'test_and_set_bit' "TestSetPage$uname" "$lname" "$($policy 1)"
+ ;;
+ testclear)
+ generate_test 'test_and_clear_bit' "TestClearPage$uname" "$lname" "$($policy 1)"
+ ;;
+ __set)
+ generate_mod '__set_bit' "__SetPage$uname" "$lname" "$($policy)"
+ ;;
+ __clear)
+ generate_mod '__clear_bit' "__ClearPage$uname" "$lname" "$($policy)"
+ ;;
+ testfalse)
+ generate_false "Page$uname"
+ ;;
+ setnoop)
+ generate_noop "SetPage$uname"
+ ;;
+ clearnoop)
+ generate_noop "ClearPage$uname"
+ ;;
+ testsetfalse)
+ generate_false "TestSetPage$uname"
+ ;;
+ testclearfalse)
+ generate_false "TestClearPage$uname"
+ ;;
+ __clearnoop)
+ generate_noop "__ClearPage$uname"
+ ;;
+ *)
+ fatal "$helper: unknown helper"
+ ;;
+ esac
+}
+
+generate_helpers() {
+ while read uname lname policy helpers; do
+ for helper in $helpers; do
+ generate_helper "$helper" "$uname" "$lname" "$policy"
+ done
+ done
+}
+
+while read l; do
+ case "$l" in
+ [A-Z]*)
+ echo "$l" | generate_helpers
+ ;;
+ *)
+ echo "$l"
+ ;;
+ esac
+done
--
2.8.0.rc3
This patch makes all generated page flag helpers to accept pointer to
struct head_page as well as struct page.
In case if pointer to struct head_page is passed, we assume that it's
head page and bypass policy constrain checks.
Note, to get get inteface consistent we would need to make non-generated
page flag helper to accept struct head_page as well.
Not-yet-signed-off-by: Kirill A. Shutemov <[email protected]>
---
scripts/mkpageflags.sh | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/scripts/mkpageflags.sh b/scripts/mkpageflags.sh
index 29d46bccaea4..272aab4ad1a3 100755
--- a/scripts/mkpageflags.sh
+++ b/scripts/mkpageflags.sh
@@ -6,23 +6,23 @@ fatal() {
}
any() {
- echo "(__p)"
+ echo "((struct page *)(__p))"
}
head() {
- echo "compound_head(__p)"
+ echo "compound_head((struct page *)(__p))"
}
no_tail() {
local enforce="${1:+VM_BUG_ON_PGFLAGS(PageTail(__p), __p);}"
- echo "({$enforce compound_head(__p);})"
+ echo "({$enforce compound_head((struct page *)(__p));})"
}
no_compound() {
local enforce="${1:+VM_BUG_ON_PGFLAGS(PageCompound(__p), __p);}"
- echo "({$enforce __p;})"
+ echo "({$enforce ((struct page *)(__p));})"
}
generate_test() {
@@ -34,7 +34,9 @@ generate_test() {
cat <<EOF
#define $uname(__p) ({ \\
int ret; \\
- if (__builtin_types_compatible_p(typeof(*(__p)), struct page)) \\
+ if (__builtin_types_compatible_p(typeof(*(__p)), struct head_page)) \\
+ ret = $op(PG_$lname, &((struct head_page *)(__p))->page.flags); \\
+ else if (__builtin_types_compatible_p(typeof(*(__p)), struct page)) \\
ret = $op(PG_$lname, &$page->flags); \\
else \\
BUILD_BUG(); \\
@@ -52,7 +54,9 @@ generate_mod() {
cat <<EOF
#define $uname(__p) do { \\
- if (__builtin_types_compatible_p(typeof(*(__p)), struct page)) \\
+ if (__builtin_types_compatible_p(typeof(*(__p)), struct head_page)) \\
+ $op(PG_$lname, &((struct head_page *)(__p))->page.flags); \\
+ else if (__builtin_types_compatible_p(typeof(*(__p)), struct page)) \\
$op(PG_$lname, &$page->flags); \\
else \\
BUILD_BUG(); \\
--
2.8.0.rc3
This patch creates new type that is compatible with struct page on
memory layout, but distinct from C point of view.
compound_head_t() has the same functionality as compound_head(), but
returns pointer on struct head_page.
Not-yet-signed-off-by: Kirill A. Shutemov <[email protected]>
---
include/linux/mm_types.h | 4 ++++
include/linux/page-flags.h | 9 ++++++++-
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 944b2b37313b..247e86adaa1c 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -225,6 +225,10 @@ struct page {
#endif
;
+struct head_page {
+ struct page page;
+};
+
struct page_frag {
struct page *page;
#if (BITS_PER_LONG > 32) || (PAGE_SIZE >= 65536)
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index d111caad2a22..54801253b85c 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -133,7 +133,9 @@ enum pageflags {
#ifndef __GENERATING_BOUNDS_H
-struct page; /* forward declaration */
+/* forward declaration */
+struct page;
+struct head_page;
static inline struct page *compound_head(struct page *page)
{
@@ -144,6 +146,11 @@ static inline struct page *compound_head(struct page *page)
return page;
}
+static inline struct head_page *compound_head_t(struct page *page)
+{
+ return (struct head_page *)compound_head(page);
+}
+
static __always_inline int PageTail(struct page *page)
{
return READ_ONCE(page->compound_head) & 1;
--
2.8.0.rc3
Could you just mark compound_head __pure? That would tell the compiler
that it's safe to re-use the return value as long as there is no memory
mutation in between.
On Sun, Mar 27, 2016 at 04:33:04PM -0400, George Spelvin wrote:
> Could you just mark compound_head __pure? That would tell the compiler
> that it's safe to re-use the return value as long as there is no memory
> mutation in between.
>
Hm. It has some positive impact, but it's not dramatic. For instance,
mm/swap.o results:
add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-43 (-43)
function old new delta
__page_cache_release 319 298 -21
release_pages 722 700 -22
mark_page_accessed() problem was not fixed by that.
--
Kirill A. Shutemov
On Sun, Mar 27, 2016 at 10:46:49PM +0300, Kirill A. Shutemov wrote:
> The idea is to introduce new type to indicate head page --
> 'struct head_page' -- it's compatible with struct page on memory layout,
> but distinct from C point of view. compound_head() should return pointer
> of that type. For the proof-of-concept I've introduced new helper --
> compound_head_t().
>
Well, it's good for optimizing the specific case of mark_page_accessed(). I'm
more worried about the general level of bloat, since the Page* macros are used
in so many places. And generating page-flags.h with a script is something to be
avoided if at all possible.
I wasn't following the discussion around the original page-flags patchset. Can
you point me to a discussion of the benefits of the page "policy" checks --- why
are they suddenly needed when they weren't before? Or any helpful comments in
the code?
On Thu, Mar 31, 2016 at 08:33:29PM -0500, Eric Biggers wrote:
> On Sun, Mar 27, 2016 at 10:46:49PM +0300, Kirill A. Shutemov wrote:
> > The idea is to introduce new type to indicate head page --
> > 'struct head_page' -- it's compatible with struct page on memory layout,
> > but distinct from C point of view. compound_head() should return pointer
> > of that type. For the proof-of-concept I've introduced new helper --
> > compound_head_t().
> >
>
> Well, it's good for optimizing the specific case of mark_page_accessed(). I'm
> more worried about the general level of bloat, since the Page* macros are used
> in so many places. And generating page-flags.h with a script is something to be
> avoided if at all possible.
I think it can be done without generating page-flags.h. We can generate
with preprocessor Head* helpers in addition to Page*. New heplers would
opperate with struct head_page rather than struct page.
> I wasn't following the discussion around the original page-flags patchset. Can
> you point me to a discussion of the benefits of the page "policy" checks --- why
> are they suddenly needed when they weren't before? Or any helpful comments in
> the code?
Recent THP refcounting rework (went into v4.5) made possible mapping part
of huge page with PTEs. Basically, we now have page table entries which
point to tail pages. It means we have tail pages in codepaths where we
haven't before and we need to deal with this.
Many of the flags apply to whole compound page, not a subpage. So we need
to redirect page flag operation to head page.
--
Kirill A. Shutemov