[PATCH lttng-modules 2/5] fix: mm/vmscan: drop may_writepage and classzone_idx from direct reclaim begin template (v5.2)

[PATCH lttng-modules 2/5] fix: mm/vmscan: drop may_writepage and classzone_idx from direct reclaim begin template (v5.2)

[Michael Jeanson]

See upstream commit:

  commit 3481c37ffa1de58ef140d0fe9eabf56305e74666
  Author: Yafang Shao <laoar.shao@gmail.com>
  Date:   Mon May 13 17:19:14 2019 -0700

    mm/vmscan: drop may_writepage and classzone_idx from direct reclaim begin template

    There are three tracepoints using this template, which are
    mm_vmscan_direct_reclaim_begin,
    mm_vmscan_memcg_reclaim_begin,
    mm_vmscan_memcg_softlimit_reclaim_begin.

    Regarding mm_vmscan_direct_reclaim_begin,
    sc.may_writepage is !laptop_mode, that's a static setting, and
    reclaim_idx is derived from gfp_mask which is already show in this
    tracepoint.

    Regarding mm_vmscan_memcg_reclaim_begin,
    may_writepage is !laptop_mode too, and reclaim_idx is (MAX_NR_ZONES-1),
    which are both static value.

    mm_vmscan_memcg_softlimit_reclaim_begin is the same with
    mm_vmscan_memcg_reclaim_begin.

    So we can drop them all.

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
---
 .../events/lttng-module/mm_vmscan.h           | 37 ++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/instrumentation/events/lttng-module/mm_vmscan.h b/instrumentation/events/lttng-module/mm_vmscan.h
index cc7a275..db4a906 100644
--- a/instrumentation/events/lttng-module/mm_vmscan.h
+++ b/instrumentation/events/lttng-module/mm_vmscan.h
@@ -114,7 +114,42 @@ LTTNG_TRACEPOINT_EVENT(mm_vmscan_wakeup_kswapd,
 )
 #endif
 
-#if (LINUX_VERSION_CODE >= KERNEL_VERSION(4,8,0))
+#if (LINUX_VERSION_CODE >= KERNEL_VERSION(5,2,0))
+
+LTTNG_TRACEPOINT_EVENT_CLASS(mm_vmscan_direct_reclaim_begin_template,
+
+	TP_PROTO(int order, gfp_t gfp_flags),
+
+	TP_ARGS(order, gfp_flags),
+
+	TP_FIELDS(
+		ctf_integer(int, order, order)
+		ctf_integer(gfp_t, gfp_flags, gfp_flags)
+	)
+)
+
+LTTNG_TRACEPOINT_EVENT_INSTANCE(mm_vmscan_direct_reclaim_begin_template, mm_vmscan_direct_reclaim_begin,
+
+	TP_PROTO(int order, gfp_t gfp_flags),
+
+	TP_ARGS(order, gfp_flags)
+)
+
+LTTNG_TRACEPOINT_EVENT_INSTANCE(mm_vmscan_direct_reclaim_begin_template, mm_vmscan_memcg_reclaim_begin,
+
+	TP_PROTO(int order, gfp_t gfp_flags),
+
+	TP_ARGS(order, gfp_flags)
+)
+
+LTTNG_TRACEPOINT_EVENT_INSTANCE(mm_vmscan_direct_reclaim_begin_template, mm_vmscan_memcg_softlimit_reclaim_begin,
+
+	TP_PROTO(int order, gfp_t gfp_flags),
+
+	TP_ARGS(order, gfp_flags)
+)
+
+#elif (LINUX_VERSION_CODE >= KERNEL_VERSION(4,8,0))
 
 LTTNG_TRACEPOINT_EVENT_CLASS(mm_vmscan_direct_reclaim_begin_template,
 
-- 
2.17.1

[PATCH lttng-modules 3/5] fix: mm/vmscan: simplify trace_reclaim_flags and trace_shrink_flags (v5.2)

[Michael Jeanson]

See upstream commit:

  commit 60b62ff7cc4217ac3de76535fa4c1510a798dbcb
  Author: Yafang Shao <laoar.shao@gmail.com>
  Date:   Mon May 13 17:23:08 2019 -0700

    mm/vmscan: simplify trace_reclaim_flags and trace_shrink_flags

    trace_reclaim_flags and trace_shrink_flags are almost the same.
    We can simplify them to avoid redundant code.

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
---
 .../events/lttng-module/mm_vmscan.h           | 52 +++++++++++++++++--
 1 file changed, 49 insertions(+), 3 deletions(-)

diff --git a/instrumentation/events/lttng-module/mm_vmscan.h b/instrumentation/events/lttng-module/mm_vmscan.h
index db4a906..417472c 100644
--- a/instrumentation/events/lttng-module/mm_vmscan.h
+++ b/instrumentation/events/lttng-module/mm_vmscan.h
@@ -26,7 +26,16 @@ typedef int isolate_mode_t;
 
 #endif
 
-#if (LINUX_VERSION_CODE >= KERNEL_VERSION(4,5,0))
+#if (LINUX_VERSION_CODE >= KERNEL_VERSION(5,2,0))
+
+#include <linux/mm_inline.h>
+
+#define trace_reclaim_flags(file) ( \
+	(file ? RECLAIM_WB_FILE : RECLAIM_WB_ANON) | \
+	(RECLAIM_WB_ASYNC) \
+	)
+
+#elif (LINUX_VERSION_CODE >= KERNEL_VERSION(4,5,0))
 
 #include <linux/mm_inline.h>
 
