Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1334114 > unrolled thread

[PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation js1304@gmail.com - 2016-02-15 04:10 +0100
    Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page  reference manipulation Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-02-15 06:10 +0100
      Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page  reference manipulation Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-02-15 06:30 +0100
        Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page  reference manipulation Joonsoo Kim <js1304@gmail.com> - 2016-02-15 15:20 +0100
    Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page  reference manipulation Steven Rostedt <rostedt@goodmis.org> - 2016-02-15 17:10 +0100
      Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page  reference manipulation Joonsoo Kim <iamjoonsoo.kim@lge.com> - 2016-02-16 01:50 +0100
        Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page  reference manipulation Steven Rostedt <rostedt@goodmis.org> - 2016-02-16 02:20 +0100
          Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page  reference manipulation Joonsoo Kim <js1304@gmail.com> - 2016-02-18 08:50 +0100
            Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page  reference manipulation Steven Rostedt <rostedt@goodmis.org> - 2016-02-18 15:30 +0100
    Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page  reference manipulation Steven Rostedt <rostedt@goodmis.org> - 2016-02-18 15:30 +0100
      Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page  reference manipulation Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-02-19 01:40 +0100
        Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page  reference manipulation Joonsoo Kim <js1304@gmail.com> - 2016-02-19 02:40 +0100
          Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page  reference manipulation Steven Rostedt <rostedt@goodmis.org> - 2016-02-19 02:50 +0100
          Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page  reference manipulation Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2016-02-19 03:20 +0100
      Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page  reference manipulation Joonsoo Kim <js1304@gmail.com> - 2016-02-19 02:30 +0100

#1334114 — [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation

From: Joonsoo Kim <iamjoonsoo.kim@lge.com>

CMA allocation should be guaranteed to succeed by definition, but,
unfortunately, it would be failed sometimes. It is hard to track down
the problem, because it is related to page reference manipulation and
we don't have any facility to analyze it.

This patch adds tracepoints to track down page reference manipulation.
With it, we can find exact reason of failure and can fix the problem.
Following is an example of tracepoint output.

<...>-9018  [004]    92.678375: page_ref_set:         pfn=0x17ac9 flags=0x0 count=1 mapcount=0 mapping=(nil) mt=4 val=1
<...>-9018  [004]    92.678378: kernel_stack:
 => get_page_from_freelist (ffffffff81176659)
 => __alloc_pages_nodemask (ffffffff81176d22)
 => alloc_pages_vma (ffffffff811bf675)
 => handle_mm_fault (ffffffff8119e693)
 => __do_page_fault (ffffffff810631ea)
 => trace_do_page_fault (ffffffff81063543)
 => do_async_page_fault (ffffffff8105c40a)
 => async_page_fault (ffffffff817581d8)
[snip]
<...>-9018  [004]    92.678379: page_ref_mod:         pfn=0x17ac9 flags=0x40048 count=2 mapcount=1 mapping=0xffff880015a78dc1 mt=4 val=1
[snip]
...
...
<...>-9131  [001]    93.174468: test_pages_isolated:  start_pfn=0x17800 end_pfn=0x17c00 fin_pfn=0x17ac9 ret=fail
[snip]
<...>-9018  [004]    93.174843: page_ref_mod_and_test: pfn=0x17ac9 flags=0x40068 count=0 mapcount=0 mapping=0xffff880015a78dc1 mt=4 val=-1 ret=1
 => release_pages (ffffffff8117c9e4)
 => free_pages_and_swap_cache (ffffffff811b0697)
 => tlb_flush_mmu_free (ffffffff81199616)
 => tlb_finish_mmu (ffffffff8119a62c)
 => exit_mmap (ffffffff811a53f7)
 => mmput (ffffffff81073f47)
 => do_exit (ffffffff810794e9)
 => do_group_exit (ffffffff81079def)
 => SyS_exit_group (ffffffff81079e74)
 => entry_SYSCALL_64_fastpath (ffffffff817560b6)

This output shows that problem comes from exit path. In exit path,
to improve performance, pages are not freed immediately. They are gathered
and processed by batch. During this process, migration cannot be possible
and CMA allocation is failed. This problem is hard to find without this
page reference tracepoint facility.

Enabling this feature bloat kernel text 30 KB in my configuration.

   text    data     bss     dec     hex filename
12127327        2243616 1507328 15878271         f2487f vmlinux_disabled
12157208        2258880 1507328 15923416         f2f8d8 vmlinux_enabled

v2:
o Use static key of each tracepoints to avoid function call overhead
when tracepoints are disabled.
o Print human-readable page flag through show_page_flags()
o Add more description to Kconfig.debug.

Acked-by: Michal Nazarewicz <mina86@mina86.com>
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
 include/linux/page_ref.h        |  90 +++++++++++++++++++++++++--
 include/trace/events/page_ref.h | 133 ++++++++++++++++++++++++++++++++++++++++
 mm/Kconfig.debug                |  13 ++++
 mm/Makefile                     |   1 +
 mm/debug_page_ref.c             |  53 ++++++++++++++++
 5 files changed, 285 insertions(+), 5 deletions(-)
 create mode 100644 include/trace/events/page_ref.h
 create mode 100644 mm/debug_page_ref.c

diff --git a/include/linux/page_ref.h b/include/linux/page_ref.h
index 534249c..fd6d9a5 100644
--- a/include/linux/page_ref.h
+++ b/include/linux/page_ref.h
@@ -1,6 +1,54 @@
 #include <linux/atomic.h>
 #include <linux/mm_types.h>
 #include <linux/page-flags.h>
+#include <linux/tracepoint-defs.h>
+
+extern struct tracepoint __tracepoint_page_ref_set;
+extern struct tracepoint __tracepoint_page_ref_mod;
+extern struct tracepoint __tracepoint_page_ref_mod_and_test;
+extern struct tracepoint __tracepoint_page_ref_mod_and_return;
+extern struct tracepoint __tracepoint_page_ref_mod_unless;
+extern struct tracepoint __tracepoint_page_ref_freeze;
+extern struct tracepoint __tracepoint_page_ref_unfreeze;
+
+#ifdef CONFIG_DEBUG_PAGE_REF
+#define page_ref_tracepoint_active(t) static_key_false(&(t).key)
+
+extern void __page_ref_set(struct page *page, int v);
+extern void __page_ref_mod(struct page *page, int v);
+extern void __page_ref_mod_and_test(struct page *page, int v, int ret);
+extern void __page_ref_mod_and_return(struct page *page, int v, int ret);
+extern void __page_ref_mod_unless(struct page *page, int v, int u);
+extern void __page_ref_freeze(struct page *page, int v, int ret);
+extern void __page_ref_unfreeze(struct page *page, int v);
+
+#else
+
+#define page_ref_tracepoint_active(t) false
+
+static inline void __page_ref_set(struct page *page, int v)
+{
+}
+static inline void __page_ref_mod(struct page *page, int v)
+{
+}
+static inline void __page_ref_mod_and_test(struct page *page, int v, int ret)
+{
+}
+static inline void __page_ref_mod_and_return(struct page *page, int v, int ret)
+{
+}
+static inline void __page_ref_mod_unless(struct page *page, int v, int u)
+{
+}
+static inline void __page_ref_freeze(struct page *page, int v, int ret)
+{
+}
+static inline void __page_ref_unfreeze(struct page *page, int v)
+{
+}
+
+#endif
 
 static inline int page_count(struct page *page)
 {
@@ -10,6 +58,8 @@ static inline int page_count(struct page *page)
 static inline void set_page_count(struct page *page, int v)
 {
 	atomic_set(&page->_count, v);
+	if (page_ref_tracepoint_active(__tracepoint_page_ref_set))
+		__page_ref_set(page, v);
 }
 
 /*
@@ -24,46 +74,74 @@ static inline void init_page_count(struct page *page)
 static inline void page_ref_add(struct page *page, int nr)
 {
 	atomic_add(nr, &page->_count);
+	if (page_ref_tracepoint_active(__tracepoint_page_ref_mod))
+		__page_ref_mod(page, nr);
 }
 
 static inline void page_ref_sub(struct page *page, int nr)
 {
 	atomic_sub(nr, &page->_count);
+	if (page_ref_tracepoint_active(__tracepoint_page_ref_mod))
+		__page_ref_mod(page, -nr);
 }
 
 static inline void page_ref_inc(struct page *page)
 {
 	atomic_inc(&page->_count);
+	if (page_ref_tracepoint_active(__tracepoint_page_ref_mod))
+		__page_ref_mod(page, 1);
 }
 
 static inline void page_ref_dec(struct page *page)
 {
 	atomic_dec(&page->_count);
+	if (page_ref_tracepoint_active(__tracepoint_page_ref_mod))
+		__page_ref_mod(page, -1);
 }
 
 static inline int page_ref_sub_and_test(struct page *page, int nr)
 {
-	return atomic_sub_and_test(nr, &page->_count);
+	int ret = atomic_sub_and_test(nr, &page->_count);
+
+	if (page_ref_tracepoint_active(__tracepoint_page_ref_mod_and_test))
+		__page_ref_mod_and_test(page, -nr, ret);
+	return ret;
 }
 
 static inline int page_ref_dec_and_test(struct page *page)
 {
-	return atomic_dec_and_test(&page->_count);
+	int ret = atomic_dec_and_test(&page->_count);
+
+	if (page_ref_tracepoint_active(__tracepoint_page_ref_mod_and_test))
+		__page_ref_mod_and_test(page, -1, ret);
+	return ret;
 }
 
 static inline int page_ref_dec_return(struct page *page)
 {
-	return atomic_dec_return(&page->_count);
+	int ret = atomic_dec_return(&page->_count);
+
+	if (page_ref_tracepoint_active(__tracepoint_page_ref_mod_and_return))
+		__page_ref_mod_and_return(page, -1, ret);
+	return ret;
 }
 
 static inline int page_ref_add_unless(struct page *page, int nr, int u)
 {
-	return atomic_add_unless(&page->_count, nr, u);
+	int ret = atomic_add_unless(&page->_count, nr, u);
+
+	if (page_ref_tracepoint_active(__tracepoint_page_ref_mod_unless))
+		__page_ref_mod_unless(page, nr, ret);
+	return ret;
 }
 
 static inline int page_ref_freeze(struct page *page, int count)
 {
-	return likely(atomic_cmpxchg(&page->_count, count, 0) == count);
+	int ret = likely(atomic_cmpxchg(&page->_count, count, 0) == count);
+
+	if (page_ref_tracepoint_active(__tracepoint_page_ref_freeze))
+		__page_ref_freeze(page, count, ret);
+	return ret;
 }
 
 static inline void page_ref_unfreeze(struct page *page, int count)
@@ -72,5 +150,7 @@ static inline void page_ref_unfreeze(struct page *page, int count)
 	VM_BUG_ON(count == 0);
 
 	atomic_set(&page->_count, count);
+	if (page_ref_tracepoint_active(__tracepoint_page_ref_unfreeze))
+		__page_ref_unfreeze(page, count);
 }
 
diff --git a/include/trace/events/page_ref.h b/include/trace/events/page_ref.h
new file mode 100644
index 0000000..9fb920a
--- /dev/null
+++ b/include/trace/events/page_ref.h
@@ -0,0 +1,133 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM page_ref
+
+#if !defined(_TRACE_PAGE_REF_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_PAGE_REF_H
+
+#include <linux/types.h>
+#include <linux/tracepoint.h>
+#include <trace/events/mmflags.h>
+
+DECLARE_EVENT_CLASS(page_ref_mod_template,
+
+	TP_PROTO(struct page *page, int v),
+
+	TP_ARGS(page, v),
+
+	TP_STRUCT__entry(
+		__field(unsigned long, pfn)
+		__field(unsigned long, flags)
+		__field(int, count)
+		__field(int, mapcount)
+		__field(void *, mapping)
+		__field(int, mt)
+		__field(int, val)
+	),
+
+	TP_fast_assign(
+		__entry->pfn = page_to_pfn(page);
+		__entry->flags = page->flags;
+		__entry->count = atomic_read(&page->_count);
+		__entry->mapcount = page_mapcount(page);
+		__entry->mapping = page->mapping;
+		__entry->mt = get_pageblock_migratetype(page);
+		__entry->val = v;
+	),
+
+	TP_printk("pfn=0x%lx flags=%s count=%d mapcount=%d mapping=%p mt=%d val=%d",
+		__entry->pfn,
+		show_page_flags(__entry->flags & ((1UL << NR_PAGEFLAGS) - 1)),
+		__entry->count,
+		__entry->mapcount, __entry->mapping, __entry->mt,
+		__entry->val)
+);
+
+DEFINE_EVENT(page_ref_mod_template, page_ref_set,
+
+	TP_PROTO(struct page *page, int v),
+
+	TP_ARGS(page, v)
+);
+
+DEFINE_EVENT(page_ref_mod_template, page_ref_mod,
+
+	TP_PROTO(struct page *page, int v),
+
+	TP_ARGS(page, v)
+);
+
+DECLARE_EVENT_CLASS(page_ref_mod_and_test_template,
+
+	TP_PROTO(struct page *page, int v, int ret),
+
+	TP_ARGS(page, v, ret),
+
+	TP_STRUCT__entry(
+		__field(unsigned long, pfn)
+		__field(unsigned long, flags)
+		__field(int, count)
+		__field(int, mapcount)
+		__field(void *, mapping)
+		__field(int, mt)
+		__field(int, val)
+		__field(int, ret)
+	),
+
+	TP_fast_assign(
+		__entry->pfn = page_to_pfn(page);
+		__entry->flags = page->flags;
+		__entry->count = atomic_read(&page->_count);
+		__entry->mapcount = page_mapcount(page);
+		__entry->mapping = page->mapping;
+		__entry->mt = get_pageblock_migratetype(page);
+		__entry->val = v;
+		__entry->ret = ret;
+	),
+
+	TP_printk("pfn=0x%lx flags=%s count=%d mapcount=%d mapping=%p mt=%d val=%d ret=%d",
+		__entry->pfn,
+		show_page_flags(__entry->flags & ((1UL << NR_PAGEFLAGS) - 1)),
+		__entry->count,
+		__entry->mapcount, __entry->mapping, __entry->mt,
+		__entry->val, __entry->ret)
+);
+
+DEFINE_EVENT(page_ref_mod_and_test_template, page_ref_mod_and_test,
+
+	TP_PROTO(struct page *page, int v, int ret),
+
+	TP_ARGS(page, v, ret)
+);
+
+DEFINE_EVENT(page_ref_mod_and_test_template, page_ref_mod_and_return,
+
+	TP_PROTO(struct page *page, int v, int ret),
+
+	TP_ARGS(page, v, ret)
+);
+
+DEFINE_EVENT(page_ref_mod_and_test_template, page_ref_mod_unless,
+
+	TP_PROTO(struct page *page, int v, int ret),
+
+	TP_ARGS(page, v, ret)
+);
+
+DEFINE_EVENT(page_ref_mod_and_test_template, page_ref_freeze,
+
+	TP_PROTO(struct page *page, int v, int ret),
+
+	TP_ARGS(page, v, ret)
+);
+
+DEFINE_EVENT(page_ref_mod_template, page_ref_unfreeze,
+
+	TP_PROTO(struct page *page, int v),
+
+	TP_ARGS(page, v)
+);
+
+#endif /* _TRACE_PAGE_COUNT_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/mm/Kconfig.debug b/mm/Kconfig.debug
index ee57dea..0596495 100644
--- a/mm/Kconfig.debug
+++ b/mm/Kconfig.debug
@@ -75,3 +75,16 @@ config PAGE_POISONING_ZERO
 	   allocation.
 
 	   If unsure, say N
+	bool
+
+config DEBUG_PAGE_REF
+	bool "Enable tracepoint to track down page reference manipulation"
+	depends on DEBUG_KERNEL
+	---help---
+	  This is the feature to add tracepoint for tracking down page reference
+	  manipulation. This tracking is useful to diagnosis functional failure
+	  due to migration failure caused by page reference mismatch. Be
+	  careful to turn on this feature because it could bloat some kernel
+	  text. In my configuration, it bloats 30 KB. Although kernel text will
+	  be bloated, there would be no runtime performance overhead if
+	  tracepoint isn't enabled thanks to jump label.
diff --git a/mm/Makefile b/mm/Makefile
index fb1a794..4f0f135 100644
--- a/mm/Makefile
+++ b/mm/Makefile
@@ -99,3 +99,4 @@ obj-$(CONFIG_CMA_DEBUGFS) += cma_debug.o
 obj-$(CONFIG_USERFAULTFD) += userfaultfd.o
 obj-$(CONFIG_IDLE_PAGE_TRACKING) += page_idle.o
 obj-$(CONFIG_FRAME_VECTOR) += frame_vector.o
+obj-$(CONFIG_DEBUG_PAGE_REF) += debug_page_ref.o
diff --git a/mm/debug_page_ref.c b/mm/debug_page_ref.c
new file mode 100644
index 0000000..87e60e8
--- /dev/null
+++ b/mm/debug_page_ref.c
@@ -0,0 +1,53 @@
+#include <linux/tracepoint.h>
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/page_ref.h>
+
+void __page_ref_set(struct page *page, int v)
+{
+	trace_page_ref_set(page, v);
+}
+EXPORT_SYMBOL(__page_ref_set);
+EXPORT_TRACEPOINT_SYMBOL(page_ref_set);
+
+void __page_ref_mod(struct page *page, int v)
+{
+	trace_page_ref_mod(page, v);
+}
+EXPORT_SYMBOL(__page_ref_mod);
+EXPORT_TRACEPOINT_SYMBOL(page_ref_mod);
+
+void __page_ref_mod_and_test(struct page *page, int v, int ret)
+{
+	trace_page_ref_mod_and_test(page, v, ret);
+}
+EXPORT_SYMBOL(__page_ref_mod_and_test);
+EXPORT_TRACEPOINT_SYMBOL(page_ref_mod_and_test);
+
+void __page_ref_mod_and_return(struct page *page, int v, int ret)
+{
+	trace_page_ref_mod_and_return(page, v, ret);
+}
+EXPORT_SYMBOL(__page_ref_mod_and_return);
+EXPORT_TRACEPOINT_SYMBOL(page_ref_mod_and_return);
+
+void __page_ref_mod_unless(struct page *page, int v, int u)
+{
+	trace_page_ref_mod_unless(page, v, u);
+}
+EXPORT_SYMBOL(__page_ref_mod_unless);
+EXPORT_TRACEPOINT_SYMBOL(page_ref_mod_unless);
+
+void __page_ref_freeze(struct page *page, int v, int ret)
+{
+	trace_page_ref_freeze(page, v, ret);
+}
+EXPORT_SYMBOL(__page_ref_freeze);
+EXPORT_TRACEPOINT_SYMBOL(page_ref_freeze);
+
+void __page_ref_unfreeze(struct page *page, int v)
+{
+	trace_page_ref_unfreeze(page, v);
+}
+EXPORT_SYMBOL(__page_ref_unfreeze);
+EXPORT_TRACEPOINT_SYMBOL(page_ref_unfreeze);
-- 
1.9.1

[toc] | [next] | [standalone]

#1334153 — Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation

Hello Joonsoo,

On (02/15/16 12:04), js1304@gmail.com wrote:
[..]
> <...>-9018  [004]    92.678375: page_ref_set:         pfn=0x17ac9 flags=0x0 count=1 mapcount=0 mapping=(nil) mt=4 val=1
> <...>-9018  [004]    92.678378: kernel_stack:
>  => get_page_from_freelist (ffffffff81176659)
>  => __alloc_pages_nodemask (ffffffff81176d22)
>  => alloc_pages_vma (ffffffff811bf675)
>  => handle_mm_fault (ffffffff8119e693)
>  => __do_page_fault (ffffffff810631ea)
>  => trace_do_page_fault (ffffffff81063543)
>  => do_async_page_fault (ffffffff8105c40a)
>  => async_page_fault (ffffffff817581d8)
> [snip]
> <...>-9018  [004]    92.678379: page_ref_mod:         pfn=0x17ac9 flags=0x40048 count=2 mapcount=1 mapping=0xffff880015a78dc1 mt=4 val=1
> [snip]
[..]
> o Print human-readable page flag through show_page_flags()

not even a nitpick, just for note, the examples don't use show_page_flags().


[..]
> diff --git a/include/linux/page_ref.h b/include/linux/page_ref.h
> index 534249c..fd6d9a5 100644
> --- a/include/linux/page_ref.h
> +++ b/include/linux/page_ref.h
> @@ -1,6 +1,54 @@
>  #include <linux/atomic.h>
>  #include <linux/mm_types.h>
>  #include <linux/page-flags.h>

will this compile with !CONFIG_TRACEPOINTS config?

+#ifdef CONFIG_TRACEPOINTS
 #include <linux/tracepoint-defs.h>

 extern struct tracepoint __tracepoint_page_ref_set;
 extern struct tracepoint __tracepoint_page_ref_mod;
 extern struct tracepoint __tracepoint_page_ref_mod_and_test;
 extern struct tracepoint __tracepoint_page_ref_mod_and_return;
 extern struct tracepoint __tracepoint_page_ref_mod_unless;
 extern struct tracepoint __tracepoint_page_ref_freeze;
 extern struct tracepoint __tracepoint_page_ref_unfreeze;

 #ifdef CONFIG_DEBUG_PAGE_REF
 #define page_ref_tracepoint_active(t) static_key_false(&(t).key)

 extern void __page_ref_set(struct page *page, int v);
 extern void __page_ref_mod(struct page *page, int v);
 extern void __page_ref_mod_and_test(struct page *page, int v, int ret);
 extern void __page_ref_mod_and_return(struct page *page, int v, int ret);
 extern void __page_ref_mod_unless(struct page *page, int v, int u);
 extern void __page_ref_freeze(struct page *page, int v, int ret);
 extern void __page_ref_unfreeze(struct page *page, int v);

 #else

 #define page_ref_tracepoint_active(t) false

 static inline void __page_ref_set(struct page *page, int v)
 {
 }
 static inline void __page_ref_mod(struct page *page, int v)
 {
 }
 static inline void __page_ref_mod_and_test(struct page *page, int v, int ret)
 {
 }
 static inline void __page_ref_mod_and_return(struct page *page, int v, int ret)
 {
 }
 static inline void __page_ref_mod_unless(struct page *page, int v, int u)
 {
 }
 static inline void __page_ref_freeze(struct page *page, int v, int ret)
 {
 }
 static inline void __page_ref_unfreeze(struct page *page, int v)
 {
 }

 #endif /* CONFIG_DEBUG_PAGE_REF */

+#endif /* CONFIG_TRACEPOINTS */

	-ss

[toc] | [prev] | [next] | [standalone]

#1334157 — Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation

On (02/15/16 14:08), Sergey Senozhatsky wrote:
> 
> will this compile with !CONFIG_TRACEPOINTS config?
> 

uh.. sorry, was composed in email client. seems the correct way to do it is

+#if defined CONFIG_DEBUG_PAGE_REF && defined CONFIG_TRACEPOINTS

 #include <linux/tracepoint-defs.h>

 #define page_ref_tracepoint_active(t) static_key_false(&(t).key)

 extern struct tracepoint __tracepoint_page_ref_set;
 ...

 extern void __page_ref_set(struct page *page, int v);
 ...

#else

 #define page_ref_tracepoint_active(t) false

 static inline void __page_ref_set(struct page *page, int v)
 {
 }
 ...

#endif



or add a dependency of PAGE_REF on CONFIG_TRACEPOINTS in Kconfig.

	-ss

[toc] | [prev] | [next] | [standalone]

#1334476 — Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation

2016-02-15 14:28 GMT+09:00 Sergey Senozhatsky
<sergey.senozhatsky.work@gmail.com>:
> On (02/15/16 14:08), Sergey Senozhatsky wrote:
>>
>> will this compile with !CONFIG_TRACEPOINTS config?
>>

Yes, even if !CONFIG_TRACEPOINTS, it is compiled well.

> uh.. sorry, was composed in email client. seems the correct way to do it is
>
> +#if defined CONFIG_DEBUG_PAGE_REF && defined CONFIG_TRACEPOINTS
>
>  #include <linux/tracepoint-defs.h>
>
>  #define page_ref_tracepoint_active(t) static_key_false(&(t).key)
>
>  extern struct tracepoint __tracepoint_page_ref_set;
>  ...
>
>  extern void __page_ref_set(struct page *page, int v);
>  ...
>
> #else
>
>  #define page_ref_tracepoint_active(t) false
>
>  static inline void __page_ref_set(struct page *page, int v)
>  {
>  }
>  ...
>
> #endif
>
>
>
> or add a dependency of PAGE_REF on CONFIG_TRACEPOINTS in Kconfig.

Thanks for catching it.
I will add "depends on CONFIG_TRACEPOINTS" to Kconfig because
this feature has no meaning if !CONFIG_TRACEPOINTS.


Thanks.

[toc] | [prev] | [next] | [standalone]

#1334564 — Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation

On Mon, 15 Feb 2016 12:04:50 +0900
js1304@gmail.com wrote:

> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> 
> CMA allocation should be guaranteed to succeed by definition, but,
> unfortunately, it would be failed sometimes. It is hard to track down
> the problem, because it is related to page reference manipulation and
> we don't have any facility to analyze it.
> 
> This patch adds tracepoints to track down page reference manipulation.
> With it, we can find exact reason of failure and can fix the problem.
> Following is an example of tracepoint output.
> 
> <...>-9018  [004]    92.678375: page_ref_set:         pfn=0x17ac9 flags=0x0 count=1 mapcount=0 mapping=(nil) mt=4 val=1
> <...>-9018  [004]    92.678378: kernel_stack:
>  => get_page_from_freelist (ffffffff81176659)
>  => __alloc_pages_nodemask (ffffffff81176d22)
>  => alloc_pages_vma (ffffffff811bf675)
>  => handle_mm_fault (ffffffff8119e693)
>  => __do_page_fault (ffffffff810631ea)
>  => trace_do_page_fault (ffffffff81063543)
>  => do_async_page_fault (ffffffff8105c40a)
>  => async_page_fault (ffffffff817581d8)  
> [snip]
> <...>-9018  [004]    92.678379: page_ref_mod:         pfn=0x17ac9 flags=0x40048 count=2 mapcount=1 mapping=0xffff880015a78dc1 mt=4 val=1
> [snip]
> ...
> ...
> <...>-9131  [001]    93.174468: test_pages_isolated:  start_pfn=0x17800 end_pfn=0x17c00 fin_pfn=0x17ac9 ret=fail
> [snip]
> <...>-9018  [004]    93.174843: page_ref_mod_and_test: pfn=0x17ac9 flags=0x40068 count=0 mapcount=0 mapping=0xffff880015a78dc1 mt=4 val=-1 ret=1
>  => release_pages (ffffffff8117c9e4)
>  => free_pages_and_swap_cache (ffffffff811b0697)
>  => tlb_flush_mmu_free (ffffffff81199616)
>  => tlb_finish_mmu (ffffffff8119a62c)
>  => exit_mmap (ffffffff811a53f7)
>  => mmput (ffffffff81073f47)
>  => do_exit (ffffffff810794e9)
>  => do_group_exit (ffffffff81079def)
>  => SyS_exit_group (ffffffff81079e74)
>  => entry_SYSCALL_64_fastpath (ffffffff817560b6)  
> 
> This output shows that problem comes from exit path. In exit path,
> to improve performance, pages are not freed immediately. They are gathered
> and processed by batch. During this process, migration cannot be possible
> and CMA allocation is failed. This problem is hard to find without this
> page reference tracepoint facility.
> 
> Enabling this feature bloat kernel text 30 KB in my configuration.
> 
>    text    data     bss     dec     hex filename
> 12127327        2243616 1507328 15878271         f2487f vmlinux_disabled
> 12157208        2258880 1507328 15923416         f2f8d8 vmlinux_enabled
> 
> v2:
> o Use static key of each tracepoints to avoid function call overhead
> when tracepoints are disabled.
> o Print human-readable page flag through show_page_flags()
> o Add more description to Kconfig.debug.
> 
> Acked-by: Michal Nazarewicz <mina86@mina86.com>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> ---
>  include/linux/page_ref.h        |  90 +++++++++++++++++++++++++--
>  include/trace/events/page_ref.h | 133 ++++++++++++++++++++++++++++++++++++++++
>  mm/Kconfig.debug                |  13 ++++
>  mm/Makefile                     |   1 +
>  mm/debug_page_ref.c             |  53 ++++++++++++++++
>  5 files changed, 285 insertions(+), 5 deletions(-)
>  create mode 100644 include/trace/events/page_ref.h
>  create mode 100644 mm/debug_page_ref.c
> 
> diff --git a/include/linux/page_ref.h b/include/linux/page_ref.h
> index 534249c..fd6d9a5 100644
> --- a/include/linux/page_ref.h
> +++ b/include/linux/page_ref.h
> @@ -1,6 +1,54 @@
>  #include <linux/atomic.h>
>  #include <linux/mm_types.h>
>  #include <linux/page-flags.h>
> +#include <linux/tracepoint-defs.h>
> +
> +extern struct tracepoint __tracepoint_page_ref_set;
> +extern struct tracepoint __tracepoint_page_ref_mod;
> +extern struct tracepoint __tracepoint_page_ref_mod_and_test;
> +extern struct tracepoint __tracepoint_page_ref_mod_and_return;
> +extern struct tracepoint __tracepoint_page_ref_mod_unless;
> +extern struct tracepoint __tracepoint_page_ref_freeze;
> +extern struct tracepoint __tracepoint_page_ref_unfreeze;
> +
> +#ifdef CONFIG_DEBUG_PAGE_REF
> +#define page_ref_tracepoint_active(t) static_key_false(&(t).key)

Please don't open code this. Use the following instead:

  trace_page_ref_set_enabled()
  trace_page_ref_mod_enabled()
  trace_page_ref_mod_and_test_enabled()
  trace_page_ref_mod_and_return_enabled()
  trace_page_ref_mod_unless_enabled()
  trace_page_ref_freeze_enabled()
  trace_page_ref_unfreeze_enabled()

They return true when CONFIG_TRACEPOINTS is configured in and the
tracepoint is enabled, and false otherwise.

-- Steve


> +
> +extern void __page_ref_set(struct page *page, int v);
> +extern void __page_ref_mod(struct page *page, int v);
> +extern void __page_ref_mod_and_test(struct page *page, int v, int ret);
> +extern void __page_ref_mod_and_return(struct page *page, int v, int ret);
> +extern void __page_ref_mod_unless(struct page *page, int v, int u);
> +extern void __page_ref_freeze(struct page *page, int v, int ret);
> +extern void __page_ref_unfreeze(struct page *page, int v);
> +
> +#else
> +
>

[toc] | [prev] | [next] | [standalone]

#1334893 — Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation

On Mon, Feb 15, 2016 at 11:07:41AM -0500, Steven Rostedt wrote:
> On Mon, 15 Feb 2016 12:04:50 +0900
> js1304@gmail.com wrote:
> 
> > From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> > 
> > CMA allocation should be guaranteed to succeed by definition, but,
> > unfortunately, it would be failed sometimes. It is hard to track down
> > the problem, because it is related to page reference manipulation and
> > we don't have any facility to analyze it.
> > 
> > This patch adds tracepoints to track down page reference manipulation.
> > With it, we can find exact reason of failure and can fix the problem.
> > Following is an example of tracepoint output.
> > 
> > <...>-9018  [004]    92.678375: page_ref_set:         pfn=0x17ac9 flags=0x0 count=1 mapcount=0 mapping=(nil) mt=4 val=1
> > <...>-9018  [004]    92.678378: kernel_stack:
> >  => get_page_from_freelist (ffffffff81176659)
> >  => __alloc_pages_nodemask (ffffffff81176d22)
> >  => alloc_pages_vma (ffffffff811bf675)
> >  => handle_mm_fault (ffffffff8119e693)
> >  => __do_page_fault (ffffffff810631ea)
> >  => trace_do_page_fault (ffffffff81063543)
> >  => do_async_page_fault (ffffffff8105c40a)
> >  => async_page_fault (ffffffff817581d8)  
> > [snip]
> > <...>-9018  [004]    92.678379: page_ref_mod:         pfn=0x17ac9 flags=0x40048 count=2 mapcount=1 mapping=0xffff880015a78dc1 mt=4 val=1
> > [snip]
> > ...
> > ...
> > <...>-9131  [001]    93.174468: test_pages_isolated:  start_pfn=0x17800 end_pfn=0x17c00 fin_pfn=0x17ac9 ret=fail
> > [snip]
> > <...>-9018  [004]    93.174843: page_ref_mod_and_test: pfn=0x17ac9 flags=0x40068 count=0 mapcount=0 mapping=0xffff880015a78dc1 mt=4 val=-1 ret=1
> >  => release_pages (ffffffff8117c9e4)
> >  => free_pages_and_swap_cache (ffffffff811b0697)
> >  => tlb_flush_mmu_free (ffffffff81199616)
> >  => tlb_finish_mmu (ffffffff8119a62c)
> >  => exit_mmap (ffffffff811a53f7)
> >  => mmput (ffffffff81073f47)
> >  => do_exit (ffffffff810794e9)
> >  => do_group_exit (ffffffff81079def)
> >  => SyS_exit_group (ffffffff81079e74)
> >  => entry_SYSCALL_64_fastpath (ffffffff817560b6)  
> > 
> > This output shows that problem comes from exit path. In exit path,
> > to improve performance, pages are not freed immediately. They are gathered
> > and processed by batch. During this process, migration cannot be possible
> > and CMA allocation is failed. This problem is hard to find without this
> > page reference tracepoint facility.
> > 
> > Enabling this feature bloat kernel text 30 KB in my configuration.
> > 
> >    text    data     bss     dec     hex filename
> > 12127327        2243616 1507328 15878271         f2487f vmlinux_disabled
> > 12157208        2258880 1507328 15923416         f2f8d8 vmlinux_enabled
> > 
> > v2:
> > o Use static key of each tracepoints to avoid function call overhead
> > when tracepoints are disabled.
> > o Print human-readable page flag through show_page_flags()
> > o Add more description to Kconfig.debug.
> > 
> > Acked-by: Michal Nazarewicz <mina86@mina86.com>
> > Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> > ---
> >  include/linux/page_ref.h        |  90 +++++++++++++++++++++++++--
> >  include/trace/events/page_ref.h | 133 ++++++++++++++++++++++++++++++++++++++++
> >  mm/Kconfig.debug                |  13 ++++
> >  mm/Makefile                     |   1 +
> >  mm/debug_page_ref.c             |  53 ++++++++++++++++
> >  5 files changed, 285 insertions(+), 5 deletions(-)
> >  create mode 100644 include/trace/events/page_ref.h
> >  create mode 100644 mm/debug_page_ref.c
> > 
> > diff --git a/include/linux/page_ref.h b/include/linux/page_ref.h
> > index 534249c..fd6d9a5 100644
> > --- a/include/linux/page_ref.h
> > +++ b/include/linux/page_ref.h
> > @@ -1,6 +1,54 @@
> >  #include <linux/atomic.h>
> >  #include <linux/mm_types.h>
> >  #include <linux/page-flags.h>
> > +#include <linux/tracepoint-defs.h>
> > +
> > +extern struct tracepoint __tracepoint_page_ref_set;
> > +extern struct tracepoint __tracepoint_page_ref_mod;
> > +extern struct tracepoint __tracepoint_page_ref_mod_and_test;
> > +extern struct tracepoint __tracepoint_page_ref_mod_and_return;
> > +extern struct tracepoint __tracepoint_page_ref_mod_unless;
> > +extern struct tracepoint __tracepoint_page_ref_freeze;
> > +extern struct tracepoint __tracepoint_page_ref_unfreeze;
> > +
> > +#ifdef CONFIG_DEBUG_PAGE_REF
> > +#define page_ref_tracepoint_active(t) static_key_false(&(t).key)
> 
> Please don't open code this. Use the following instead:
> 
>   trace_page_ref_set_enabled()
>   trace_page_ref_mod_enabled()
>   trace_page_ref_mod_and_test_enabled()
>   trace_page_ref_mod_and_return_enabled()
>   trace_page_ref_mod_unless_enabled()
>   trace_page_ref_freeze_enabled()
>   trace_page_ref_unfreeze_enabled()
> 
> They return true when CONFIG_TRACEPOINTS is configured in and the
> tracepoint is enabled, and false otherwise.

This implementation is what you proposed before. Please refer below
link and source.

https://lkml.org/lkml/2015/12/9/699
arch/x86/include/asm/msr.h

There is header file dependency problem between mm.h and tracepoint.h.
page_ref.h should be included in mm.h and tracepoint.h cannot
be included in this case.

Thanks.

[toc] | [prev] | [next] | [standalone]

#1334899 — Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation

On Tue, 16 Feb 2016 09:47:20 +0900
Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:

> > They return true when CONFIG_TRACEPOINTS is configured in and the
> > tracepoint is enabled, and false otherwise.  
> 
> This implementation is what you proposed before. Please refer below
> link and source.
> 
> https://lkml.org/lkml/2015/12/9/699
> arch/x86/include/asm/msr.h

That was a year ago, how am I suppose to remember ;-)

> 
> There is header file dependency problem between mm.h and tracepoint.h.
> page_ref.h should be included in mm.h and tracepoint.h cannot
> be included in this case.

Ah, OK, I forgot about that. I'll take another look at it again.

A lot happened since then, that's all a fog to me.

-- Steve

[toc] | [prev] | [next] | [standalone]

#1337103 — Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation

2016-02-16 10:16 GMT+09:00 Steven Rostedt <rostedt@goodmis.org>:
> On Tue, 16 Feb 2016 09:47:20 +0900
> Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
>
>> > They return true when CONFIG_TRACEPOINTS is configured in and the
>> > tracepoint is enabled, and false otherwise.
>>
>> This implementation is what you proposed before. Please refer below
>> link and source.
>>
>> https://lkml.org/lkml/2015/12/9/699
>> arch/x86/include/asm/msr.h
>
> That was a year ago, how am I suppose to remember ;-)

