[PATCH] mm/zsmalloc: add trace events for zs_compact

[PATCH] mm/zsmalloc: add trace events for zs_compact

[Ganesh Mahendran]

Currently zsmalloc is widely used in android device.
Sometimes, we want to see how frequently zs_compact is
triggered or how may pages freed by zs_compact(), or which
zsmalloc pool is compacted.

Most of the time, user can get the brief information from
trace_mm_shrink_slab_[start | end], but in some senario,
they do not use zsmalloc shrinker, but trigger compaction manually.
So add some trace events in zs_compact is convenient. Also we
can add some zsmalloc specific information(pool name, total compact
pages, etc) in zsmalloc trace.

This patch add two trace events for zs_compact(), below the trace log:
-----------------------------
root@land:/ # cat /d/tracing/trace
         kswapd0-125   [007] ...1   174.176979: zsmalloc_compact_start: pool zram0
         kswapd0-125   [007] ...1   174.181967: zsmalloc_compact_end: pool zram0: 608 pages compacted(total 1794)
         kswapd0-125   [000] ...1   184.134475: zsmalloc_compact_start: pool zram0
         kswapd0-125   [000] ...1   184.135010: zsmalloc_compact_end: pool zram0: 62 pages compacted(total 1856)
         kswapd0-125   [003] ...1   226.927221: zsmalloc_compact_start: pool zram0
         kswapd0-125   [003] ...1   226.928575: zsmalloc_compact_end: pool zram0: 250 pages compacted(total 2106)
-----------------------------

Signed-off-by: Ganesh Mahendran <opensource.ganesh@gmail.com>
---
 include/trace/events/zsmalloc.h | 56 +++++++++++++++++++++++++++++++++++++++++
 mm/zsmalloc.c                   | 10 ++++++++
 2 files changed, 66 insertions(+)
 create mode 100644 include/trace/events/zsmalloc.h

diff --git a/include/trace/events/zsmalloc.h b/include/trace/events/zsmalloc.h
new file mode 100644
index 0000000..3b6f14e
--- /dev/null
+++ b/include/trace/events/zsmalloc.h
@@ -0,0 +1,56 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM zsmalloc
+
+#if !defined(_TRACE_ZSMALLOC_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_ZSMALLOC_H
+
+#include <linux/types.h>
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(zsmalloc_compact_start,
+
+	TP_PROTO(const char *pool_name),
+
+	TP_ARGS(pool_name),
+
+	TP_STRUCT__entry(
+		__field(const char *, pool_name)
+	),
+
+	TP_fast_assign(
+		__entry->pool_name = pool_name;
+	),
+
+	TP_printk("pool %s",
+		  __entry->pool_name)
+);
+
+TRACE_EVENT(zsmalloc_compact_end,
+
+	TP_PROTO(const char *pool_name, unsigned long pages_compacted,
+			unsigned long pages_total_compacted),
+
+	TP_ARGS(pool_name, pages_compacted, pages_total_compacted),
+
+	TP_STRUCT__entry(
+		__field(const char *, pool_name)
+		__field(unsigned long, pages_compacted)
+		__field(unsigned long, pages_total_compacted)
+	),
+
+	TP_fast_assign(
+		__entry->pool_name = pool_name;
+		__entry->pages_compacted = pages_compacted;
+		__entry->pages_total_compacted = pages_total_compacted;
+	),
+
+	TP_printk("pool %s: %ld pages compacted(total %ld)",
+		  __entry->pool_name,
+		  __entry->pages_compacted,
+		  __entry->pages_total_compacted)
+);
+
+#endif /* _TRACE_ZSMALLOC_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 213d0e1..441b9f7 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -30,6 +30,8 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#define CREATE_TRACE_POINTS
+
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/sched.h>
@@ -52,6 +54,7 @@
 #include <linux/mount.h>
 #include <linux/compaction.h>
 #include <linux/pagemap.h>
+#include <trace/events/zsmalloc.h>
 
 #define ZSPAGE_MAGIC	0x58
 
@@ -2330,6 +2333,9 @@ unsigned long zs_compact(struct zs_pool *pool)
 {
 	int i;
 	struct size_class *class;
+	unsigned long pages_compacted_before = pool->stats.pages_compacted;
+
+	trace_zsmalloc_compact_start(pool->name);
 
 	for (i = zs_size_classes - 1; i >= 0; i--) {
 		class = pool->size_class[i];
@@ -2340,6 +2346,10 @@ unsigned long zs_compact(struct zs_pool *pool)
 		__zs_compact(pool, class);
 	}
 
+	trace_zsmalloc_compact_end(pool->name,
+		pool->stats.pages_compacted - pages_compacted_before,
+		pool->stats.pages_compacted);
+
 	return pool->stats.pages_compacted;
 }
 EXPORT_SYMBOL_GPL(zs_compact);
-- 
1.9.1

Re: [PATCH] mm/zsmalloc: add trace events for zs_compact

[Minchan Kim]

Hello Ganesh,

On Tue, Jun 07, 2016 at 04:56:44PM +0800, Ganesh Mahendran wrote:
> Currently zsmalloc is widely used in android device.
> Sometimes, we want to see how frequently zs_compact is
> triggered or how may pages freed by zs_compact(), or which
> zsmalloc pool is compacted.
> 
> Most of the time, user can get the brief information from
> trace_mm_shrink_slab_[start | end], but in some senario,
> they do not use zsmalloc shrinker, but trigger compaction manually.
> So add some trace events in zs_compact is convenient. Also we
> can add some zsmalloc specific information(pool name, total compact
> pages, etc) in zsmalloc trace.

Sorry, I cannot understand what's the problem now and what you want to
solve. Could you elaborate it a bit?

Thanks.