@@ -559,7 +568,20 @@ LTTNG_TRACEPOINT_EVENT_INSTANCE(mm_vmscan_lru_isolate_template, mm_vmscan_memcg_
 )
 #endif
 
-#if (LINUX_VERSION_CODE >= KERNEL_VERSION(4,5,0))
+#if (LINUX_VERSION_CODE >= KERNEL_VERSION(5,2,0))
+LTTNG_TRACEPOINT_EVENT(mm_vmscan_writepage,
+
+	TP_PROTO(struct page *page),
+
+	TP_ARGS(page),
+
+	TP_FIELDS(
+		ctf_integer_hex(struct page *, page, page)
+		ctf_integer(int, reclaim_flags, trace_reclaim_flags(
+				page_is_file_cache(page)))
+	)
+)
+#elif (LINUX_VERSION_CODE >= KERNEL_VERSION(4,5,0))
 LTTNG_TRACEPOINT_EVENT(mm_vmscan_writepage,
 
 	TP_PROTO(struct page *page),
@@ -586,7 +608,31 @@ LTTNG_TRACEPOINT_EVENT(mm_vmscan_writepage,
 )
 #endif
 
-#if (LINUX_VERSION_CODE >= KERNEL_VERSION(4,17,0))
+#if (LINUX_VERSION_CODE >= KERNEL_VERSION(5,2,0))
+LTTNG_TRACEPOINT_EVENT(mm_vmscan_lru_shrink_inactive,
+
+	TP_PROTO(int nid,
+		unsigned long nr_scanned, unsigned long nr_reclaimed,
+		struct reclaim_stat *stat, int priority, int file),
+
+	TP_ARGS(nid, nr_scanned, nr_reclaimed, stat, priority, file),
+
+	TP_FIELDS(
+		ctf_integer(int, nid, nid)
+		ctf_integer(unsigned long, nr_scanned, nr_scanned)
+		ctf_integer(unsigned long, nr_reclaimed, nr_reclaimed)
+		ctf_integer(unsigned long, nr_dirty, stat->nr_dirty)
+		ctf_integer(unsigned long, nr_writeback, stat->nr_writeback)
+		ctf_integer(unsigned long, nr_congested, stat->nr_congested)
+		ctf_integer(unsigned long, nr_immediate, stat->nr_immediate)
+		ctf_integer(unsigned long, nr_activate, stat->nr_activate)
+		ctf_integer(unsigned long, nr_ref_keep, stat->nr_ref_keep)
+		ctf_integer(unsigned long, nr_unmap_fail, stat->nr_unmap_fail)
+		ctf_integer(int, priority, priority)
+		ctf_integer(int, reclaim_flags, trace_reclaim_flags(file))
+	)
+)
+#elif (LINUX_VERSION_CODE >= KERNEL_VERSION(4,17,0))
 LTTNG_TRACEPOINT_EVENT(mm_vmscan_lru_shrink_inactive,
 
 	TP_PROTO(int nid,
-- 
2.17.1

[PATCH lttng-modules 4/5] fix: mm: move recent_rotated pages calculation to shrink_inactive_list() (v5.2)

[Michael Jeanson]

See upstream commit:

  commit 886cf1901db962cee5f8b82b9b260079a5e8a4eb
  Author: Kirill Tkhai <ktkhai@virtuozzo.com>
  Date:   Mon May 13 17:16:51 2019 -0700

    mm: move recent_rotated pages calculation to shrink_inactive_list()

    Patch series "mm: Generalize putback functions"]

    putback_inactive_pages() and move_active_pages_to_lru() are almost
    similar, so this patchset merges them ina single function.

    This patch (of 4):

    The patch moves the calculation from putback_inactive_pages() to
    shrink_inactive_list().  This makes putback_inactive_pages() looking more
    similar to move_active_pages_to_lru().

    To do that, we account activated pages in reclaim_stat::nr_activate.
    Since a page may change its LRU type from anon to file cache inside
    shrink_page_list() (see ClearPageSwapBacked()), we have to account pages
    for the both types.  So, nr_activate becomes an array.

    Previously we used nr_activate to account PGACTIVATE events, but now we
    account them into pgactivate variable (since they are about number of
    pages in general, not about sum of hpage_nr_pages).

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
---
 instrumentation/events/lttng-module/mm_vmscan.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/instrumentation/events/lttng-module/mm_vmscan.h b/instrumentation/events/lttng-module/mm_vmscan.h
index 417472c..3f9ffde 100644
--- a/instrumentation/events/lttng-module/mm_vmscan.h
+++ b/instrumentation/events/lttng-module/mm_vmscan.h
@@ -625,7 +625,8 @@ LTTNG_TRACEPOINT_EVENT(mm_vmscan_lru_shrink_inactive,
 		ctf_integer(unsigned long, nr_writeback, stat->nr_writeback)
 		ctf_integer(unsigned long, nr_congested, stat->nr_congested)
 		ctf_integer(unsigned long, nr_immediate, stat->nr_immediate)
-		ctf_integer(unsigned long, nr_activate, stat->nr_activate)
+		ctf_integer(unsigned long, nr_activate0, stat->nr_activate[0])
+		ctf_integer(unsigned long, nr_activate1, stat->nr_activate[1])
 		ctf_integer(unsigned long, nr_ref_keep, stat->nr_ref_keep)
 		ctf_integer(unsigned long, nr_unmap_fail, stat->nr_unmap_fail)
 		ctf_integer(int, priority, priority)
-- 
2.17.1

[PATCH lttng-modules 5/5] fix: random: only read from /dev/random after its pool has received 128 bits (v5.2)

[Michael Jeanson]

See upstream commit:

  commit eb9d1bf079bb438d1a066d72337092935fc770f6
  Author: Theodore Ts'o <tytso@mit.edu>
  Date:   Wed Feb 20 16:06:38 2019 -0500

    random: only read from /dev/random after its pool has received 128 bits

    Immediately after boot, we allow reads from /dev/random before its
    entropy pool has been fully initialized.  Fix this so that we don't
    allow this until the blocking pool has received 128 bits.

    We do this by repurposing the initialized flag in the entropy pool
    struct, and use the initialized flag in the blocking pool to indicate
    whether it is safe to pull from the blocking pool.

    To do this, we needed to rework when we decide to push entropy from the
    input pool to the blocking pool, since the initialized flag for the
    input pool was used for this purpose.  To simplify things, we no
    longer use the initialized flag for that purpose, nor do we use the
    entropy_total field any more.

Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
---
 instrumentation/events/lttng-module/random.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/instrumentation/events/lttng-module/random.h b/instrumentation/events/lttng-module/random.h
index f165600..80b2279 100644
--- a/instrumentation/events/lttng-module/random.h
+++ b/instrumentation/events/lttng-module/random.h
@@ -38,6 +38,24 @@ LTTNG_TRACEPOINT_EVENT_INSTANCE_MAP(random__mix_pool_bytes, mix_pool_bytes_noloc
 	TP_ARGS(pool_name, bytes, IP)
 )
 
+#if (LINUX_VERSION_CODE >= KERNEL_VERSION(5,2,0))
+LTTNG_TRACEPOINT_EVENT_MAP(credit_entropy_bits,
+
+	random_credit_entropy_bits,
+
+	TP_PROTO(const char *pool_name, int bits, int entropy_count,
+		 unsigned long IP),
+
+	TP_ARGS(pool_name, bits, entropy_count, IP),
+
+	TP_FIELDS(
+		ctf_string(pool_name, pool_name)
+		ctf_integer(int, bits, bits)
+		ctf_integer(int, entropy_count, entropy_count)
+		ctf_integer(unsigned long, IP, IP)
+	)
+)
+#else
 LTTNG_TRACEPOINT_EVENT_MAP(credit_entropy_bits,
 
 	random_credit_entropy_bits,
@@ -55,6 +73,7 @@ LTTNG_TRACEPOINT_EVENT_MAP(credit_entropy_bits,
 		ctf_integer(unsigned long, IP, IP)
 	)
 )
+#endif
 
 LTTNG_TRACEPOINT_EVENT_MAP(get_random_bytes,
 
-- 
2.17.1

Re: [PATCH lttng-modules 4/5] fix: mm: move recent_rotated pages calculation to shrink_inactive_list() (v5.2)

[Mathieu Desnoyers]

----- On May 21, 2019, at 4:33 PM, Michael Jeanson mjeanson@efficios.com wrote:

> See upstream commit:
> 
>  commit 886cf1901db962cee5f8b82b9b260079a5e8a4eb
>  Author: Kirill Tkhai <ktkhai@virtuozzo.com>
>  Date:   Mon May 13 17:16:51 2019 -0700
> 
>    mm: move recent_rotated pages calculation to shrink_inactive_list()
> 
>    Patch series "mm: Generalize putback functions"]
> 
>    putback_inactive_pages() and move_active_pages_to_lru() are almost
>    similar, so this patchset merges them ina single function.
> 
>    This patch (of 4):
> 
>    The patch moves the calculation from putback_inactive_pages() to
>    shrink_inactive_list().  This makes putback_inactive_pages() looking more
>    similar to move_active_pages_to_lru().
> 
>    To do that, we account activated pages in reclaim_stat::nr_activate.
>    Since a page may change its LRU type from anon to file cache inside
>    shrink_page_list() (see ClearPageSwapBacked()), we have to account pages
>    for the both types.  So, nr_activate becomes an array.
> 
>    Previously we used nr_activate to account PGACTIVATE events, but now we
>    account them into pgactivate variable (since they are about number of
>    pages in general, not about sum of hpage_nr_pages).
> 
> Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
> ---
> instrumentation/events/lttng-module/mm_vmscan.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/instrumentation/events/lttng-module/mm_vmscan.h
> b/instrumentation/events/lttng-module/mm_vmscan.h
> index 417472c..3f9ffde 100644
> --- a/instrumentation/events/lttng-module/mm_vmscan.h
> +++ b/instrumentation/events/lttng-module/mm_vmscan.h
> @@ -625,7 +625,8 @@ LTTNG_TRACEPOINT_EVENT(mm_vmscan_lru_shrink_inactive,
> 		ctf_integer(unsigned long, nr_writeback, stat->nr_writeback)
> 		ctf_integer(unsigned long, nr_congested, stat->nr_congested)
> 		ctf_integer(unsigned long, nr_immediate, stat->nr_immediate)
> -		ctf_integer(unsigned long, nr_activate, stat->nr_activate)
> +		ctf_integer(unsigned long, nr_activate0, stat->nr_activate[0])
> +		ctf_integer(unsigned long, nr_activate1, stat->nr_activate[1])

This patch is for lttng-modules, but I'm adding Kirill Tkhai (author of the Linux
kernel commit causing the need for this change in lttng-modules) and Steven Rostedt
in CC, because I feel they might want to join this discussion naming of
userspace-visible TRACE_EVENT() fields.

Based on the upstream Linux commit, it looks like only the TP_printk() has
sane names for event fields, but could really improve on the naming for the
binary version of those fields:

-       TP_printk("nid=%d nr_scanned=%ld nr_reclaimed=%ld nr_dirty=%ld nr_writeback=%ld nr_congested=%ld nr_immediate=%ld nr_activate=%ld nr_ref_keep=%ld nr_unmap_fail=%ld priority=%d flags=%s",
+       TP_printk("nid=%d nr_scanned=%ld nr_reclaimed=%ld nr_dirty=%ld nr_writeback=%ld nr_congested=%ld nr_immediate=%ld nr_activate_anon=%d nr_activate_file=%d nr_ref_keep=%ld nr_unmap_fail=%ld priority=%d flags=%s",
                __entry->nid,
                __entry->nr_scanned, __entry->nr_reclaimed,
                __entry->nr_dirty, __entry->nr_writeback,
                __entry->nr_congested, __entry->nr_immediate,
-               __entry->nr_activate, __entry->nr_ref_keep,
-               __entry->nr_unmap_fail, __entry->priority,
+               __entry->nr_activate0, __entry->nr_activate1,
+               __entry->nr_ref_keep, __entry->nr_unmap_fail,
+               __entry->priority,
                show_reclaim_flags(__entry->reclaim_flags))
 );

So I recommend we do the following in lttng-modules:

Rename the field nr_activate0 to nr_activate_anon,
Rename the field nr_activate1 to nr_activate_file.

So users can make something out of those tracepoints without digging
into the kernel source code.

Even if Steven and Kirill end up choosing to change the name of those
fields upstream in trace-event, it won't have any impact on lttng-modules.

It would make sense to change those newly introduced exposed names in the
upstream kernel as well though.

Thanks,

Mathieu


> 		ctf_integer(unsigned long, nr_ref_keep, stat->nr_ref_keep)
> 		ctf_integer(unsigned long, nr_unmap_fail, stat->nr_unmap_fail)
> 		ctf_integer(int, priority, priority)
> --
> 2.17.1

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH lttng-modules 4/5] fix: mm: move recent_rotated pages calculation to shrink_inactive_list() (v5.2)

[Mathieu Desnoyers]

----- On May 21, 2019, at 4:33 PM, Michael Jeanson mjeanson@efficios.com wrote:

> See upstream commit:
> 
>  commit 886cf1901db962cee5f8b82b9b260079a5e8a4eb
>  Author: Kirill Tkhai <ktkhai@virtuozzo.com>
>  Date:   Mon May 13 17:16:51 2019 -0700
> 
>    mm: move recent_rotated pages calculation to shrink_inactive_list()
> 
>    Patch series "mm: Generalize putback functions"]
> 
>    putback_inactive_pages() and move_active_pages_to_lru() are almost
>    similar, so this patchset merges them ina single function.
> 
>    This patch (of 4):
> 
>    The patch moves the calculation from putback_inactive_pages() to
>    shrink_inactive_list().  This makes putback_inactive_pages() looking more
>    similar to move_active_pages_to_lru().
> 
>    To do that, we account activated pages in reclaim_stat::nr_activate.
>    Since a page may change its LRU type from anon to file cache inside
>    shrink_page_list() (see ClearPageSwapBacked()), we have to account pages
>    for the both types.  So, nr_activate becomes an array.
> 
>    Previously we used nr_activate to account PGACTIVATE events, but now we
>    account them into pgactivate variable (since they are about number of
>    pages in general, not about sum of hpage_nr_pages).
> 
> Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
> ---
> instrumentation/events/lttng-module/mm_vmscan.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/instrumentation/events/lttng-module/mm_vmscan.h
> b/instrumentation/events/lttng-module/mm_vmscan.h
> index 417472c..3f9ffde 100644
> --- a/instrumentation/events/lttng-module/mm_vmscan.h
> +++ b/instrumentation/events/lttng-module/mm_vmscan.h
> @@ -625,7 +625,8 @@ LTTNG_TRACEPOINT_EVENT(mm_vmscan_lru_shrink_inactive,
> 		ctf_integer(unsigned long, nr_writeback, stat->nr_writeback)
> 		ctf_integer(unsigned long, nr_congested, stat->nr_congested)
> 		ctf_integer(unsigned long, nr_immediate, stat->nr_immediate)
> -		ctf_integer(unsigned long, nr_activate, stat->nr_activate)
> +		ctf_integer(unsigned long, nr_activate0, stat->nr_activate[0])
> +		ctf_integer(unsigned long, nr_activate1, stat->nr_activate[1])

This patch is for lttng-modules, but I'm adding Kirill Tkhai (author of the Linux
kernel commit causing the need for this change in lttng-modules) and Steven Rostedt
in CC, because I feel they might want to join this discussion naming of
userspace-visible TRACE_EVENT() fields.

Based on the upstream Linux commit, it looks like only the TP_printk() has
sane names for event fields, but could really improve on the naming for the
binary version of those fields:

-       TP_printk("nid=%d nr_scanned=%ld nr_reclaimed=%ld nr_dirty=%ld nr_writeback=%ld nr_congested=%ld nr_immediate=%ld nr_activate=%ld nr_ref_keep=%ld nr_unmap_fail=%ld priority=%d flags=%s",
+       TP_printk("nid=%d nr_scanned=%ld nr_reclaimed=%ld nr_dirty=%ld nr_writeback=%ld nr_congested=%ld nr_immediate=%ld nr_activate_anon=%d nr_activate_file=%d nr_ref_keep=%ld nr_unmap_fail=%ld priority=%d flags=%s",
                __entry->nid,
                __entry->nr_scanned, __entry->nr_reclaimed,
                __entry->nr_dirty, __entry->nr_writeback,
                __entry->nr_congested, __entry->nr_immediate,
-               __entry->nr_activate, __entry->nr_ref_keep,
-               __entry->nr_unmap_fail, __entry->priority,
+               __entry->nr_activate0, __entry->nr_activate1,
+               __entry->nr_ref_keep, __entry->nr_unmap_fail,
+               __entry->priority,
                show_reclaim_flags(__entry->reclaim_flags))
 );

So I recommend we do the following in lttng-modules:

Rename the field nr_activate0 to nr_activate_anon,
Rename the field nr_activate1 to nr_activate_file.

So users can make something out of those tracepoints without digging
into the kernel source code.

Even if Steven and Kirill end up choosing to change the name of those
fields upstream in trace-event, it won't have any impact on lttng-modules.

It would make sense to change those newly introduced exposed names in the
upstream kernel as well though.

Thanks,

Mathieu


> 		ctf_integer(unsigned long, nr_ref_keep, stat->nr_ref_keep)
> 		ctf_integer(unsigned long, nr_unmap_fail, stat->nr_unmap_fail)
> 		ctf_integer(int, priority, priority)
> --
> 2.17.1

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH lttng-modules 4/5] fix: mm: move recent_rotated pages calculation to shrink_inactive_list() (v5.2)