I think you are smart enough to remember. :)
I will add it on commit description on next spin.

>>
>> There is header file dependency problem between mm.h and tracepoint.h.
>> page_ref.h should be included in mm.h and tracepoint.h cannot
>> be included in this case.
>
> Ah, OK, I forgot about that. I'll take another look at it again.
>
> A lot happened since then, that's all a fog to me.

Okay. Please let me know result of another look.

Thanks.

[toc] | [prev] | [next] | [standalone]

#1337394 — Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation

On Thu, 18 Feb 2016 16:46:08 +0900
Joonsoo Kim <js1304@gmail.com> wrote:

> 2016-02-16 10:16 GMT+09:00 Steven Rostedt <rostedt@goodmis.org>:
> > On Tue, 16 Feb 2016 09:47:20 +0900
> > Joonsoo Kim <iamjoonsoo.kim@lge.com> wrote:
> >  
> >> > They return true when CONFIG_TRACEPOINTS is configured in and the
> >> > tracepoint is enabled, and false otherwise.  
> >>
> >> This implementation is what you proposed before. Please refer below
> >> link and source.
> >>
> >> https://lkml.org/lkml/2015/12/9/699
> >> arch/x86/include/asm/msr.h  
> >
> > That was a year ago, how am I suppose to remember ;-)  
> 
> I think you are smart enough to remember. :)
> I will add it on commit description on next spin.
> 
>