> 
> This patch add two trace events for zs_compact(), below the trace log:
> -----------------------------
> root@land:/ # cat /d/tracing/trace
>          kswapd0-125   [007] ...1   174.176979: zsmalloc_compact_start: pool zram0
>          kswapd0-125   [007] ...1   174.181967: zsmalloc_compact_end: pool zram0: 608 pages compacted(total 1794)
>          kswapd0-125   [000] ...1   184.134475: zsmalloc_compact_start: pool zram0
>          kswapd0-125   [000] ...1   184.135010: zsmalloc_compact_end: pool zram0: 62 pages compacted(total 1856)
>          kswapd0-125   [003] ...1   226.927221: zsmalloc_compact_start: pool zram0
>          kswapd0-125   [003] ...1   226.928575: zsmalloc_compact_end: pool zram0: 250 pages compacted(total 2106)
> -----------------------------
> 
> Signed-off-by: Ganesh Mahendran <opensource.ganesh@gmail.com>
> ---
>  include/trace/events/zsmalloc.h | 56 +++++++++++++++++++++++++++++++++++++++++
>  mm/zsmalloc.c                   | 10 ++++++++
>  2 files changed, 66 insertions(+)
>  create mode 100644 include/trace/events/zsmalloc.h
> 
> diff --git a/include/trace/events/zsmalloc.h b/include/trace/events/zsmalloc.h
> new file mode 100644
> index 0000000..3b6f14e
> --- /dev/null
> +++ b/include/trace/events/zsmalloc.h
> @@ -0,0 +1,56 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM zsmalloc
> +
> +#if !defined(_TRACE_ZSMALLOC_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_ZSMALLOC_H
> +
> +#include <linux/types.h>
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT(zsmalloc_compact_start,
> +
> +	TP_PROTO(const char *pool_name),
> +
> +	TP_ARGS(pool_name),
> +
> +	TP_STRUCT__entry(
> +		__field(const char *, pool_name)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->pool_name = pool_name;
> +	),
> +
> +	TP_printk("pool %s",
> +		  __entry->pool_name)
> +);
> +
> +TRACE_EVENT(zsmalloc_compact_end,
> +
> +	TP_PROTO(const char *pool_name, unsigned long pages_compacted,
> +			unsigned long pages_total_compacted),
> +
> +	TP_ARGS(pool_name, pages_compacted, pages_total_compacted),
> +
> +	TP_STRUCT__entry(
> +		__field(const char *, pool_name)
> +		__field(unsigned long, pages_compacted)
> +		__field(unsigned long, pages_total_compacted)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->pool_name = pool_name;
> +		__entry->pages_compacted = pages_compacted;
> +		__entry->pages_total_compacted = pages_total_compacted;
> +	),
> +
> +	TP_printk("pool %s: %ld pages compacted(total %ld)",
> +		  __entry->pool_name,
> +		  __entry->pages_compacted,
> +		  __entry->pages_total_compacted)
> +);
> +
> +#endif /* _TRACE_ZSMALLOC_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index 213d0e1..441b9f7 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -30,6 +30,8 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> +#define CREATE_TRACE_POINTS
> +
>  #include <linux/module.h>
>  #include <linux/kernel.h>
>  #include <linux/sched.h>
> @@ -52,6 +54,7 @@
>  #include <linux/mount.h>
>  #include <linux/compaction.h>
>  #include <linux/pagemap.h>
> +#include <trace/events/zsmalloc.h>
>  
>  #define ZSPAGE_MAGIC	0x58
>  
> @@ -2330,6 +2333,9 @@ unsigned long zs_compact(struct zs_pool *pool)
>  {
>  	int i;
>  	struct size_class *class;
> +	unsigned long pages_compacted_before = pool->stats.pages_compacted;
> +
> +	trace_zsmalloc_compact_start(pool->name);
>  
>  	for (i = zs_size_classes - 1; i >= 0; i--) {
>  		class = pool->size_class[i];
> @@ -2340,6 +2346,10 @@ unsigned long zs_compact(struct zs_pool *pool)
>  		__zs_compact(pool, class);
>  	}
>  
> +	trace_zsmalloc_compact_end(pool->name,
> +		pool->stats.pages_compacted - pages_compacted_before,
> +		pool->stats.pages_compacted);
> +
>  	return pool->stats.pages_compacted;
>  }
>  EXPORT_SYMBOL_GPL(zs_compact);
> -- 
> 1.9.1
> 

Re: [PATCH] mm/zsmalloc: add trace events for zs_compact

[Ganesh Mahendran]

Hi, Minchan:

2016-06-08 8:16 GMT+08:00 Minchan Kim <minchan@kernel.org>:
> Hello Ganesh,
>
> On Tue, Jun 07, 2016 at 04:56:44PM +0800, Ganesh Mahendran wrote:
>> Currently zsmalloc is widely used in android device.
>> Sometimes, we want to see how frequently zs_compact is
>> triggered or how may pages freed by zs_compact(), or which
>> zsmalloc pool is compacted.
>>
>> Most of the time, user can get the brief information from
>> trace_mm_shrink_slab_[start | end], but in some senario,
>> they do not use zsmalloc shrinker, but trigger compaction manually.
>> So add some trace events in zs_compact is convenient. Also we
>> can add some zsmalloc specific information(pool name, total compact
>> pages, etc) in zsmalloc trace.
>
> Sorry, I cannot understand what's the problem now and what you want to
> solve. Could you elaborate it a bit?
>
> Thanks.

We have backported the zs_compact() to our product(kernel 3.18).
It is usefull for a longtime running device.
But there is not a convenient way to get the detailed information
of zs_comapct() which is usefull for  performance optimization.
Information about how much time zs_compact used, which pool is
compacted, how many page freed, etc.
With these information, we will know what is going on in zs_comapct.
And draw the relation between free mem and zs_comapct.