[Steven Rostedt]

On Tue, 21 May 2019 16:53:36 -0400 (EDT)
Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:

> So I recommend we do the following in lttng-modules:
> 
> Rename the field nr_activate0 to nr_activate_anon,
> Rename the field nr_activate1 to nr_activate_file.
> 
> So users can make something out of those tracepoints without digging
> into the kernel source code.
> 
> Even if Steven and Kirill end up choosing to change the name of those
> fields upstream in trace-event, it won't have any impact on lttng-modules.
> 
> It would make sense to change those newly introduced exposed names in the
> upstream kernel as well though.

I'm fine with whatever Kirill decides.

-- Steve

Re: [PATCH lttng-modules 4/5] fix: mm: move recent_rotated pages calculation to shrink_inactive_list() (v5.2)

[Kirill Tkhai]

On 21.05.2019 23:53, Mathieu Desnoyers wrote:
> ----- On May 21, 2019, at 4:33 PM, Michael Jeanson mjeanson@efficios.com wrote:
> 
>> See upstream commit:
>>
>>  commit 886cf1901db962cee5f8b82b9b260079a5e8a4eb
>>  Author: Kirill Tkhai <ktkhai@virtuozzo.com>
>>  Date:   Mon May 13 17:16:51 2019 -0700
>>
>>    mm: move recent_rotated pages calculation to shrink_inactive_list()
>>
>>    Patch series "mm: Generalize putback functions"]
>>
>>    putback_inactive_pages() and move_active_pages_to_lru() are almost
>>    similar, so this patchset merges them ina single function.
>>
>>    This patch (of 4):
>>
>>    The patch moves the calculation from putback_inactive_pages() to
>>    shrink_inactive_list().  This makes putback_inactive_pages() looking more
>>    similar to move_active_pages_to_lru().
>>
>>    To do that, we account activated pages in reclaim_stat::nr_activate.
>>    Since a page may change its LRU type from anon to file cache inside
>>    shrink_page_list() (see ClearPageSwapBacked()), we have to account pages
>>    for the both types.  So, nr_activate becomes an array.
>>
>>    Previously we used nr_activate to account PGACTIVATE events, but now we
>>    account them into pgactivate variable (since they are about number of
>>    pages in general, not about sum of hpage_nr_pages).
>>
>> Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
>> ---
>> instrumentation/events/lttng-module/mm_vmscan.h | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/instrumentation/events/lttng-module/mm_vmscan.h
>> b/instrumentation/events/lttng-module/mm_vmscan.h
>> index 417472c..3f9ffde 100644
>> --- a/instrumentation/events/lttng-module/mm_vmscan.h
>> +++ b/instrumentation/events/lttng-module/mm_vmscan.h
>> @@ -625,7 +625,8 @@ LTTNG_TRACEPOINT_EVENT(mm_vmscan_lru_shrink_inactive,
>> 		ctf_integer(unsigned long, nr_writeback, stat->nr_writeback)
>> 		ctf_integer(unsigned long, nr_congested, stat->nr_congested)
>> 		ctf_integer(unsigned long, nr_immediate, stat->nr_immediate)
>> -		ctf_integer(unsigned long, nr_activate, stat->nr_activate)
>> +		ctf_integer(unsigned long, nr_activate0, stat->nr_activate[0])
>> +		ctf_integer(unsigned long, nr_activate1, stat->nr_activate[1])
> 
> This patch is for lttng-modules, but I'm adding Kirill Tkhai (author of the Linux
> kernel commit causing the need for this change in lttng-modules) and Steven Rostedt
> in CC, because I feel they might want to join this discussion naming of
> userspace-visible TRACE_EVENT() fields.
> 
> Based on the upstream Linux commit, it looks like only the TP_printk() has
> sane names for event fields, but could really improve on the naming for the
> binary version of those fields:
> 
> -       TP_printk("nid=%d nr_scanned=%ld nr_reclaimed=%ld nr_dirty=%ld nr_writeback=%ld nr_congested=%ld nr_immediate=%ld nr_activate=%ld nr_ref_keep=%ld nr_unmap_fail=%ld priority=%d flags=%s",
> +       TP_printk("nid=%d nr_scanned=%ld nr_reclaimed=%ld nr_dirty=%ld nr_writeback=%ld nr_congested=%ld nr_immediate=%ld nr_activate_anon=%d nr_activate_file=%d nr_ref_keep=%ld nr_unmap_fail=%ld priority=%d flags=%s",
>                 __entry->nid,
>                 __entry->nr_scanned, __entry->nr_reclaimed,
>                 __entry->nr_dirty, __entry->nr_writeback,
>                 __entry->nr_congested, __entry->nr_immediate,
> -               __entry->nr_activate, __entry->nr_ref_keep,
> -               __entry->nr_unmap_fail, __entry->priority,
> +               __entry->nr_activate0, __entry->nr_activate1,
> +               __entry->nr_ref_keep, __entry->nr_unmap_fail,
> +               __entry->priority,
>                 show_reclaim_flags(__entry->reclaim_flags))
>  );
> 
> So I recommend we do the following in lttng-modules:
> 
> Rename the field nr_activate0 to nr_activate_anon,
> Rename the field nr_activate1 to nr_activate_file.
> 
> So users can make something out of those tracepoints without digging
> into the kernel source code.
> 
> Even if Steven and Kirill end up choosing to change the name of those
> fields upstream in trace-event, it won't have any impact on lttng-modules.
> 
> It would make sense to change those newly introduced exposed names in the
> upstream kernel as well though.