Better yet, add it to the code. I'll reply to the patch.

-- Steve

[toc] | [prev] | [next] | [standalone]

#1337390 — Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation

On Mon, 15 Feb 2016 12:04:50 +0900
js1304@gmail.com wrote:


> diff --git a/include/linux/page_ref.h b/include/linux/page_ref.h
> index 534249c..fd6d9a5 100644
> --- a/include/linux/page_ref.h
> +++ b/include/linux/page_ref.h
> @@ -1,6 +1,54 @@
>  #include <linux/atomic.h>
>  #include <linux/mm_types.h>
>  #include <linux/page-flags.h>
> +#include <linux/tracepoint-defs.h>
> +
> +extern struct tracepoint __tracepoint_page_ref_set;
> +extern struct tracepoint __tracepoint_page_ref_mod;
> +extern struct tracepoint __tracepoint_page_ref_mod_and_test;
> +extern struct tracepoint __tracepoint_page_ref_mod_and_return;
> +extern struct tracepoint __tracepoint_page_ref_mod_unless;
> +extern struct tracepoint __tracepoint_page_ref_freeze;
> +extern struct tracepoint __tracepoint_page_ref_unfreeze;
> +
> +#ifdef CONFIG_DEBUG_PAGE_REF

Please add a comment here. Something to the effect of:

/*
 * Ideally we would want to use the trace_<tracepoint>_enabled() helper
 * functions. But due to include header file issues, that is not
 * feasible. Instead we have to open code the static key functions.
 *
 * See trace_##name##_enabled(void) in include/linux/tracepoint.h
 */

I may have to work on something that lets these helpers be defined in
headers. I have some ideas on how to do that. But for now, this
solution is fine.

-- Steve


> +#define page_ref_tracepoint_active(t) static_key_false(&(t).key)
> +
> +extern void __page_ref_set(struct page *page, int v);
> +extern void __page_ref_mod(struct page *page, int v);
> +extern void __page_ref_mod_and_test(struct page *page, int v, int ret);
> +extern void __page_ref_mod_and_return(struct page *page, int v, int ret);
> +extern void __page_ref_mod_unless(struct page *page, int v, int u);
> +extern void __page_ref_freeze(struct page *page, int v, int ret);
> +extern void __page_ref_unfreeze(struct page *page, int v);
> +
> +#else
> +
> +#define page_ref_tracepoint_active(t) false
> +
> +static inline void __page_ref_set(struct page *page, int v)
> +{
> +}
> +static inline void __page_ref_mod(struct page *page, int v)
> +{
> +}
> +static inline void __page_ref_mod_and_test(struct page *page, int v, int ret)
> +{
> +}
> +static inline void __page_ref_mod_and_return(struct page *page, int v, int ret)
> +{
> +}
> +static inline void __page_ref_mod_unless(struct page *page, int v, int u)
> +{
> +}
> +static inline void __page_ref_freeze(struct page *page, int v, int ret)
> +{
> +}
> +static inline void __page_ref_unfreeze(struct page *page, int v)
> +{
> +}
> +
> +#endif
>  
>