>
>>
>> This patch add two trace events for zs_compact(), below the trace log:
>> -----------------------------
>> root@land:/ # cat /d/tracing/trace
>>          kswapd0-125   [007] ...1   174.176979: zsmalloc_compact_start: pool zram0
>>          kswapd0-125   [007] ...1   174.181967: zsmalloc_compact_end: pool zram0: 608 pages compacted(total 1794)
>>          kswapd0-125   [000] ...1   184.134475: zsmalloc_compact_start: pool zram0
>>          kswapd0-125   [000] ...1   184.135010: zsmalloc_compact_end: pool zram0: 62 pages compacted(total 1856)
>>          kswapd0-125   [003] ...1   226.927221: zsmalloc_compact_start: pool zram0
>>          kswapd0-125   [003] ...1   226.928575: zsmalloc_compact_end: pool zram0: 250 pages compacted(total 2106)
>> -----------------------------
>>
>> Signed-off-by: Ganesh Mahendran <opensource.ganesh@gmail.com>
>> ---
>>  include/trace/events/zsmalloc.h | 56 +++++++++++++++++++++++++++++++++++++++++
>>  mm/zsmalloc.c                   | 10 ++++++++
>>  2 files changed, 66 insertions(+)
>>  create mode 100644 include/trace/events/zsmalloc.h
>>
>> diff --git a/include/trace/events/zsmalloc.h b/include/trace/events/zsmalloc.h
>> new file mode 100644
>> index 0000000..3b6f14e
>> --- /dev/null
>> +++ b/include/trace/events/zsmalloc.h
>> @@ -0,0 +1,56 @@
>> +#undef TRACE_SYSTEM
>> +#define TRACE_SYSTEM zsmalloc
>> +
>> +#if !defined(_TRACE_ZSMALLOC_H) || defined(TRACE_HEADER_MULTI_READ)
>> +#define _TRACE_ZSMALLOC_H
>> +
>> +#include <linux/types.h>
>> +#include <linux/tracepoint.h>
>> +
>> +TRACE_EVENT(zsmalloc_compact_start,
>> +
>> +     TP_PROTO(const char *pool_name),
>> +
>> +     TP_ARGS(pool_name),
>> +
>> +     TP_STRUCT__entry(
>> +             __field(const char *, pool_name)
>> +     ),
>> +
>> +     TP_fast_assign(
>> +             __entry->pool_name = pool_name;
>> +     ),
>> +
>> +     TP_printk("pool %s",
>> +               __entry->pool_name)
>> +);
>> +
>> +TRACE_EVENT(zsmalloc_compact_end,
>> +
>> +     TP_PROTO(const char *pool_name, unsigned long pages_compacted,
>> +                     unsigned long pages_total_compacted),
>> +
>> +     TP_ARGS(pool_name, pages_compacted, pages_total_compacted),
>> +
>> +     TP_STRUCT__entry(
>> +             __field(const char *, pool_name)
>> +             __field(unsigned long, pages_compacted)
>> +             __field(unsigned long, pages_total_compacted)
>> +     ),
>> +
>> +     TP_fast_assign(
>> +             __entry->pool_name = pool_name;
>> +             __entry->pages_compacted = pages_compacted;
>> +             __entry->pages_total_compacted = pages_total_compacted;
>> +     ),
>> +
>> +     TP_printk("pool %s: %ld pages compacted(total %ld)",
>> +               __entry->pool_name,
>> +               __entry->pages_compacted,
>> +               __entry->pages_total_compacted)
>> +);
>> +
>> +#endif /* _TRACE_ZSMALLOC_H */
>> +
>> +/* This part must be outside protection */
>> +#include <trace/define_trace.h>
>> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
>> index 213d0e1..441b9f7 100644
>> --- a/mm/zsmalloc.c
>> +++ b/mm/zsmalloc.c
>> @@ -30,6 +30,8 @@
>>
>>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>>
>> +#define CREATE_TRACE_POINTS
>> +
>>  #include <linux/module.h>
>>  #include <linux/kernel.h>
>>  #include <linux/sched.h>
>> @@ -52,6 +54,7 @@
>>  #include <linux/mount.h>
>>  #include <linux/compaction.h>
>>  #include <linux/pagemap.h>
>> +#include <trace/events/zsmalloc.h>
>>
>>  #define ZSPAGE_MAGIC 0x58
>>
>> @@ -2330,6 +2333,9 @@ unsigned long zs_compact(struct zs_pool *pool)
>>  {
>>       int i;
>>       struct size_class *class;
>> +     unsigned long pages_compacted_before = pool->stats.pages_compacted;
>> +
>> +     trace_zsmalloc_compact_start(pool->name);
>>
>>       for (i = zs_size_classes - 1; i >= 0; i--) {
>>               class = pool->size_class[i];
>> @@ -2340,6 +2346,10 @@ unsigned long zs_compact(struct zs_pool *pool)
>>               __zs_compact(pool, class);
>>       }
>>
>> +     trace_zsmalloc_compact_end(pool->name,
>> +             pool->stats.pages_compacted - pages_compacted_before,
>> +             pool->stats.pages_compacted);
>> +
>>       return pool->stats.pages_compacted;
>>  }
>>  EXPORT_SYMBOL_GPL(zs_compact);
>> --
>> 1.9.1
>>

Re: [PATCH] mm/zsmalloc: add trace events for zs_compact

[Minchan Kim]

On Wed, Jun 08, 2016 at 09:48:30AM +0800, Ganesh Mahendran wrote:
> Hi, Minchan:
> 
> 2016-06-08 8:16 GMT+08:00 Minchan Kim <minchan@kernel.org>:
> > Hello Ganesh,
> >
> > On Tue, Jun 07, 2016 at 04:56:44PM +0800, Ganesh Mahendran wrote:
> >> Currently zsmalloc is widely used in android device.
> >> Sometimes, we want to see how frequently zs_compact is
> >> triggered or how may pages freed by zs_compact(), or which
> >> zsmalloc pool is compacted.
> >>
> >> Most of the time, user can get the brief information from
> >> trace_mm_shrink_slab_[start | end], but in some senario,
> >> they do not use zsmalloc shrinker, but trigger compaction manually.
> >> So add some trace events in zs_compact is convenient. Also we
> >> can add some zsmalloc specific information(pool name, total compact
> >> pages, etc) in zsmalloc trace.
> >
> > Sorry, I cannot understand what's the problem now and what you want to
> > solve. Could you elaborate it a bit?
> >
> > Thanks.
> 
> We have backported the zs_compact() to our product(kernel 3.18).
> It is usefull for a longtime running device.
> But there is not a convenient way to get the detailed information
> of zs_comapct() which is usefull for  performance optimization.
> Information about how much time zs_compact used, which pool is
> compacted, how many page freed, etc.

You can know how many pages are freed by object compaction via mm_stat
each /sys/block/zram-id/mm_stat. And you can use function_graph to know
how much time zs_compact used.


