Bloat caused by unnecessary calls to compound_head()?

Bloat caused by unnecessary calls to compound_head()?

[Eric Biggers]

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

--
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>

Re: Bloat caused by unnecessary calls to compound_head()?

[Kirill A. Shutemov]

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

--
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>

[PATCH 1/4] page-flags: generate page-flags helpers with script

[Kirill A. Shutemov]

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 <kirill.shutemov@linux.intel.com>
---
 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

--
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>

[PATCH 2/4] mm: introduce struct head_page and compound_head_t

[Kirill A. Shutemov]

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 <kirill.shutemov@linux.intel.com>
---
 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

--
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>

[PATCH 3/4] page-flags: make page flag helpers accept struct head_page

[Kirill A. Shutemov]

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 <kirill.shutemov@linux.intel.com>
---
 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

--
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>

[PATCH 4/4] mm: convert make_page_accessed to use compount_page_t()

[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 <kirill.shutemov@linux.intel.com>
---
 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

--
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>

Re: Bloat caused by unnecessary calls to compound_head()?

[Eric Biggers]

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?

--
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>

Re: Bloat caused by unnecessary calls to compound_head()?

[Kirill A. Shutemov]

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

--
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>