[toc] | [prev] | [next] | [standalone]

#1337808 — Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation

On (02/18/16 09:29), Steven Rostedt wrote:
> > diff --git a/include/linux/page_ref.h b/include/linux/page_ref.h
> > index 534249c..fd6d9a5 100644
> > --- a/include/linux/page_ref.h
> > +++ b/include/linux/page_ref.h
> > @@ -1,6 +1,54 @@
> >  #include <linux/atomic.h>
> >  #include <linux/mm_types.h>
> >  #include <linux/page-flags.h>
> > +#include <linux/tracepoint-defs.h>
> > +
> > +extern struct tracepoint __tracepoint_page_ref_set;
> > +extern struct tracepoint __tracepoint_page_ref_mod;
> > +extern struct tracepoint __tracepoint_page_ref_mod_and_test;
> > +extern struct tracepoint __tracepoint_page_ref_mod_and_return;
> > +extern struct tracepoint __tracepoint_page_ref_mod_unless;
> > +extern struct tracepoint __tracepoint_page_ref_freeze;
> > +extern struct tracepoint __tracepoint_page_ref_unfreeze;
> > +
> > +#ifdef CONFIG_DEBUG_PAGE_REF
> 
> Please add a comment here. Something to the effect of:
> 
> /*
>  * Ideally we would want to use the trace_<tracepoint>_enabled() helper
>  * functions. But due to include header file issues, that is not
>  * feasible. Instead we have to open code the static key functions.
>  *
>  * See trace_##name##_enabled(void) in include/linux/tracepoint.h
>  */
> 