> With these information, we will know what is going on in zs_comapct.
> And draw the relation between free mem and zs_comapct.
> 
> >
> >>
> >> This patch add two trace events for zs_compact(), below the trace log:
> >> -----------------------------
> >> root@land:/ # cat /d/tracing/trace
> >>          kswapd0-125   [007] ...1   174.176979: zsmalloc_compact_start: pool zram0
> >>          kswapd0-125   [007] ...1   174.181967: zsmalloc_compact_end: pool zram0: 608 pages compacted(total 1794)
> >>          kswapd0-125   [000] ...1   184.134475: zsmalloc_compact_start: pool zram0
> >>          kswapd0-125   [000] ...1   184.135010: zsmalloc_compact_end: pool zram0: 62 pages compacted(total 1856)
> >>          kswapd0-125   [003] ...1   226.927221: zsmalloc_compact_start: pool zram0
> >>          kswapd0-125   [003] ...1   226.928575: zsmalloc_compact_end: pool zram0: 250 pages compacted(total 2106)
> >> -----------------------------
> >>
> >> Signed-off-by: Ganesh Mahendran <opensource.ganesh@gmail.com>
> >> ---
> >>  include/trace/events/zsmalloc.h | 56 +++++++++++++++++++++++++++++++++++++++++
> >>  mm/zsmalloc.c                   | 10 ++++++++
> >>  2 files changed, 66 insertions(+)
> >>  create mode 100644 include/trace/events/zsmalloc.h
> >>
> >> diff --git a/include/trace/events/zsmalloc.h b/include/trace/events/zsmalloc.h
> >> new file mode 100644
> >> index 0000000..3b6f14e
> >> --- /dev/null
> >> +++ b/include/trace/events/zsmalloc.h
> >> @@ -0,0 +1,56 @@
> >> +#undef TRACE_SYSTEM
> >> +#define TRACE_SYSTEM zsmalloc
> >> +
> >> +#if !defined(_TRACE_ZSMALLOC_H) || defined(TRACE_HEADER_MULTI_READ)
> >> +#define _TRACE_ZSMALLOC_H
> >> +
> >> +#include <linux/types.h>
> >> +#include <linux/tracepoint.h>
> >> +
> >> +TRACE_EVENT(zsmalloc_compact_start,
> >> +
> >> +     TP_PROTO(const char *pool_name),
> >> +
> >> +     TP_ARGS(pool_name),
> >> +
> >> +     TP_STRUCT__entry(
> >> +             __field(const char *, pool_name)
> >> +     ),
> >> +
> >> +     TP_fast_assign(
> >> +             __entry->pool_name = pool_name;
> >> +     ),
> >> +
> >> +     TP_printk("pool %s",
> >> +               __entry->pool_name)
> >> +);
> >> +
> >> +TRACE_EVENT(zsmalloc_compact_end,
> >> +
> >> +     TP_PROTO(const char *pool_name, unsigned long pages_compacted,
> >> +                     unsigned long pages_total_compacted),
> >> +
> >> +     TP_ARGS(pool_name, pages_compacted, pages_total_compacted),
> >> +
> >> +     TP_STRUCT__entry(
> >> +             __field(const char *, pool_name)
> >> +             __field(unsigned long, pages_compacted)
> >> +             __field(unsigned long, pages_total_compacted)
> >> +     ),
> >> +
> >> +     TP_fast_assign(
> >> +             __entry->pool_name = pool_name;
> >> +             __entry->pages_compacted = pages_compacted;
> >> +             __entry->pages_total_compacted = pages_total_compacted;
> >> +     ),
> >> +
> >> +     TP_printk("pool %s: %ld pages compacted(total %ld)",
> >> +               __entry->pool_name,
> >> +               __entry->pages_compacted,
> >> +               __entry->pages_total_compacted)
> >> +);
> >> +
> >> +#endif /* _TRACE_ZSMALLOC_H */
> >> +
> >> +/* This part must be outside protection */
> >> +#include <trace/define_trace.h>
> >> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> >> index 213d0e1..441b9f7 100644
> >> --- a/mm/zsmalloc.c
> >> +++ b/mm/zsmalloc.c
> >> @@ -30,6 +30,8 @@
> >>
> >>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> >>
> >> +#define CREATE_TRACE_POINTS
> >> +
> >>  #include <linux/module.h>
> >>  #include <linux/kernel.h>
> >>  #include <linux/sched.h>
> >> @@ -52,6 +54,7 @@
> >>  #include <linux/mount.h>
> >>  #include <linux/compaction.h>
> >>  #include <linux/pagemap.h>
> >> +#include <trace/events/zsmalloc.h>
> >>
> >>  #define ZSPAGE_MAGIC 0x58
> >>
> >> @@ -2330,6 +2333,9 @@ unsigned long zs_compact(struct zs_pool *pool)
> >>  {
> >>       int i;
> >>       struct size_class *class;
> >> +     unsigned long pages_compacted_before = pool->stats.pages_compacted;
> >> +
> >> +     trace_zsmalloc_compact_start(pool->name);
> >>
> >>       for (i = zs_size_classes - 1; i >= 0; i--) {
> >>               class = pool->size_class[i];
> >> @@ -2340,6 +2346,10 @@ unsigned long zs_compact(struct zs_pool *pool)
> >>               __zs_compact(pool, class);
> >>       }
> >>
> >> +     trace_zsmalloc_compact_end(pool->name,
> >> +             pool->stats.pages_compacted - pages_compacted_before,
> >> +             pool->stats.pages_compacted);
> >> +
> >>       return pool->stats.pages_compacted;
> >>  }
> >>  EXPORT_SYMBOL_GPL(zs_compact);
> >> --
> >> 1.9.1
> >>

Re: [PATCH] mm/zsmalloc: add trace events for zs_compact

[Ganesh Mahendran]

Hi, Minchan:

2016-06-08 13:13 GMT+08:00 Minchan Kim <minchan@kernel.org>:
> On Wed, Jun 08, 2016 at 09:48:30AM +0800, Ganesh Mahendran wrote:
>> Hi, Minchan:
>>
>> 2016-06-08 8:16 GMT+08:00 Minchan Kim <minchan@kernel.org>:
>> > Hello Ganesh,
>> >
>> > On Tue, Jun 07, 2016 at 04:56:44PM +0800, Ganesh Mahendran wrote:
>> >> Currently zsmalloc is widely used in android device.
>> >> Sometimes, we want to see how frequently zs_compact is
>> >> triggered or how may pages freed by zs_compact(), or which
>> >> zsmalloc pool is compacted.
>> >>
>> >> Most of the time, user can get the brief information from
>> >> trace_mm_shrink_slab_[start | end], but in some senario,
>> >> they do not use zsmalloc shrinker, but trigger compaction manually.
>> >> So add some trace events in zs_compact is convenient. Also we
>> >> can add some zsmalloc specific information(pool name, total compact
>> >> pages, etc) in zsmalloc trace.
>> >
>> > Sorry, I cannot understand what's the problem now and what you want to
>> > solve. Could you elaborate it a bit?
>> >
>> > Thanks.
>>
>> We have backported the zs_compact() to our product(kernel 3.18).
>> It is usefull for a longtime running device.
>> But there is not a convenient way to get the detailed information
>> of zs_comapct() which is usefull for  performance optimization.
>> Information about how much time zs_compact used, which pool is
>> compacted, how many page freed, etc.
>
> You can know how many pages are freed by object compaction via mm_stat
> each /sys/block/zram-id/mm_stat. And you can use function_graph to know
> how much time zs_compact used.

zsmalloc is not only used by zram, but also zswap. Maybe
others in the future.

I tried to use function_graph. It seems there are too much log
printed:
------
root@leo-test:/sys/kernel/debug/tracing# cat trace
# tracer: function_graph
#
# CPU  DURATION                  FUNCTION CALLS
# |     |   |                     |   |   |   |
 2)               |  zs_compact [zsmalloc]() {
 2)               |  /* zsmalloc_compact_start: pool zram0 */
 2)   0.889 us    |    _raw_spin_lock();
 2)   0.896 us    |    isolate_zspage [zsmalloc]();
 2)   0.938 us    |    _raw_spin_lock();
 2)   0.875 us    |    isolate_zspage [zsmalloc]();
 2)   0.942 us    |    _raw_spin_lock();
 2)   0.962 us    |    isolate_zspage [zsmalloc]();