Yeah, since these are exported to userspace, we should better rename them in the way
you suggested.

Thanks,
Kirill

Re: [PATCH lttng-modules 1/5] fix: timer/trace: Improve timer tracing (v5.2)

[Mathieu Desnoyers]

All merged into lttng-modules master, 2.11, 2.10, 2.9 branches.

I updated the field names for patch 4/5 as discussed.

Thanks,

Mathieu

----- On May 21, 2019, at 4:33 PM, Michael Jeanson mjeanson@efficios.com wrote:

> See upstream commit:
> 
>  commit f28d3d5346e97e60c81f933ac89ccf015430e5cf
>  Author: Anna-Maria Gleixner <anna-maria@linutronix.de>
>  Date:   Thu Mar 21 13:09:21 2019 +0100
> 
>    timer/trace: Improve timer tracing
> 
>    Timers are added to the timer wheel off by one. This is required in
>    case a timer is queued directly before incrementing jiffies to prevent
>    early timer expiry.
> 
>    When reading a timer trace and relying only on the expiry time of the timer
>    in the timer_start trace point and on the now in the timer_expiry_entry
>    trace point, it seems that the timer fires late. With the current
>    timer_expiry_entry trace point information only now=jiffies is printed but
>    not the value of base->clk. This makes it impossible to draw a conclusion
>    to the index of base->clk and makes it impossible to examine timer problems
>    without additional trace points.
> 
>    Therefore add the base->clk value to the timer_expire_entry trace
>    point, to be able to calculate the index the timer base is located at
>    during collecting expired timers.
> 
> Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
> ---
> instrumentation/events/lttng-module/timer.h | 22 +++++++++++++++++++++
> 1 file changed, 22 insertions(+)
> 
> diff --git a/instrumentation/events/lttng-module/timer.h
> b/instrumentation/events/lttng-module/timer.h
> index 8703ea0..997084f 100644
> --- a/instrumentation/events/lttng-module/timer.h
> +++ b/instrumentation/events/lttng-module/timer.h
> @@ -89,6 +89,27 @@ LTTNG_TRACEPOINT_EVENT(timer_start,
> )
> #endif /* #else #if (LINUX_VERSION_CODE >= KERNEL_VERSION(4,2,0)) */
> 
> +#if (LINUX_VERSION_CODE >= KERNEL_VERSION(5,2,0))
> +/**
> + * timer_expire_entry - called immediately before the timer callback
> + * @timer:	pointer to struct timer_list
> + *
> + * Allows to determine the timer latency.
> + */
> +LTTNG_TRACEPOINT_EVENT(timer_expire_entry,
> +
> +	TP_PROTO(struct timer_list *timer, unsigned long baseclk),
> +
> +	TP_ARGS(timer, baseclk),
> +
> +	TP_FIELDS(
> +		ctf_integer_hex(void *, timer, timer)
> +		ctf_integer(unsigned long, now, jiffies)
> +		ctf_integer_hex(void *, function, timer->function)
> +		ctf_integer(unsigned long, baseclk, baseclk)
> +	)
> +)
> +#else
> /**
>  * timer_expire_entry - called immediately before the timer callback
>  * @timer:	pointer to struct timer_list
> @@ -107,6 +128,7 @@ LTTNG_TRACEPOINT_EVENT(timer_expire_entry,
> 		ctf_integer_hex(void *, function, timer->function)
> 	)
> )
> +#endif
> 
> /**
>  * timer_expire_exit - called immediately after the timer callback returns
> --
> 2.17.1

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com