not sure if it's worth mentioning in the comment, but the other
concern here is the performance impact of an extra function call,
I believe. otherwise, Joonsoo would just do:

in include/linux/page_ref.h

static inline void set_page_count(struct page *page, int v)
{
	atomic_set(&page->_count, v);
	__page_ref_set(page, v);
}
...



and in mm/debug_page_ref.c

void __page_ref_set(struct page *page, int v)
{
	if (trace_page_ref_set_enabled())
		trace_page_ref_set(page, v);
}
EXPORT_SYMBOL(__page_ref_set);
EXPORT_TRACEPOINT_SYMBOL(page_ref_set);
...

	-ss

[toc] | [prev] | [next] | [standalone]

#1337822 — Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation

2016-02-19 9:34 GMT+09:00 Sergey Senozhatsky
<sergey.senozhatsky.work@gmail.com>:
> On (02/18/16 09:29), Steven Rostedt wrote:
>> > diff --git a/include/linux/page_ref.h b/include/linux/page_ref.h
>> > index 534249c..fd6d9a5 100644
>> > --- a/include/linux/page_ref.h
>> > +++ b/include/linux/page_ref.h
>> > @@ -1,6 +1,54 @@
>> >  #include <linux/atomic.h>
>> >  #include <linux/mm_types.h>
>> >  #include <linux/page-flags.h>
>> > +#include <linux/tracepoint-defs.h>
>> > +
>> > +extern struct tracepoint __tracepoint_page_ref_set;
>> > +extern struct tracepoint __tracepoint_page_ref_mod;
>> > +extern struct tracepoint __tracepoint_page_ref_mod_and_test;
>> > +extern struct tracepoint __tracepoint_page_ref_mod_and_return;
>> > +extern struct tracepoint __tracepoint_page_ref_mod_unless;
>> > +extern struct tracepoint __tracepoint_page_ref_freeze;
>> > +extern struct tracepoint __tracepoint_page_ref_unfreeze;
>> > +
>> > +#ifdef CONFIG_DEBUG_PAGE_REF
>>
>> Please add a comment here. Something to the effect of:
>>
>> /*
>>  * Ideally we would want to use the trace_<tracepoint>_enabled() helper
>>  * functions. But due to include header file issues, that is not
>>  * feasible. Instead we have to open code the static key functions.
>>  *
>>  * See trace_##name##_enabled(void) in include/linux/tracepoint.h
>>  */
>>
>
> not sure if it's worth mentioning in the comment, but the other
> concern here is the performance impact of an extra function call,
> I believe. otherwise, Joonsoo would just do:

It's very natural thing so I'm not sure it is worth mentioning.

> in include/linux/page_ref.h
>
> static inline void set_page_count(struct page *page, int v)
> {
>         atomic_set(&page->_count, v);
>         __page_ref_set(page, v);
> }
> ...
>
>
>
> and in mm/debug_page_ref.c
>
> void __page_ref_set(struct page *page, int v)
> {
>         if (trace_page_ref_set_enabled())
>                 trace_page_ref_set(page, v);
> }
> EXPORT_SYMBOL(__page_ref_set);
> EXPORT_TRACEPOINT_SYMBOL(page_ref_set);

It is what I did in v1.

Thanks.

[toc] | [prev] | [next] | [standalone]

#1337825 — Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation

On Fri, 19 Feb 2016 10:39:10 +0900
Joonsoo Kim <js1304@gmail.com> wrote:


> > not sure if it's worth mentioning in the comment, but the other
> > concern here is the performance impact of an extra function call,
> > I believe. otherwise, Joonsoo would just do:  
> 
> It's very natural thing so I'm not sure it is worth mentioning.
> 

Agreed.

-- Steve

[toc] | [prev] | [next] | [standalone]

#1337836 — Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation

On (02/19/16 10:39), Joonsoo Kim wrote:
[..]
> > not sure if it's worth mentioning in the comment, but the other
> > concern here is the performance impact of an extra function call,
> > I believe. otherwise, Joonsoo would just do:
> 
> It's very natural thing so I'm not sure it is worth mentioning.