...
 2)   0.879 us    |      insert_zspage [zsmalloc]();
 2)   4.520 us    |    }
 2)   0.975 us    |    _raw_spin_lock();
 2)   0.890 us    |    isolate_zspage [zsmalloc]();
 2)   0.882 us    |    _raw_spin_lock();
 2)   0.894 us    |    isolate_zspage [zsmalloc]();
 2)               |  /* zsmalloc_compact_end: pool zram0: 0 pages
compacted(total 0) */
 2) # 1351.241 us |  }
------
=> 1351.241 us used

And it seems the overhead of function_graph is bigger than trace event.

bash-3682  [002] ....  1439.180646: zsmalloc_compact_start: pool zram0
bash-3682  [002] ....  1439.180659: zsmalloc_compact_end: pool zram0:
0 pages compacted(total 0)
=> 13 us > 1351.241 us

Thanks.

>
>
>> With these information, we will know what is going on in zs_comapct.
>> And draw the relation between free mem and zs_comapct.
>>
>> >
>> >>
>> >> This patch add two trace events for zs_compact(), below the trace log:
>> >> -----------------------------
>> >> root@land:/ # cat /d/tracing/trace
>> >>          kswapd0-125   [007] ...1   174.176979: zsmalloc_compact_start: pool zram0
>> >>          kswapd0-125   [007] ...1   174.181967: zsmalloc_compact_end: pool zram0: 608 pages compacted(total 1794)
>> >>          kswapd0-125   [000] ...1   184.134475: zsmalloc_compact_start: pool zram0
>> >>          kswapd0-125   [000] ...1   184.135010: zsmalloc_compact_end: pool zram0: 62 pages compacted(total 1856)
>> >>          kswapd0-125   [003] ...1   226.927221: zsmalloc_compact_start: pool zram0
>> >>          kswapd0-125   [003] ...1   226.928575: zsmalloc_compact_end: pool zram0: 250 pages compacted(total 2106)
>> >> -----------------------------
>> >>
>> >> Signed-off-by: Ganesh Mahendran <opensource.ganesh@gmail.com>
>> >> ---
>> >>  include/trace/events/zsmalloc.h | 56 +++++++++++++++++++++++++++++++++++++++++
>> >>  mm/zsmalloc.c                   | 10 ++++++++
>> >>  2 files changed, 66 insertions(+)
>> >>  create mode 100644 include/trace/events/zsmalloc.h
>> >>
>> >> diff --git a/include/trace/events/zsmalloc.h b/include/trace/events/zsmalloc.h
>> >> new file mode 100644
>> >> index 0000000..3b6f14e
>> >> --- /dev/null
>> >> +++ b/include/trace/events/zsmalloc.h
>> >> @@ -0,0 +1,56 @@
>> >> +#undef TRACE_SYSTEM
>> >> +#define TRACE_SYSTEM zsmalloc
>> >> +
>> >> +#if !defined(_TRACE_ZSMALLOC_H) || defined(TRACE_HEADER_MULTI_READ)
>> >> +#define _TRACE_ZSMALLOC_H
>> >> +
>> >> +#include <linux/types.h>
>> >> +#include <linux/tracepoint.h>
>> >> +
>> >> +TRACE_EVENT(zsmalloc_compact_start,
>> >> +
>> >> +     TP_PROTO(const char *pool_name),
>> >> +
>> >> +     TP_ARGS(pool_name),
>> >> +
>> >> +     TP_STRUCT__entry(
>> >> +             __field(const char *, pool_name)
>> >> +     ),
>> >> +
>> >> +     TP_fast_assign(
>> >> +             __entry->pool_name = pool_name;
>> >> +     ),
>> >> +
>> >> +     TP_printk("pool %s",
>> >> +               __entry->pool_name)
>> >> +);
>> >> +
>> >> +TRACE_EVENT(zsmalloc_compact_end,
>> >> +
>> >> +     TP_PROTO(const char *pool_name, unsigned long pages_compacted,
>> >> +                     unsigned long pages_total_compacted),
>> >> +
>> >> +     TP_ARGS(pool_name, pages_compacted, pages_total_compacted),
>> >> +
>> >> +     TP_STRUCT__entry(
>> >> +             __field(const char *, pool_name)
>> >> +             __field(unsigned long, pages_compacted)
>> >> +             __field(unsigned long, pages_total_compacted)
>> >> +     ),
>> >> +
>> >> +     TP_fast_assign(
>> >> +             __entry->pool_name = pool_name;
>> >> +             __entry->pages_compacted = pages_compacted;
>> >> +             __entry->pages_total_compacted = pages_total_compacted;
>> >> +     ),
>> >> +
>> >> +     TP_printk("pool %s: %ld pages compacted(total %ld)",
>> >> +               __entry->pool_name,
>> >> +               __entry->pages_compacted,
>> >> +               __entry->pages_total_compacted)
>> >> +);
>> >> +
>> >> +#endif /* _TRACE_ZSMALLOC_H */
>> >> +
>> >> +/* This part must be outside protection */
>> >> +#include <trace/define_trace.h>
>> >> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
>> >> index 213d0e1..441b9f7 100644
>> >> --- a/mm/zsmalloc.c
>> >> +++ b/mm/zsmalloc.c
>> >> @@ -30,6 +30,8 @@
>> >>
>> >>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>> >>
>> >> +#define CREATE_TRACE_POINTS
>> >> +
>> >>  #include <linux/module.h>
>> >>  #include <linux/kernel.h>
>> >>  #include <linux/sched.h>
>> >> @@ -52,6 +54,7 @@
>> >>  #include <linux/mount.h>
>> >>  #include <linux/compaction.h>
>> >>  #include <linux/pagemap.h>
>> >> +#include <trace/events/zsmalloc.h>
>> >>
>> >>  #define ZSPAGE_MAGIC 0x58
>> >>
>> >> @@ -2330,6 +2333,9 @@ unsigned long zs_compact(struct zs_pool *pool)
>> >>  {
>> >>       int i;
>> >>       struct size_class *class;
>> >> +     unsigned long pages_compacted_before = pool->stats.pages_compacted;
>> >> +
>> >> +     trace_zsmalloc_compact_start(pool->name);
>> >>
>> >>       for (i = zs_size_classes - 1; i >= 0; i--) {
>> >>               class = pool->size_class[i];
>> >> @@ -2340,6 +2346,10 @@ unsigned long zs_compact(struct zs_pool *pool)
>> >>               __zs_compact(pool, class);
>> >>       }
>> >>
>> >> +     trace_zsmalloc_compact_end(pool->name,
>> >> +             pool->stats.pages_compacted - pages_compacted_before,
>> >> +             pool->stats.pages_compacted);
>> >> +
>> >>       return pool->stats.pages_compacted;
>> >>  }
>> >>  EXPORT_SYMBOL_GPL(zs_compact);
>> >> --
>> >> 1.9.1
>> >>