agree.

> > and in mm/debug_page_ref.c
> >
> > void __page_ref_set(struct page *page, int v)
> > {
> >         if (trace_page_ref_set_enabled())
> >                 trace_page_ref_set(page, v);
> > }
> > EXPORT_SYMBOL(__page_ref_set);
> > EXPORT_TRACEPOINT_SYMBOL(page_ref_set);
> 
> It is what I did in v1.

ah... indeed. well, "That was a year ago, how am I suppose to remember"

	-ss

[toc] | [prev] | [next] | [standalone]

#1337818 — Re: [PATCH 2/2] mm/page_ref: add tracepoint to track down page reference manipulation

2016-02-18 23:29 GMT+09:00 Steven Rostedt <rostedt@goodmis.org>:
> On Mon, 15 Feb 2016 12:04:50 +0900
> js1304@gmail.com wrote:
>
>
>> diff --git a/include/linux/page_ref.h b/include/linux/page_ref.h
>> index 534249c..fd6d9a5 100644
>> --- a/include/linux/page_ref.h
>> +++ b/include/linux/page_ref.h
>> @@ -1,6 +1,54 @@
>>  #include <linux/atomic.h>
>>  #include <linux/mm_types.h>
>>  #include <linux/page-flags.h>
>> +#include <linux/tracepoint-defs.h>
>> +
>> +extern struct tracepoint __tracepoint_page_ref_set;
>> +extern struct tracepoint __tracepoint_page_ref_mod;
>> +extern struct tracepoint __tracepoint_page_ref_mod_and_test;
>> +extern struct tracepoint __tracepoint_page_ref_mod_and_return;
>> +extern struct tracepoint __tracepoint_page_ref_mod_unless;
>> +extern struct tracepoint __tracepoint_page_ref_freeze;
>> +extern struct tracepoint __tracepoint_page_ref_unfreeze;
>> +
>> +#ifdef CONFIG_DEBUG_PAGE_REF
>
> Please add a comment here. Something to the effect of:

Okay!

> /*
>  * Ideally we would want to use the trace_<tracepoint>_enabled() helper
>  * functions. But due to include header file issues, that is not
>  * feasible. Instead we have to open code the static key functions.
>  *
>  * See trace_##name##_enabled(void) in include/linux/tracepoint.h
>  */
>
> I may have to work on something that lets these helpers be defined in
> headers. I have some ideas on how to do that. But for now, this
> solution is fine.

Okay.

Thanks.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web