Re: [PATCH] mm/zsmalloc: add trace events for zs_compact

[Sergey Senozhatsky]

Hello,

On (06/08/16 14:39), Ganesh Mahendran wrote:
> >> > On Tue, Jun 07, 2016 at 04:56:44PM +0800, Ganesh Mahendran wrote:
> >> >> Currently zsmalloc is widely used in android device.
> >> >> Sometimes, we want to see how frequently zs_compact is
> >> >> triggered or how may pages freed by zs_compact(), or which
> >> >> zsmalloc pool is compacted.
> >> >>
> >> >> Most of the time, user can get the brief information from
> >> >> trace_mm_shrink_slab_[start | end], but in some senario,
> >> >> they do not use zsmalloc shrinker, but trigger compaction manually.
> >> >> So add some trace events in zs_compact is convenient. Also we
> >> >> can add some zsmalloc specific information(pool name, total compact
> >> >> pages, etc) in zsmalloc trace.
> >> >
> >> > Sorry, I cannot understand what's the problem now and what you want to
> >> > solve. Could you elaborate it a bit?
> >> >
> >> > Thanks.
> >>
> >> We have backported the zs_compact() to our product(kernel 3.18).
> >> It is usefull for a longtime running device.
> >> But there is not a convenient way to get the detailed information
> >> of zs_comapct() which is usefull for  performance optimization.
> >> Information about how much time zs_compact used, which pool is
> >> compacted, how many page freed, etc.

sorry, couldn't check my email earlier.

zs_compact() is just one of the N sites that are getting called by
the shrinker; optimization here will "solve" only 1/N of the problems.
are there any trace events in any other shrinker callbacks?


why trace_mm_shrink_slab_start()/trace_mm_shrink_slab_end()/etc. don't work you?

	-ss

Re: [PATCH] mm/zsmalloc: add trace events for zs_compact

[Ganesh Mahendran]

Hi, Sergey:

2016-06-08 22:10 GMT+08:00 Sergey Senozhatsky <sergey.senozhatsky@gmail.com>:
> Hello,
>
> On (06/08/16 14:39), Ganesh Mahendran wrote:
>> >> > On Tue, Jun 07, 2016 at 04:56:44PM +0800, Ganesh Mahendran wrote:
>> >> >> Currently zsmalloc is widely used in android device.
>> >> >> Sometimes, we want to see how frequently zs_compact is
>> >> >> triggered or how may pages freed by zs_compact(), or which
>> >> >> zsmalloc pool is compacted.
>> >> >>
>> >> >> Most of the time, user can get the brief information from
>> >> >> trace_mm_shrink_slab_[start | end], but in some senario,
>> >> >> they do not use zsmalloc shrinker, but trigger compaction manually.
>> >> >> So add some trace events in zs_compact is convenient. Also we
>> >> >> can add some zsmalloc specific information(pool name, total compact
>> >> >> pages, etc) in zsmalloc trace.
>> >> >
>> >> > Sorry, I cannot understand what's the problem now and what you want to
>> >> > solve. Could you elaborate it a bit?
>> >> >
>> >> > Thanks.
>> >>
>> >> We have backported the zs_compact() to our product(kernel 3.18).
>> >> It is usefull for a longtime running device.
>> >> But there is not a convenient way to get the detailed information
>> >> of zs_comapct() which is usefull for  performance optimization.
>> >> Information about how much time zs_compact used, which pool is
>> >> compacted, how many page freed, etc.
>
> sorry, couldn't check my email earlier.

Sorry for the late response. I'm off for a few days annual leave.

>
> zs_compact() is just one of the N sites that are getting called by
> the shrinker; optimization here will "solve" only 1/N of the problems.
> are there any trace events in any other shrinker callbacks?

Yes, there are trace events in ext4 shrinker:
-----
fs/ext4/extents_status.c: ext4_es_scan()
{
        trace_ext4_es_shrink_scan_enter(sbi->s_sb, nr_to_scan, ret);
...
        nr_shrunk = __es_shrink(sbi, nr_to_scan, NULL);

        trace_ext4_es_shrink_scan_exit(sbi->s_sb, nr_shrunk, ret);
}
-----

>
>
> why trace_mm_shrink_slab_start()/trace_mm_shrink_slab_end()/etc. don't work you?

I think trace_mm_shrink_slab_start/end is for general usage. If we
have some specific information(such as pool name,  shrink count
or others in the future), it will be convenient when we have
zs_compact trace event.

Also, if user do manually zs_compact instead of shrinker,
he can get zs_compact information from trace event.

Thanks.

>
>         -ss

Re: [PATCH] mm/zsmalloc: add trace events for zs_compact

[Minchan Kim]

On Tue, Jun 07, 2016 at 04:56:44PM +0800, Ganesh Mahendran wrote:
> Currently zsmalloc is widely used in android device.
> Sometimes, we want to see how frequently zs_compact is
> triggered or how may pages freed by zs_compact(), or which
> zsmalloc pool is compacted.
> 
> Most of the time, user can get the brief information from
> trace_mm_shrink_slab_[start | end], but in some senario,
> they do not use zsmalloc shrinker, but trigger compaction manually.
> So add some trace events in zs_compact is convenient. Also we
> can add some zsmalloc specific information(pool name, total compact
> pages, etc) in zsmalloc trace.
> 
> This patch add two trace events for zs_compact(), below the trace log:
> -----------------------------
> root@land:/ # cat /d/tracing/trace
>          kswapd0-125   [007] ...1   174.176979: zsmalloc_compact_start: pool zram0
>          kswapd0-125   [007] ...1   174.181967: zsmalloc_compact_end: pool zram0: 608 pages compacted(total 1794)
>          kswapd0-125   [000] ...1   184.134475: zsmalloc_compact_start: pool zram0
>          kswapd0-125   [000] ...1   184.135010: zsmalloc_compact_end: pool zram0: 62 pages compacted(total 1856)
>          kswapd0-125   [003] ...1   226.927221: zsmalloc_compact_start: pool zram0
>          kswapd0-125   [003] ...1   226.928575: zsmalloc_compact_end: pool zram0: 250 pages compacted(total 2106)
> -----------------------------
> 
> Signed-off-by: Ganesh Mahendran <opensource.ganesh@gmail.com>
> ---
>  include/trace/events/zsmalloc.h | 56 +++++++++++++++++++++++++++++++++++++++++
>  mm/zsmalloc.c                   | 10 ++++++++
>  2 files changed, 66 insertions(+)
>  create mode 100644 include/trace/events/zsmalloc.h
> 
> diff --git a/include/trace/events/zsmalloc.h b/include/trace/events/zsmalloc.h
> new file mode 100644
> index 0000000..3b6f14e
> --- /dev/null
> +++ b/include/trace/events/zsmalloc.h
> @@ -0,0 +1,56 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM zsmalloc
> +
> +#if !defined(_TRACE_ZSMALLOC_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_ZSMALLOC_H
> +
> +#include <linux/types.h>
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT(zsmalloc_compact_start,

I prefer zs_compact_start.

> +
> +	TP_PROTO(const char *pool_name),
> +
> +	TP_ARGS(pool_name),
> +
> +	TP_STRUCT__entry(
> +		__field(const char *, pool_name)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->pool_name = pool_name;
> +	),
> +
> +	TP_printk("pool %s",
> +		  __entry->pool_name)
> +);
> +
> +TRACE_EVENT(zsmalloc_compact_end,
> +
> +	TP_PROTO(const char *pool_name, unsigned long pages_compacted,
> +			unsigned long pages_total_compacted),

Hmm, do we really need pages_total_compacted?

> +
> +	TP_ARGS(pool_name, pages_compacted, pages_total_compacted),
> +
> +	TP_STRUCT__entry(
> +		__field(const char *, pool_name)
> +		__field(unsigned long, pages_compacted)
> +		__field(unsigned long, pages_total_compacted)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->pool_name = pool_name;
> +		__entry->pages_compacted = pages_compacted;
> +		__entry->pages_total_compacted = pages_total_compacted;
> +	),
> +
> +	TP_printk("pool %s: %ld pages compacted(total %ld)",
> +		  __entry->pool_name,
> +		  __entry->pages_compacted,
> +		  __entry->pages_total_compacted)
> +);
> +
> +#endif /* _TRACE_ZSMALLOC_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index 213d0e1..441b9f7 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -30,6 +30,8 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> +#define CREATE_TRACE_POINTS
> +
>  #include <linux/module.h>
>  #include <linux/kernel.h>
>  #include <linux/sched.h>
> @@ -52,6 +54,7 @@
>  #include <linux/mount.h>
>  #include <linux/compaction.h>
>  #include <linux/pagemap.h>
> +#include <trace/events/zsmalloc.h>
>  
>  #define ZSPAGE_MAGIC	0x58
>  
> @@ -2330,6 +2333,9 @@ unsigned long zs_compact(struct zs_pool *pool)
>  {
>  	int i;
>  	struct size_class *class;
> +	unsigned long pages_compacted_before = pool->stats.pages_compacted;
> +
> +	trace_zsmalloc_compact_start(pool->name);

How about moving it into __zs_compact with size_class information?
It would be more useful, I think.

>  
>  	for (i = zs_size_classes - 1; i >= 0; i--) {
>  		class = pool->size_class[i];
> @@ -2340,6 +2346,10 @@ unsigned long zs_compact(struct zs_pool *pool)
>  		__zs_compact(pool, class);
>  	}
>  
> +	trace_zsmalloc_compact_end(pool->name,
> +		pool->stats.pages_compacted - pages_compacted_before,
> +		pool->stats.pages_compacted);
> +
>  	return pool->stats.pages_compacted;
>  }
>  EXPORT_SYMBOL_GPL(zs_compact);
> -- 
> 1.9.1
> 

Re: [PATCH] mm/zsmalloc: add trace events for zs_compact

[Minchan Kim]

On Wed, Jun 08, 2016 at 02:39:19PM +0800, Ganesh Mahendran wrote:

<snip>

> zsmalloc is not only used by zram, but also zswap. Maybe
> others in the future.
> 
> I tried to use function_graph. It seems there are too much log
> printed:
> ------
> root@leo-test:/sys/kernel/debug/tracing# cat trace
> # tracer: function_graph
> #
> # CPU  DURATION                  FUNCTION CALLS
> # |     |   |                     |   |   |   |
>  2)               |  zs_compact [zsmalloc]() {
>  2)               |  /* zsmalloc_compact_start: pool zram0 */
>  2)   0.889 us    |    _raw_spin_lock();
>  2)   0.896 us    |    isolate_zspage [zsmalloc]();
>  2)   0.938 us    |    _raw_spin_lock();
>  2)   0.875 us    |    isolate_zspage [zsmalloc]();
>  2)   0.942 us    |    _raw_spin_lock();
>  2)   0.962 us    |    isolate_zspage [zsmalloc]();
> ...
>  2)   0.879 us    |      insert_zspage [zsmalloc]();
>  2)   4.520 us    |    }
>  2)   0.975 us    |    _raw_spin_lock();
>  2)   0.890 us    |    isolate_zspage [zsmalloc]();
>  2)   0.882 us    |    _raw_spin_lock();
>  2)   0.894 us    |    isolate_zspage [zsmalloc]();
>  2)               |  /* zsmalloc_compact_end: pool zram0: 0 pages
> compacted(total 0) */
>  2) # 1351.241 us |  }
> ------
> => 1351.241 us used
> 
> And it seems the overhead of function_graph is bigger than trace event.
> 
> bash-3682  [002] ....  1439.180646: zsmalloc_compact_start: pool zram0
> bash-3682  [002] ....  1439.180659: zsmalloc_compact_end: pool zram0:
> 0 pages compacted(total 0)
> => 13 us > 1351.241 us

You could use set_ftrace_filter to cut out.

To introduce new event trace to get a elasped time, it's pointless,
I think.

It should have more like pool name you mentioned.
Like saying other thread, It would be better to show
[pool name, compact size_class,
the number of object moved, the number of freed page], IMO.

Thanks.

Re: [PATCH] mm/zsmalloc: add trace events for zs_compact

[Sergey Senozhatsky]

Hello,

On (06/13/16 13:42), Minchan Kim wrote:
[..]
> > compacted(total 0) */
> >  2) # 1351.241 us |  }
> > ------
> > => 1351.241 us used
> > 
> > And it seems the overhead of function_graph is bigger than trace event.
> > 
> > bash-3682  [002] ....  1439.180646: zsmalloc_compact_start: pool zram0
> > bash-3682  [002] ....  1439.180659: zsmalloc_compact_end: pool zram0:
> > 0 pages compacted(total 0)
> > => 13 us > 1351.241 us
> 
> You could use set_ftrace_filter to cut out.
> 
> To introduce new event trace to get a elasped time, it's pointless,
> I think.
> 
> It should have more like pool name you mentioned.
> Like saying other thread, It would be better to show
> [pool name, compact size_class,
> the number of object moved, the number of freed page], IMO.

just my 5 cents:

some parts (of the info above) are already available: zram<ID> maps to
pool<ID> name, which maps to a sysfs file name, that can contain the rest.
I'm just trying to understand what kind of optimizations we are talking
about here and how would timings help... compaction can spin on class
lock, for example, if the device in question is busy, etc. etc. on the
other hand we have a per-class info in zsmalloc pool stats output, so
why not extend it instead of introducing a new debugging interface?

	-ss

Re: [PATCH] mm/zsmalloc: add trace events for zs_compact

[Ganesh Mahendran]

2016-06-13 12:42 GMT+08:00 Minchan Kim <minchan@kernel.org>:
> On Wed, Jun 08, 2016 at 02:39:19PM +0800, Ganesh Mahendran wrote:
>
> <snip>
>
>> zsmalloc is not only used by zram, but also zswap. Maybe
>> others in the future.
>>
>> I tried to use function_graph. It seems there are too much log
>> printed:
>> ------
>> root@leo-test:/sys/kernel/debug/tracing# cat trace
>> # tracer: function_graph
>> #
>> # CPU  DURATION                  FUNCTION CALLS
>> # |     |   |                     |   |   |   |
>>  2)               |  zs_compact [zsmalloc]() {
>>  2)               |  /* zsmalloc_compact_start: pool zram0 */
>>  2)   0.889 us    |    _raw_spin_lock();
>>  2)   0.896 us    |    isolate_zspage [zsmalloc]();
>>  2)   0.938 us    |    _raw_spin_lock();
>>  2)   0.875 us    |    isolate_zspage [zsmalloc]();
>>  2)   0.942 us    |    _raw_spin_lock();
>>  2)   0.962 us    |    isolate_zspage [zsmalloc]();
>> ...
>>  2)   0.879 us    |      insert_zspage [zsmalloc]();
>>  2)   4.520 us    |    }
>>  2)   0.975 us    |    _raw_spin_lock();
>>  2)   0.890 us    |    isolate_zspage [zsmalloc]();
>>  2)   0.882 us    |    _raw_spin_lock();
>>  2)   0.894 us    |    isolate_zspage [zsmalloc]();
>>  2)               |  /* zsmalloc_compact_end: pool zram0: 0 pages
>> compacted(total 0) */
>>  2) # 1351.241 us |  }
>> ------
>> => 1351.241 us used
>>
>> And it seems the overhead of function_graph is bigger than trace event.
>>
>> bash-3682  [002] ....  1439.180646: zsmalloc_compact_start: pool zram0
>> bash-3682  [002] ....  1439.180659: zsmalloc_compact_end: pool zram0:
>> 0 pages compacted(total 0)
>> => 13 us > 1351.241 us
>
> You could use  to cut out.
>
> To introduce new event trace to get a elasped time, it's pointless,
> I think.

Agree.

>
> It should have more like pool name you mentioned.
> Like saying other thread, It would be better to show
> [pool name, compact size_class,
> the number of object moved, the number of freed page], IMO.

Thanks for you suggestion!
I would be useful to see compact details for each class.
I will send another patch to do this.

Thanks.

>
> Thanks.

Re: [PATCH] mm/zsmalloc: add trace events for zs_compact

[Ganesh Mahendran]

2016-06-13 13:12 GMT+08:00 Sergey Senozhatsky
<sergey.senozhatsky.work@gmail.com>:
> Hello,
>
> On (06/13/16 13:42), Minchan Kim wrote:
> [..]
>> > compacted(total 0) */
>> >  2) # 1351.241 us |  }
>> > ------
>> > => 1351.241 us used
>> >
>> > And it seems the overhead of function_graph is bigger than trace event.
>> >
>> > bash-3682  [002] ....  1439.180646: zsmalloc_compact_start: pool zram0
>> > bash-3682  [002] ....  1439.180659: zsmalloc_compact_end: pool zram0:
>> > 0 pages compacted(total 0)
>> > => 13 us > 1351.241 us
>>
>> You could use set_ftrace_filter to cut out.
>>
>> To introduce new event trace to get a elasped time, it's pointless,
>> I think.
>>
>> It should have more like pool name you mentioned.
>> Like saying other thread, It would be better to show
>> [pool name, compact size_class,
>> the number of object moved, the number of freed page], IMO.
>
> just my 5 cents:
>
> some parts (of the info above) are already available: zram<ID> maps to
> pool<ID> name, which maps to a sysfs file name, that can contain the rest.
> I'm just trying to understand what kind of optimizations we are talking
> about here and how would timings help... compaction can spin on class
> lock, for example, if the device in question is busy, etc. etc. on the
> other hand we have a per-class info in zsmalloc pool stats output, so
> why not extend it instead of introducing a new debugging interface?

I've considered adding new interface in /sys/../zsmalloc/ or uasing
trace_mm_shrink_slab_[start/end] to get such information.
But none of them can cover all the cases:
1) distinguish which zs pool is compacted.
2) freed pages of zs_compact(), total freed pages of zs_compact()
3) realtime log printed

Actually, the trace event added in zs_compact not only just for
debugging/optimization inside zsmalloc, but also for system level.
We can do some analysis by combining data from zs_compac(), system
information(like free mem, swap info, LMK, etc)

Thanks.

>
>         -ss

Re: [PATCH] mm/zsmalloc: add trace events for zs_compact

[Sergey Senozhatsky]

On (06/13/16 15:49), Ganesh Mahendran wrote:
[..]
> > some parts (of the info above) are already available: zram<ID> maps to
> > pool<ID> name, which maps to a sysfs file name, that can contain the rest.
> > I'm just trying to understand what kind of optimizations we are talking
> > about here and how would timings help... compaction can spin on class
> > lock, for example, if the device in question is busy, etc. etc. on the
> > other hand we have a per-class info in zsmalloc pool stats output, so
> > why not extend it instead of introducing a new debugging interface?
> 
> I've considered adding new interface in /sys/../zsmalloc/ or uasing
> trace_mm_shrink_slab_[start/end] to get such information.
> But none of them can cover all the cases:
> 1) distinguish which zs pool is compacted.
> 2) freed pages of zs_compact(), total freed pages of zs_compact()
> 3) realtime log printed

I'm not against the patch in general, just curious, do you have any
specific optimization in mind? if so, can we start with that optimization
then, otherwise, can we define what type of optimizations this tracing
will boost?

what I'm thinking of, we have a zsmalloc debugfs file, which provides
per-device->per-pool->per-class stats:

cat /sys/kernel/debug/zsmalloc/zram0/classes
 class  size almost_full almost_empty obj_allocated   obj_used pages_used pages_per_zspage freeable

so the 'missing' thing is just one column, right? the total freed
pages number is already accounted.

thoughts?

	-ss