[PATCH V2] vmscan/trace: Add 'active' and 'file' info to trace_mm_vmscan_lru_isolate.

[PATCH V2] vmscan/trace: Add 'active' and 'file' info to trace_mm_vmscan_lru_isolate.

[Tao Ma]

From: Tao Ma <boyu.mt@taobao.com>

In trace_mm_vmscan_lru_isolate, we don't output 'active' and 'file'
information to the trace event and it is a bit inconvenient for the
user to get the real information(like pasted below).
mm_vmscan_lru_isolate: isolate_mode=2 order=0 nr_requested=32 nr_scanned=32
nr_taken=32 contig_taken=0 contig_dirty=0 contig_failed=0

So this patch adds these 2 info to the trace event and it now looks like:
mm_vmscan_lru_isolate: isolate_mode=2 order=0 nr_requested=32 nr_scanned=32
nr_taken=32 contig_taken=0 contig_dirty=0 contig_failed=0 active=1 file=0

Cc: Mel Gorman <mel@csn.ul.ie>
Cc: Rik van Riel <riel@redhat.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Tao Ma <boyu.mt@taobao.com>
---
 include/trace/events/vmscan.h |   25 +++++++++++++++++--------
 mm/memcontrol.c               |    2 +-
 mm/vmscan.c                   |    6 +++---
 3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
index edc4b3d..82bc49c 100644
--- a/include/trace/events/vmscan.h
+++ b/include/trace/events/vmscan.h
@@ -266,9 +266,10 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
 		unsigned long nr_lumpy_taken,
 		unsigned long nr_lumpy_dirty,
 		unsigned long nr_lumpy_failed,
-		isolate_mode_t isolate_mode),
+		isolate_mode_t isolate_mode,
+		int active, int file),
 
-	TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode),
+	TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode, active, file),
 
 	TP_STRUCT__entry(
 		__field(int, order)
@@ -279,6 +280,8 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
 		__field(unsigned long, nr_lumpy_dirty)
 		__field(unsigned long, nr_lumpy_failed)
 		__field(isolate_mode_t, isolate_mode)
+		__field(int, active)
+		__field(int, file)
 	),
 
 	TP_fast_assign(
@@ -290,9 +293,11 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
 		__entry->nr_lumpy_dirty = nr_lumpy_dirty;
 		__entry->nr_lumpy_failed = nr_lumpy_failed;
 		__entry->isolate_mode = isolate_mode;
+		__entry->active = active;
+		__entry->file = file;
 	),
 
-	TP_printk("isolate_mode=%d order=%d nr_requested=%lu nr_scanned=%lu nr_taken=%lu contig_taken=%lu contig_dirty=%lu contig_failed=%lu",
+	TP_printk("isolate_mode=%d order=%d nr_requested=%lu nr_scanned=%lu nr_taken=%lu contig_taken=%lu contig_dirty=%lu contig_failed=%lu active=%d file=%d",
 		__entry->isolate_mode,
 		__entry->order,
 		__entry->nr_requested,
@@ -300,7 +305,9 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
 		__entry->nr_taken,
 		__entry->nr_lumpy_taken,
 		__entry->nr_lumpy_dirty,
-		__entry->nr_lumpy_failed)
+		__entry->nr_lumpy_failed,
+		__entry->active,
+		__entry->file)
 );
 
 DEFINE_EVENT(mm_vmscan_lru_isolate_template, mm_vmscan_lru_isolate,
@@ -312,9 +319,10 @@ DEFINE_EVENT(mm_vmscan_lru_isolate_template, mm_vmscan_lru_isolate,
 		unsigned long nr_lumpy_taken,
 		unsigned long nr_lumpy_dirty,
 		unsigned long nr_lumpy_failed,
-		isolate_mode_t isolate_mode),
+		isolate_mode_t isolate_mode,
+		int active, int file),
 
-	TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode)
+	TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode, active, file)
 
 );
 
@@ -327,9 +335,10 @@ DEFINE_EVENT(mm_vmscan_lru_isolate_template, mm_vmscan_memcg_isolate,
 		unsigned long nr_lumpy_taken,
 		unsigned long nr_lumpy_dirty,
 		unsigned long nr_lumpy_failed,
-		isolate_mode_t isolate_mode),
+		isolate_mode_t isolate_mode,
+		int active, int file),
 
-	TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode)
+	TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode, active, file)
 
 );
 
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6aff93c..246fbce 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1249,7 +1249,7 @@ unsigned long mem_cgroup_isolate_pages(unsigned long nr_to_scan,
 	*scanned = scan;
 
 	trace_mm_vmscan_memcg_isolate(0, nr_to_scan, scan, nr_taken,
-				      0, 0, 0, mode);
+				      0, 0, 0, mode, active, file);
 
 	return nr_taken;
 }
diff --git a/mm/vmscan.c b/mm/vmscan.c
index f54a05b..97955ca 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1103,7 +1103,7 @@ int __isolate_lru_page(struct page *page, isolate_mode_t mode, int file)
 static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 		struct list_head *src, struct list_head *dst,
 		unsigned long *scanned, int order, isolate_mode_t mode,
-		int file)
+		int active, int file)
 {
 	unsigned long nr_taken = 0;
 	unsigned long nr_lumpy_taken = 0;
@@ -1221,7 +1221,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 			nr_to_scan, scan,
 			nr_taken,
 			nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed,
-			mode);
+			mode, active, file);
 	return nr_taken;
 }
 
@@ -1237,7 +1237,7 @@ static unsigned long isolate_pages_global(unsigned long nr,
 	if (file)
 		lru += LRU_FILE;
 	return isolate_lru_pages(nr, &z->lru[lru].list, dst, scanned, order,
-								mode, file);
+							mode, active, file);
 }
 
 /*
-- 
1.7.0.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH V2] vmscan/trace: Add 'active' and 'file' info to trace_mm_vmscan_lru_isolate.

[KAMEZAWA Hiroyuki]

On Sun, 11 Dec 2011 22:46:24 +0800
Tao Ma <tm@tao.ma> wrote:

> From: Tao Ma <boyu.mt@taobao.com>
> 
> In trace_mm_vmscan_lru_isolate, we don't output 'active' and 'file'
> information to the trace event and it is a bit inconvenient for the
> user to get the real information(like pasted below).
> mm_vmscan_lru_isolate: isolate_mode=2 order=0 nr_requested=32 nr_scanned=32
> nr_taken=32 contig_taken=0 contig_dirty=0 contig_failed=0
> 
> So this patch adds these 2 info to the trace event and it now looks like:
> mm_vmscan_lru_isolate: isolate_mode=2 order=0 nr_requested=32 nr_scanned=32
> nr_taken=32 contig_taken=0 contig_dirty=0 contig_failed=0 active=1 file=0
> 
> Cc: Mel Gorman <mel@csn.ul.ie>
> Cc: Rik van Riel <riel@redhat.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Cc: Andrea Arcangeli <aarcange@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Tao Ma <boyu.mt@taobao.com>

Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH V2] vmscan/trace: Add 'active' and 'file' info to trace_mm_vmscan_lru_isolate.

[Minchan Kim]

Hi Tao,

On Sun, Dec 11, 2011 at 11:46 PM, Tao Ma <tm@tao.ma> wrote:
> From: Tao Ma <boyu.mt@taobao.com>
>
> In trace_mm_vmscan_lru_isolate, we don't output 'active' and 'file'
> information to the trace event and it is a bit inconvenient for the
> user to get the real information(like pasted below).
> mm_vmscan_lru_isolate: isolate_mode=2 order=0 nr_requested=32 nr_scanned=32
> nr_taken=32 contig_taken=0 contig_dirty=0 contig_failed=0
>
> So this patch adds these 2 info to the trace event and it now looks like:
> mm_vmscan_lru_isolate: isolate_mode=2 order=0 nr_requested=32 nr_scanned=32
> nr_taken=32 contig_taken=0 contig_dirty=0 contig_failed=0 active=1 file=0
>
> Cc: Mel Gorman <mel@csn.ul.ie>
> Cc: Rik van Riel <riel@redhat.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Cc: Andrea Arcangeli <aarcange@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Tao Ma <boyu.mt@taobao.com>
> ---
>  include/trace/events/vmscan.h |   25 +++++++++++++++++--------
>  mm/memcontrol.c               |    2 +-
>  mm/vmscan.c                   |    6 +++---
>  3 files changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
> index edc4b3d..82bc49c 100644
> --- a/include/trace/events/vmscan.h
> +++ b/include/trace/events/vmscan.h
> @@ -266,9 +266,10 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
>                unsigned long nr_lumpy_taken,
>                unsigned long nr_lumpy_dirty,
>                unsigned long nr_lumpy_failed,
> -               isolate_mode_t isolate_mode),
> +               isolate_mode_t isolate_mode,
> +               int active, int file),
>
> -       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode),
> +       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode, active, file),
>
>        TP_STRUCT__entry(
>                __field(int, order)
> @@ -279,6 +280,8 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
>                __field(unsigned long, nr_lumpy_dirty)
>                __field(unsigned long, nr_lumpy_failed)
>                __field(isolate_mode_t, isolate_mode)
> +               __field(int, active)
> +               __field(int, file)
>        ),
>
>        TP_fast_assign(
> @@ -290,9 +293,11 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
>                __entry->nr_lumpy_dirty = nr_lumpy_dirty;
>                __entry->nr_lumpy_failed = nr_lumpy_failed;
>                __entry->isolate_mode = isolate_mode;
> +               __entry->active = active;
> +               __entry->file = file;
>        ),
>
> -       TP_printk("isolate_mode=%d order=%d nr_requested=%lu nr_scanned=%lu nr_taken=%lu contig_taken=%lu contig_dirty=%lu contig_failed=%lu",
> +       TP_printk("isolate_mode=%d order=%d nr_requested=%lu nr_scanned=%lu nr_taken=%lu contig_taken=%lu contig_dirty=%lu contig_failed=%lu active=%d file=%d",
>                __entry->isolate_mode,
>                __entry->order,
>                __entry->nr_requested,
> @@ -300,7 +305,9 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
>                __entry->nr_taken,
>                __entry->nr_lumpy_taken,
>                __entry->nr_lumpy_dirty,
> -               __entry->nr_lumpy_failed)
> +               __entry->nr_lumpy_failed,
> +               __entry->active,
> +               __entry->file)
>  );
>
>  DEFINE_EVENT(mm_vmscan_lru_isolate_template, mm_vmscan_lru_isolate,
> @@ -312,9 +319,10 @@ DEFINE_EVENT(mm_vmscan_lru_isolate_template, mm_vmscan_lru_isolate,
>                unsigned long nr_lumpy_taken,
>                unsigned long nr_lumpy_dirty,
>                unsigned long nr_lumpy_failed,
> -               isolate_mode_t isolate_mode),
> +               isolate_mode_t isolate_mode,
> +               int active, int file),
>
> -       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode)
> +       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode, active, file)
>
>  );
>
> @@ -327,9 +335,10 @@ DEFINE_EVENT(mm_vmscan_lru_isolate_template, mm_vmscan_memcg_isolate,
>                unsigned long nr_lumpy_taken,
>                unsigned long nr_lumpy_dirty,
>                unsigned long nr_lumpy_failed,
> -               isolate_mode_t isolate_mode),
> +               isolate_mode_t isolate_mode,
> +               int active, int file),
>
> -       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode)
> +       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode, active, file)
>
>  );
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 6aff93c..246fbce 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1249,7 +1249,7 @@ unsigned long mem_cgroup_isolate_pages(unsigned long nr_to_scan,
>        *scanned = scan;
>
>        trace_mm_vmscan_memcg_isolate(0, nr_to_scan, scan, nr_taken,
> -                                     0, 0, 0, mode);
> +                                     0, 0, 0, mode, active, file);
>
>        return nr_taken;
>  }
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index f54a05b..97955ca 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1103,7 +1103,7 @@ int __isolate_lru_page(struct page *page, isolate_mode_t mode, int file)
>  static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>                struct list_head *src, struct list_head *dst,
>                unsigned long *scanned, int order, isolate_mode_t mode,
> -               int file)
> +               int active, int file)
>  {
>        unsigned long nr_taken = 0;
>        unsigned long nr_lumpy_taken = 0;
> @@ -1221,7 +1221,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>                        nr_to_scan, scan,
>                        nr_taken,
>                        nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed,
> -                       mode);
> +                       mode, active, file);
>        return nr_taken;
>  }
>
> @@ -1237,7 +1237,7 @@ static unsigned long isolate_pages_global(unsigned long nr,
>        if (file)
>                lru += LRU_FILE;
>        return isolate_lru_pages(nr, &z->lru[lru].list, dst, scanned, order,
> -                                                               mode, file);
> +                                                       mode, active, file);

I guess you want to count exact scanning number of which lru list.
But It's impossible now since we do lumpy reclaim so that trace's
result is mixed by active/inactive list scanning.
And I don't like adding new argument for just trace although it's trivial.

I think 'mode' is more proper rather than  specific 'active'.
The 'mode' can achieve your goal without passing new argument "active".

In addition to, current mmotm has various modes.
So sometime we can get more specific result rather than vauge 'active'.


/* Isolate inactive pages */
#define ISOLATE_INACTIVE        ((__force fmode_t)0x1)
/* Isolate active pages */
#define ISOLATE_ACTIVE          ((__force fmode_t)0x2)
/* Isolate clean file */
#define ISOLATE_CLEAN           ((__force fmode_t)0x4)
/* Isolate unmapped file */
#define ISOLATE_UNMAPPED        ((__force fmode_t)0x8)


-- 
Kind regards,
Minchan Kim

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH V2] vmscan/trace: Add 'active' and 'file' info to trace_mm_vmscan_lru_isolate.

[Tao Ma]

On 12/12/2011 08:59 AM, Minchan Kim wrote:
> Hi Tao,
> 
> On Sun, Dec 11, 2011 at 11:46 PM, Tao Ma <tm@tao.ma> wrote:
>> From: Tao Ma <boyu.mt@taobao.com>
>>
>> In trace_mm_vmscan_lru_isolate, we don't output 'active' and 'file'
>> information to the trace event and it is a bit inconvenient for the
>> user to get the real information(like pasted below).
>> mm_vmscan_lru_isolate: isolate_mode=2 order=0 nr_requested=32 nr_scanned=32
>> nr_taken=32 contig_taken=0 contig_dirty=0 contig_failed=0
>>
>> So this patch adds these 2 info to the trace event and it now looks like:
>> mm_vmscan_lru_isolate: isolate_mode=2 order=0 nr_requested=32 nr_scanned=32
>> nr_taken=32 contig_taken=0 contig_dirty=0 contig_failed=0 active=1 file=0
>>
>> Cc: Mel Gorman <mel@csn.ul.ie>
>> Cc: Rik van Riel <riel@redhat.com>
>> Cc: Johannes Weiner <hannes@cmpxchg.org>
>> Cc: Christoph Hellwig <hch@infradead.org>
>> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
>> Cc: Andrea Arcangeli <aarcange@redhat.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Signed-off-by: Tao Ma <boyu.mt@taobao.com>
>> ---
>>  include/trace/events/vmscan.h |   25 +++++++++++++++++--------
>>  mm/memcontrol.c               |    2 +-
>>  mm/vmscan.c                   |    6 +++---
>>  3 files changed, 21 insertions(+), 12 deletions(-)
>>
>> diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
>> index edc4b3d..82bc49c 100644
>> --- a/include/trace/events/vmscan.h
>> +++ b/include/trace/events/vmscan.h
>> @@ -266,9 +266,10 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
>>                unsigned long nr_lumpy_taken,
>>                unsigned long nr_lumpy_dirty,
>>                unsigned long nr_lumpy_failed,
>> -               isolate_mode_t isolate_mode),
>> +               isolate_mode_t isolate_mode,
>> +               int active, int file),
>>
>> -       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode),
>> +       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode, active, file),
>>
>>        TP_STRUCT__entry(
>>                __field(int, order)
>> @@ -279,6 +280,8 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
>>                __field(unsigned long, nr_lumpy_dirty)
>>                __field(unsigned long, nr_lumpy_failed)
>>                __field(isolate_mode_t, isolate_mode)
>> +               __field(int, active)
>> +               __field(int, file)
>>        ),
>>
>>        TP_fast_assign(
>> @@ -290,9 +293,11 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
>>                __entry->nr_lumpy_dirty = nr_lumpy_dirty;
>>                __entry->nr_lumpy_failed = nr_lumpy_failed;
>>                __entry->isolate_mode = isolate_mode;
>> +               __entry->active = active;
>> +               __entry->file = file;
>>        ),
>>
>> -       TP_printk("isolate_mode=%d order=%d nr_requested=%lu nr_scanned=%lu nr_taken=%lu contig_taken=%lu contig_dirty=%lu contig_failed=%lu",
>> +       TP_printk("isolate_mode=%d order=%d nr_requested=%lu nr_scanned=%lu nr_taken=%lu contig_taken=%lu contig_dirty=%lu contig_failed=%lu active=%d file=%d",
>>                __entry->isolate_mode,
>>                __entry->order,
>>                __entry->nr_requested,
>> @@ -300,7 +305,9 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
>>                __entry->nr_taken,
>>                __entry->nr_lumpy_taken,
>>                __entry->nr_lumpy_dirty,
>> -               __entry->nr_lumpy_failed)
>> +               __entry->nr_lumpy_failed,
>> +               __entry->active,
>> +               __entry->file)
>>  );
>>
>>  DEFINE_EVENT(mm_vmscan_lru_isolate_template, mm_vmscan_lru_isolate,
>> @@ -312,9 +319,10 @@ DEFINE_EVENT(mm_vmscan_lru_isolate_template, mm_vmscan_lru_isolate,
>>                unsigned long nr_lumpy_taken,
>>                unsigned long nr_lumpy_dirty,
>>                unsigned long nr_lumpy_failed,
>> -               isolate_mode_t isolate_mode),
>> +               isolate_mode_t isolate_mode,
>> +               int active, int file),
>>
>> -       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode)
>> +       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode, active, file)
>>
>>  );
>>
>> @@ -327,9 +335,10 @@ DEFINE_EVENT(mm_vmscan_lru_isolate_template, mm_vmscan_memcg_isolate,
>>                unsigned long nr_lumpy_taken,
>>                unsigned long nr_lumpy_dirty,
>>                unsigned long nr_lumpy_failed,
>> -               isolate_mode_t isolate_mode),
>> +               isolate_mode_t isolate_mode,
>> +               int active, int file),
>>
>> -       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode)
>> +       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode, active, file)
>>
>>  );
>>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 6aff93c..246fbce 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -1249,7 +1249,7 @@ unsigned long mem_cgroup_isolate_pages(unsigned long nr_to_scan,
>>        *scanned = scan;
>>
>>        trace_mm_vmscan_memcg_isolate(0, nr_to_scan, scan, nr_taken,
>> -                                     0, 0, 0, mode);
>> +                                     0, 0, 0, mode, active, file);
>>
>>        return nr_taken;
>>  }
>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>> index f54a05b..97955ca 100644
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -1103,7 +1103,7 @@ int __isolate_lru_page(struct page *page, isolate_mode_t mode, int file)
>>  static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>>                struct list_head *src, struct list_head *dst,
>>                unsigned long *scanned, int order, isolate_mode_t mode,
>> -               int file)
>> +               int active, int file)
>>  {
>>        unsigned long nr_taken = 0;
>>        unsigned long nr_lumpy_taken = 0;
>> @@ -1221,7 +1221,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>>                        nr_to_scan, scan,
>>                        nr_taken,
>>                        nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed,
>> -                       mode);
>> +                       mode, active, file);
>>        return nr_taken;
>>  }
>>
>> @@ -1237,7 +1237,7 @@ static unsigned long isolate_pages_global(unsigned long nr,
>>        if (file)
>>                lru += LRU_FILE;
>>        return isolate_lru_pages(nr, &z->lru[lru].list, dst, scanned, order,
>> -                                                               mode, file);
>> +                                                       mode, active, file);
> 
> I guess you want to count exact scanning number of which lru list.
> But It's impossible now since we do lumpy reclaim so that trace's
> result is mixed by active/inactive list scanning.
> And I don't like adding new argument for just trace although it's trivial.
yeah, I know we do lumpy reclaim, but it has no hint about whether it is
a file or anon lru. So I think we at least need a 'file=[0/1]' in this
trace event.
> 
> I think 'mode' is more proper rather than  specific 'active'.
> The 'mode' can achieve your goal without passing new argument "active".
sorry, but how can we find the real relationship between 'mode' and
'active'? I am not quite familiar with this field. So if you can
explicit describe it, I am fine to drop this field.

Thanks
Tao
> 
> In addition to, current mmotm has various modes.
> So sometime we can get more specific result rather than vauge 'active'.
> 
> 
> /* Isolate inactive pages */
> #define ISOLATE_INACTIVE        ((__force fmode_t)0x1)
> /* Isolate active pages */
> #define ISOLATE_ACTIVE          ((__force fmode_t)0x2)
> /* Isolate clean file */
> #define ISOLATE_CLEAN           ((__force fmode_t)0x4)
> /* Isolate unmapped file */
> #define ISOLATE_UNMAPPED        ((__force fmode_t)0x8)
> 
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH V2] vmscan/trace: Add 'active' and 'file' info to trace_mm_vmscan_lru_isolate.

[Minchan Kim]

On Mon, Dec 12, 2011 at 10:19 AM, Tao Ma <tm@tao.ma> wrote:
> On 12/12/2011 08:59 AM, Minchan Kim wrote:
>> Hi Tao,
>>
>> On Sun, Dec 11, 2011 at 11:46 PM, Tao Ma <tm@tao.ma> wrote:
>>> From: Tao Ma <boyu.mt@taobao.com>
>>>
>>> In trace_mm_vmscan_lru_isolate, we don't output 'active' and 'file'
>>> information to the trace event and it is a bit inconvenient for the
>>> user to get the real information(like pasted below).
>>> mm_vmscan_lru_isolate: isolate_mode=2 order=0 nr_requested=32 nr_scanned=32
>>> nr_taken=32 contig_taken=0 contig_dirty=0 contig_failed=0
>>>
>>> So this patch adds these 2 info to the trace event and it now looks like:
>>> mm_vmscan_lru_isolate: isolate_mode=2 order=0 nr_requested=32 nr_scanned=32
>>> nr_taken=32 contig_taken=0 contig_dirty=0 contig_failed=0 active=1 file=0
>>>
>>> Cc: Mel Gorman <mel@csn.ul.ie>
>>> Cc: Rik van Riel <riel@redhat.com>
>>> Cc: Johannes Weiner <hannes@cmpxchg.org>
>>> Cc: Christoph Hellwig <hch@infradead.org>
>>> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>>> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
>>> Cc: Andrea Arcangeli <aarcange@redhat.com>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Signed-off-by: Tao Ma <boyu.mt@taobao.com>
>>> ---
>>>  include/trace/events/vmscan.h |   25 +++++++++++++++++--------
>>>  mm/memcontrol.c               |    2 +-
>>>  mm/vmscan.c                   |    6 +++---
>>>  3 files changed, 21 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
>>> index edc4b3d..82bc49c 100644
>>> --- a/include/trace/events/vmscan.h
>>> +++ b/include/trace/events/vmscan.h
>>> @@ -266,9 +266,10 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
>>>                unsigned long nr_lumpy_taken,
>>>                unsigned long nr_lumpy_dirty,
>>>                unsigned long nr_lumpy_failed,
>>> -               isolate_mode_t isolate_mode),
>>> +               isolate_mode_t isolate_mode,
>>> +               int active, int file),
>>>
>>> -       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode),
>>> +       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode, active, file),
>>>
>>>        TP_STRUCT__entry(
>>>                __field(int, order)
>>> @@ -279,6 +280,8 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
>>>                __field(unsigned long, nr_lumpy_dirty)
>>>                __field(unsigned long, nr_lumpy_failed)
>>>                __field(isolate_mode_t, isolate_mode)
>>> +               __field(int, active)
>>> +               __field(int, file)
>>>        ),
>>>
>>>        TP_fast_assign(
>>> @@ -290,9 +293,11 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
>>>                __entry->nr_lumpy_dirty = nr_lumpy_dirty;
>>>                __entry->nr_lumpy_failed = nr_lumpy_failed;
>>>                __entry->isolate_mode = isolate_mode;
>>> +               __entry->active = active;
>>> +               __entry->file = file;
>>>        ),
>>>
>>> -       TP_printk("isolate_mode=%d order=%d nr_requested=%lu nr_scanned=%lu nr_taken=%lu contig_taken=%lu contig_dirty=%lu contig_failed=%lu",
>>> +       TP_printk("isolate_mode=%d order=%d nr_requested=%lu nr_scanned=%lu nr_taken=%lu contig_taken=%lu contig_dirty=%lu contig_failed=%lu active=%d file=%d",
>>>                __entry->isolate_mode,
>>>                __entry->order,
>>>                __entry->nr_requested,
>>> @@ -300,7 +305,9 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
>>>                __entry->nr_taken,
>>>                __entry->nr_lumpy_taken,
>>>                __entry->nr_lumpy_dirty,
>>> -               __entry->nr_lumpy_failed)
>>> +               __entry->nr_lumpy_failed,
>>> +               __entry->active,
>>> +               __entry->file)
>>>  );
>>>
>>>  DEFINE_EVENT(mm_vmscan_lru_isolate_template, mm_vmscan_lru_isolate,
>>> @@ -312,9 +319,10 @@ DEFINE_EVENT(mm_vmscan_lru_isolate_template, mm_vmscan_lru_isolate,
>>>                unsigned long nr_lumpy_taken,
>>>                unsigned long nr_lumpy_dirty,
>>>                unsigned long nr_lumpy_failed,
>>> -               isolate_mode_t isolate_mode),
>>> +               isolate_mode_t isolate_mode,
>>> +               int active, int file),
>>>
>>> -       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode)
>>> +       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode, active, file)
>>>
>>>  );
>>>
>>> @@ -327,9 +335,10 @@ DEFINE_EVENT(mm_vmscan_lru_isolate_template, mm_vmscan_memcg_isolate,
>>>                unsigned long nr_lumpy_taken,
>>>                unsigned long nr_lumpy_dirty,
>>>                unsigned long nr_lumpy_failed,
>>> -               isolate_mode_t isolate_mode),
>>> +               isolate_mode_t isolate_mode,
>>> +               int active, int file),
>>>
>>> -       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode)
>>> +       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode, active, file)
>>>
>>>  );
>>>
>>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>>> index 6aff93c..246fbce 100644
>>> --- a/mm/memcontrol.c
>>> +++ b/mm/memcontrol.c
>>> @@ -1249,7 +1249,7 @@ unsigned long mem_cgroup_isolate_pages(unsigned long nr_to_scan,
>>>        *scanned = scan;
>>>
>>>        trace_mm_vmscan_memcg_isolate(0, nr_to_scan, scan, nr_taken,
>>> -                                     0, 0, 0, mode);
>>> +                                     0, 0, 0, mode, active, file);
>>>
>>>        return nr_taken;
>>>  }
>>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>>> index f54a05b..97955ca 100644
>>> --- a/mm/vmscan.c
>>> +++ b/mm/vmscan.c
>>> @@ -1103,7 +1103,7 @@ int __isolate_lru_page(struct page *page, isolate_mode_t mode, int file)
>>>  static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>>>                struct list_head *src, struct list_head *dst,
>>>                unsigned long *scanned, int order, isolate_mode_t mode,
>>> -               int file)
>>> +               int active, int file)
>>>  {
>>>        unsigned long nr_taken = 0;
>>>        unsigned long nr_lumpy_taken = 0;
>>> @@ -1221,7 +1221,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>>>                        nr_to_scan, scan,
>>>                        nr_taken,
>>>                        nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed,
>>> -                       mode);
>>> +                       mode, active, file);
>>>        return nr_taken;
>>>  }
>>>
>>> @@ -1237,7 +1237,7 @@ static unsigned long isolate_pages_global(unsigned long nr,
>>>        if (file)
>>>                lru += LRU_FILE;
>>>        return isolate_lru_pages(nr, &z->lru[lru].list, dst, scanned, order,
>>> -                                                               mode, file);
>>> +                                                       mode, active, file);
>>
>> I guess you want to count exact scanning number of which lru list.
>> But It's impossible now since we do lumpy reclaim so that trace's
>> result is mixed by active/inactive list scanning.
>> And I don't like adding new argument for just trace although it's trivial.
> yeah, I know we do lumpy reclaim, but it has no hint about whether it is
> a file or anon lru. So I think we at least need a 'file=[0/1]' in this
> trace event.
>>
>> I think 'mode' is more proper rather than  specific 'active'.
>> The 'mode' can achieve your goal without passing new argument "active".
> sorry, but how can we find the real relationship between 'mode' and
> 'active'? I am not quite familiar with this field. So if you can
> explicit describe it, I am fine to drop this field.


mode is following as,

/* Isolate inactive pages */
#define ISOLATE_INACTIVE        ((__force fmode_t)0x1)
/* Isolate active pages */
#define ISOLATE_ACTIVE          ((__force fmode_t)0x2)
/* Isolate clean file */
#define ISOLATE_CLEAN           ((__force fmode_t)0x4)
/* Isolate unmapped file */
#define ISOLATE_UNMAPPED        ((__force fmode_t)0x8)

For example,

If mode is 0x1, it means we are isolating inactive.
If mode is 0x2, it means we are isolating active.
If mode is 0x4|0x2, it mean we are isolating clear pages in active list.
If mode is 0x8|0x2, it mean we are isolate unmapped page in active list.



-- 
Kind regards,
Minchan Kim

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH V2] vmscan/trace: Add 'active' and 'file' info to trace_mm_vmscan_lru_isolate.

[Tao Ma]

On 12/12/2011 09:23 AM, Minchan Kim wrote:
> On Mon, Dec 12, 2011 at 10:19 AM, Tao Ma <tm@tao.ma> wrote:
>> On 12/12/2011 08:59 AM, Minchan Kim wrote:
>>> Hi Tao,
>>>
>>> On Sun, Dec 11, 2011 at 11:46 PM, Tao Ma <tm@tao.ma> wrote:
>>>> From: Tao Ma <boyu.mt@taobao.com>
>>>>
>>>> In trace_mm_vmscan_lru_isolate, we don't output 'active' and 'file'
>>>> information to the trace event and it is a bit inconvenient for the
>>>> user to get the real information(like pasted below).
>>>> mm_vmscan_lru_isolate: isolate_mode=2 order=0 nr_requested=32 nr_scanned=32
>>>> nr_taken=32 contig_taken=0 contig_dirty=0 contig_failed=0
>>>>
>>>> So this patch adds these 2 info to the trace event and it now looks like:
>>>> mm_vmscan_lru_isolate: isolate_mode=2 order=0 nr_requested=32 nr_scanned=32
>>>> nr_taken=32 contig_taken=0 contig_dirty=0 contig_failed=0 active=1 file=0
>>>>
>>>> Cc: Mel Gorman <mel@csn.ul.ie>
>>>> Cc: Rik van Riel <riel@redhat.com>
>>>> Cc: Johannes Weiner <hannes@cmpxchg.org>
>>>> Cc: Christoph Hellwig <hch@infradead.org>
>>>> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
>>>> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
>>>> Cc: Andrea Arcangeli <aarcange@redhat.com>
>>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>>> Signed-off-by: Tao Ma <boyu.mt@taobao.com>
>>>> ---
>>>>  include/trace/events/vmscan.h |   25 +++++++++++++++++--------
>>>>  mm/memcontrol.c               |    2 +-
>>>>  mm/vmscan.c                   |    6 +++---
>>>>  3 files changed, 21 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
>>>> index edc4b3d..82bc49c 100644
>>>> --- a/include/trace/events/vmscan.h
>>>> +++ b/include/trace/events/vmscan.h
>>>> @@ -266,9 +266,10 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
>>>>                unsigned long nr_lumpy_taken,
>>>>                unsigned long nr_lumpy_dirty,
>>>>                unsigned long nr_lumpy_failed,
>>>> -               isolate_mode_t isolate_mode),
>>>> +               isolate_mode_t isolate_mode,
>>>> +               int active, int file),
>>>>
>>>> -       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode),
>>>> +       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode, active, file),
>>>>
>>>>        TP_STRUCT__entry(
>>>>                __field(int, order)
>>>> @@ -279,6 +280,8 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
>>>>                __field(unsigned long, nr_lumpy_dirty)
>>>>                __field(unsigned long, nr_lumpy_failed)
>>>>                __field(isolate_mode_t, isolate_mode)
>>>> +               __field(int, active)
>>>> +               __field(int, file)
>>>>        ),
>>>>
>>>>        TP_fast_assign(
>>>> @@ -290,9 +293,11 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
>>>>                __entry->nr_lumpy_dirty = nr_lumpy_dirty;
>>>>                __entry->nr_lumpy_failed = nr_lumpy_failed;
>>>>                __entry->isolate_mode = isolate_mode;
>>>> +               __entry->active = active;
>>>> +               __entry->file = file;
>>>>        ),
>>>>
>>>> -       TP_printk("isolate_mode=%d order=%d nr_requested=%lu nr_scanned=%lu nr_taken=%lu contig_taken=%lu contig_dirty=%lu contig_failed=%lu",
>>>> +       TP_printk("isolate_mode=%d order=%d nr_requested=%lu nr_scanned=%lu nr_taken=%lu contig_taken=%lu contig_dirty=%lu contig_failed=%lu active=%d file=%d",
>>>>                __entry->isolate_mode,
>>>>                __entry->order,
>>>>                __entry->nr_requested,
>>>> @@ -300,7 +305,9 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
>>>>                __entry->nr_taken,
>>>>                __entry->nr_lumpy_taken,
>>>>                __entry->nr_lumpy_dirty,
>>>> -               __entry->nr_lumpy_failed)
>>>> +               __entry->nr_lumpy_failed,
>>>> +               __entry->active,
>>>> +               __entry->file)
>>>>  );
>>>>
>>>>  DEFINE_EVENT(mm_vmscan_lru_isolate_template, mm_vmscan_lru_isolate,
>>>> @@ -312,9 +319,10 @@ DEFINE_EVENT(mm_vmscan_lru_isolate_template, mm_vmscan_lru_isolate,
>>>>                unsigned long nr_lumpy_taken,
>>>>                unsigned long nr_lumpy_dirty,
>>>>                unsigned long nr_lumpy_failed,
>>>> -               isolate_mode_t isolate_mode),
>>>> +               isolate_mode_t isolate_mode,
>>>> +               int active, int file),
>>>>
>>>> -       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode)
>>>> +       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode, active, file)
>>>>
>>>>  );
>>>>
>>>> @@ -327,9 +335,10 @@ DEFINE_EVENT(mm_vmscan_lru_isolate_template, mm_vmscan_memcg_isolate,
>>>>                unsigned long nr_lumpy_taken,
>>>>                unsigned long nr_lumpy_dirty,
>>>>                unsigned long nr_lumpy_failed,
>>>> -               isolate_mode_t isolate_mode),
>>>> +               isolate_mode_t isolate_mode,
>>>> +               int active, int file),
>>>>
>>>> -       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode)
>>>> +       TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode, active, file)
>>>>
>>>>  );
>>>>
>>>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>>>> index 6aff93c..246fbce 100644
>>>> --- a/mm/memcontrol.c
>>>> +++ b/mm/memcontrol.c
>>>> @@ -1249,7 +1249,7 @@ unsigned long mem_cgroup_isolate_pages(unsigned long nr_to_scan,
>>>>        *scanned = scan;
>>>>
>>>>        trace_mm_vmscan_memcg_isolate(0, nr_to_scan, scan, nr_taken,
>>>> -                                     0, 0, 0, mode);
>>>> +                                     0, 0, 0, mode, active, file);
>>>>
>>>>        return nr_taken;
>>>>  }
>>>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>>>> index f54a05b..97955ca 100644
>>>> --- a/mm/vmscan.c
>>>> +++ b/mm/vmscan.c
>>>> @@ -1103,7 +1103,7 @@ int __isolate_lru_page(struct page *page, isolate_mode_t mode, int file)
>>>>  static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>>>>                struct list_head *src, struct list_head *dst,
>>>>                unsigned long *scanned, int order, isolate_mode_t mode,
>>>> -               int file)
>>>> +               int active, int file)
>>>>  {
>>>>        unsigned long nr_taken = 0;
>>>>        unsigned long nr_lumpy_taken = 0;
>>>> @@ -1221,7 +1221,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>>>>                        nr_to_scan, scan,
>>>>                        nr_taken,
>>>>                        nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed,
>>>> -                       mode);
>>>> +                       mode, active, file);
>>>>        return nr_taken;
>>>>  }
>>>>
>>>> @@ -1237,7 +1237,7 @@ static unsigned long isolate_pages_global(unsigned long nr,
>>>>        if (file)
>>>>                lru += LRU_FILE;
>>>>        return isolate_lru_pages(nr, &z->lru[lru].list, dst, scanned, order,
>>>> -                                                               mode, file);
>>>> +                                                       mode, active, file);
>>>
>>> I guess you want to count exact scanning number of which lru list.
>>> But It's impossible now since we do lumpy reclaim so that trace's
>>> result is mixed by active/inactive list scanning.
>>> And I don't like adding new argument for just trace although it's trivial.
>> yeah, I know we do lumpy reclaim, but it has no hint about whether it is
>> a file or anon lru. So I think we at least need a 'file=[0/1]' in this
>> trace event.
>>>
>>> I think 'mode' is more proper rather than  specific 'active'.
>>> The 'mode' can achieve your goal without passing new argument "active".
>> sorry, but how can we find the real relationship between 'mode' and
>> 'active'? I am not quite familiar with this field. So if you can
>> explicit describe it, I am fine to drop this field.
> 
> 
> mode is following as,
> 
> /* Isolate inactive pages */
> #define ISOLATE_INACTIVE        ((__force fmode_t)0x1)
> /* Isolate active pages */
> #define ISOLATE_ACTIVE          ((__force fmode_t)0x2)
> /* Isolate clean file */
> #define ISOLATE_CLEAN           ((__force fmode_t)0x4)
> /* Isolate unmapped file */
> #define ISOLATE_UNMAPPED        ((__force fmode_t)0x8)
> 
> For example,
> 
> If mode is 0x1, it means we are isolating inactive.
> If mode is 0x2, it means we are isolating active.
> If mode is 0x4|0x2, it mean we are isolating clear pages in active list.
> If mode is 0x8|0x2, it mean we are isolate unmapped page in active list.
cool, thanks for the detailed explanation.
So I will drop this field, and only add 'file' to the trace event.

Thanks
Tao

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH V2] vmscan/trace: Add 'active' and 'file' info to trace_mm_vmscan_lru_isolate.

[Mel Gorman]

On Mon, Dec 12, 2011 at 09:59:20AM +0900, Minchan Kim wrote:
> > <SNIP>
> > @@ -1237,7 +1237,7 @@ static unsigned long isolate_pages_global(unsigned long nr,
> >        if (file)
> >                lru += LRU_FILE;
> >        return isolate_lru_pages(nr, &z->lru[lru].list, dst, scanned, order,
> > -                                                               mode, file);
> > +                                                       mode, active, file);
> 
> I guess you want to count exact scanning number of which lru list.
> But It's impossible now since we do lumpy reclaim so that trace's
> result is mixed by active/inactive list scanning.
> And I don't like adding new argument for just trace although it's trivial.
> 

FWIW, lumpy reclaim is why the trace point does not report the active
or file information. Seeing active==1 does not imply that only active
pages were isolated and mode is already there as Minchan points out.

Similarly, seeing file==1 does not imply that only file-backed
pages were isolated. Any processing script that depends on just this
information would be misleading.  If more information on how much
each LRU was scanned is required, the mm_vmscan_lru_shrink_inactive
tracepoint already reports the number of pages scanned, reclaimed
and whether the pages isolated were anon, file or both so ordinarily
I would suggest using just that.

That said, I see that trace_shrink_flags() is currently misleading as
it should be used sc->order instead of sc->reclaim_mode to determine
if it was file, anon or a mix of both that was isolated. That should
be fixed.

If isolate_lru_pages really needs to export the file information,
then it would be preferable to fix trace_shrink_flags() and use it to
indicate if it was file, anon or a mix of both that was isolated. The
information needed to trace this is not available in isolate_lru_pages
so it would need to be passed down. Even with that, I would also
like to see trace/postprocess/trace-vmscan-postprocess.pl updated to
illustrate how this new information can be used to debug a problem
or at least describe what sort of problem it can debug.


> I think 'mode' is more proper rather than  specific 'active'.
> The 'mode' can achieve your goal without passing new argument "active".
> 

True.

> In addition to, current mmotm has various modes.
> So sometime we can get more specific result rather than vauge 'active'.
> 

Which also means that trace/postprocess/trace-vmscan-postprocess.pl
is not using mm_vmscan_lru_isolate properly as it does not understand
ISOLATE_CLEAN and ISOLATE_UNMAPPED. The impact for the script is that
the scan count it reports will deviate from what /proc/vmstat reports
which is irritating.

-- 
Mel Gorman
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH V2] vmscan/trace: Add 'active' and 'file' info to trace_mm_vmscan_lru_isolate.

[Tao Ma]

Hi Mel,
On 12/12/2011 07:27 PM, Mel Gorman wrote:
> On Mon, Dec 12, 2011 at 09:59:20AM +0900, Minchan Kim wrote:
>>> <SNIP>
>>> @@ -1237,7 +1237,7 @@ static unsigned long isolate_pages_global(unsigned long nr,
>>>        if (file)
>>>                lru += LRU_FILE;
>>>        return isolate_lru_pages(nr, &z->lru[lru].list, dst, scanned, order,
>>> -                                                               mode, file);
>>> +                                                       mode, active, file);
>>
>> I guess you want to count exact scanning number of which lru list.
>> But It's impossible now since we do lumpy reclaim so that trace's
>> result is mixed by active/inactive list scanning.
>> And I don't like adding new argument for just trace although it's trivial.
>>
> 
> FWIW, lumpy reclaim is why the trace point does not report the active
> or file information. Seeing active==1 does not imply that only active
> pages were isolated and mode is already there as Minchan points out.
OK, thanks for the info.
> 
> Similarly, seeing file==1 does not imply that only file-backed
> pages were isolated. Any processing script that depends on just this
> information would be misleading.  If more information on how much
> each LRU was scanned is required, the mm_vmscan_lru_shrink_inactive
> tracepoint already reports the number of pages scanned, reclaimed
> and whether the pages isolated were anon, file or both so ordinarily
> I would suggest using just that.
So how can I tell the isolation list status when we do shrink_active_list?
> 
> That said, I see that trace_shrink_flags() is currently misleading as
> it should be used sc->order instead of sc->reclaim_mode to determine
> if it was file, anon or a mix of both that was isolated. That should
> be fixed.
sure, I will see how to work it out.
> 
> If isolate_lru_pages really needs to export the file information,
> then it would be preferable to fix trace_shrink_flags() and use it to
> indicate if it was file, anon or a mix of both that was isolated. The
> information needed to trace this is not available in isolate_lru_pages
> so it would need to be passed down. Even with that, I would also
> like to see trace/postprocess/trace-vmscan-postprocess.pl updated to
> illustrate how this new information can be used to debug a problem
> or at least describe what sort of problem it can debug.
Sorry, I don't ever know the existence of this script. And I will update
this script in the next try.
> 
> 
>> I think 'mode' is more proper rather than  specific 'active'.
>> The 'mode' can achieve your goal without passing new argument "active".
>>
> 
> True.
> 
>> In addition to, current mmotm has various modes.
>> So sometime we can get more specific result rather than vauge 'active'.
>>
> 
> Which also means that trace/postprocess/trace-vmscan-postprocess.pl
> is not using mm_vmscan_lru_isolate properly as it does not understand
> ISOLATE_CLEAN and ISOLATE_UNMAPPED. The impact for the script is that
> the scan count it reports will deviate from what /proc/vmstat reports
> which is irritating.
Let me see whether I can fix it or not.

Thanks
Tao

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH V2] vmscan/trace: Add 'active' and 'file' info to trace_mm_vmscan_lru_isolate.

[KOSAKI Motohiro]

> From: Tao Ma <boyu.mt@taobao.com>
>
> In trace_mm_vmscan_lru_isolate, we don't output 'active' and 'file'
> information to the trace event and it is a bit inconvenient for the
> user to get the real information(like pasted below).
> mm_vmscan_lru_isolate: isolate_mode=2 order=0 nr_requested=32 nr_scanned=32
> nr_taken=32 contig_taken=0 contig_dirty=0 contig_failed=0
>
> So this patch adds these 2 info to the trace event and it now looks like:
> mm_vmscan_lru_isolate: isolate_mode=2 order=0 nr_requested=32 nr_scanned=32
> nr_taken=32 contig_taken=0 contig_dirty=0 contig_failed=0 active=1 file=0

Acked-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH V2] vmscan/trace: Add 'active' and 'file' info to trace_mm_vmscan_lru_isolate.

[Andrew Morton]

On Sun, 11 Dec 2011 22:46:24 +0800
Tao Ma <tm@tao.ma> wrote:

> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1103,7 +1103,7 @@ int __isolate_lru_page(struct page *page, isolate_mode_t mode, int file)
>  static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>  		struct list_head *src, struct list_head *dst,
>  		unsigned long *scanned, int order, isolate_mode_t mode,
> -		int file)
> +		int active, int file)
>  {
>  	unsigned long nr_taken = 0;
>  	unsigned long nr_lumpy_taken = 0;
> @@ -1221,7 +1221,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>  			nr_to_scan, scan,
>  			nr_taken,
>  			nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed,
> -			mode);
> +			mode, active, file);
>  	return nr_taken;
>  }
>  
> @@ -1237,7 +1237,7 @@ static unsigned long isolate_pages_global(unsigned long nr,
>  	if (file)
>  		lru += LRU_FILE;
>  	return isolate_lru_pages(nr, &z->lru[lru].list, dst, scanned, order,
> -								mode, file);
> +							mode, active, file);
>  }

It would be nice to avoid adding permanent runtime overhead on behalf
of tracing.  It sounds like sending "mode" will satisfy this - please
check that in the v2 patch. 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH v3] vmscan/trace: Add 'file' info to trace_mm_vmscan_lru_isolate.

[Tao Ma]

From: Tao Ma <boyu.mt@taobao.com>

In trace_mm_vmscan_lru_isolate, we don't output 'file'
information to the trace event and it is a bit inconvenient for the
user to get the real information(like pasted below).
mm_vmscan_lru_isolate: isolate_mode=2 order=0 nr_requested=32 nr_scanned=32
nr_taken=32 contig_taken=0 contig_dirty=0 contig_failed=0

'active' can be gotten by analyzing mode(Thanks go to Minchan and Mel),
So this patch adds 'file' to the trace event and it now looks like:
mm_vmscan_lru_isolate: isolate_mode=2 order=0 nr_requested=32 nr_scanned=32
nr_taken=32 contig_taken=0 contig_dirty=0 contig_failed=0 file=0

Cc: Mel Gorman <mel@csn.ul.ie>
Cc: Minchan Kim <minchan.kim@gmail.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Acked-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Signed-off-by: Tao Ma <boyu.mt@taobao.com>
---
 include/trace/events/vmscan.h |   22 ++++++++++++++--------
 mm/memcontrol.c               |    2 +-
 mm/vmscan.c                   |    2 +-
 3 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
index edc4b3d..f64560e 100644
--- a/include/trace/events/vmscan.h
+++ b/include/trace/events/vmscan.h
@@ -266,9 +266,10 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
 		unsigned long nr_lumpy_taken,
 		unsigned long nr_lumpy_dirty,
 		unsigned long nr_lumpy_failed,
-		isolate_mode_t isolate_mode),
+		isolate_mode_t isolate_mode,
+		int file),
 
-	TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode),
+	TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode, file),
 
 	TP_STRUCT__entry(
 		__field(int, order)
@@ -279,6 +280,7 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
 		__field(unsigned long, nr_lumpy_dirty)
 		__field(unsigned long, nr_lumpy_failed)
 		__field(isolate_mode_t, isolate_mode)
+		__field(int, file)
 	),
 
 	TP_fast_assign(
@@ -290,9 +292,10 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
 		__entry->nr_lumpy_dirty = nr_lumpy_dirty;
 		__entry->nr_lumpy_failed = nr_lumpy_failed;
 		__entry->isolate_mode = isolate_mode;
+		__entry->file = file;
 	),
 
-	TP_printk("isolate_mode=%d order=%d nr_requested=%lu nr_scanned=%lu nr_taken=%lu contig_taken=%lu contig_dirty=%lu contig_failed=%lu",
+	TP_printk("isolate_mode=%d order=%d nr_requested=%lu nr_scanned=%lu nr_taken=%lu contig_taken=%lu contig_dirty=%lu contig_failed=%lu file=%d",
 		__entry->isolate_mode,
 		__entry->order,
 		__entry->nr_requested,
@@ -300,7 +303,8 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
 		__entry->nr_taken,
 		__entry->nr_lumpy_taken,
 		__entry->nr_lumpy_dirty,
-		__entry->nr_lumpy_failed)
+		__entry->nr_lumpy_failed,
+		__entry->file)
 );
 
 DEFINE_EVENT(mm_vmscan_lru_isolate_template, mm_vmscan_lru_isolate,
@@ -312,9 +316,10 @@ DEFINE_EVENT(mm_vmscan_lru_isolate_template, mm_vmscan_lru_isolate,
 		unsigned long nr_lumpy_taken,
 		unsigned long nr_lumpy_dirty,
 		unsigned long nr_lumpy_failed,
-		isolate_mode_t isolate_mode),
+		isolate_mode_t isolate_mode,
+		int file),
 
-	TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode)
+	TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode, file)
 
 );
 
@@ -327,9 +332,10 @@ DEFINE_EVENT(mm_vmscan_lru_isolate_template, mm_vmscan_memcg_isolate,
 		unsigned long nr_lumpy_taken,
 		unsigned long nr_lumpy_dirty,
 		unsigned long nr_lumpy_failed,
-		isolate_mode_t isolate_mode),
+		isolate_mode_t isolate_mode,
+		int file),
 
-	TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode)
+	TP_ARGS(order, nr_requested, nr_scanned, nr_taken, nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed, isolate_mode, file)
 
 );
 
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6aff93c..cd0082d 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1249,7 +1249,7 @@ unsigned long mem_cgroup_isolate_pages(unsigned long nr_to_scan,
 	*scanned = scan;
 
 	trace_mm_vmscan_memcg_isolate(0, nr_to_scan, scan, nr_taken,
-				      0, 0, 0, mode);
+				      0, 0, 0, mode, file);
 
 	return nr_taken;
 }
diff --git a/mm/vmscan.c b/mm/vmscan.c
index f54a05b..a444dc0 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1221,7 +1221,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 			nr_to_scan, scan,
 			nr_taken,
 			nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed,
-			mode);
+			mode, file);
 	return nr_taken;
 }
 
-- 
1.7.0.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v3] vmscan/trace: Add 'file' info to trace_mm_vmscan_lru_isolate.

[Andrew Morton]

On Wed, 14 Dec 2011 23:14:53 +0800
Tao Ma <tm@tao.ma> wrote:

> From: Tao Ma <boyu.mt@taobao.com>
> 
> In trace_mm_vmscan_lru_isolate, we don't output 'file'
> information to the trace event and it is a bit inconvenient for the
> user to get the real information(like pasted below).
> mm_vmscan_lru_isolate: isolate_mode=2 order=0 nr_requested=32 nr_scanned=32
> nr_taken=32 contig_taken=0 contig_dirty=0 contig_failed=0
> 
> 'active' can be gotten by analyzing mode(Thanks go to Minchan and Mel),
> So this patch adds 'file' to the trace event and it now looks like:
> mm_vmscan_lru_isolate: isolate_mode=2 order=0 nr_requested=32 nr_scanned=32
> nr_taken=32 contig_taken=0 contig_dirty=0 contig_failed=0 file=0
> 
> ...
>
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1249,7 +1249,7 @@ unsigned long mem_cgroup_isolate_pages(unsigned long nr_to_scan,
>  	*scanned = scan;
>  
>  	trace_mm_vmscan_memcg_isolate(0, nr_to_scan, scan, nr_taken,
> -				      0, 0, 0, mode);
> +				      0, 0, 0, mode, file);
>  
>  	return nr_taken;
>  }

This tracepoint was removed by Johannes's "mm: make per-memcg LRU lists
exclusive".

> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index f54a05b..a444dc0 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1221,7 +1221,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>  			nr_to_scan, scan,
>  			nr_taken,
>  			nr_lumpy_taken, nr_lumpy_dirty, nr_lumpy_failed,
> -			mode);
> +			mode, file);
>  	return nr_taken;
>  }

So this is the only remaining site for that tracepoint.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v3] vmscan/trace: Add 'file' info to trace_mm_vmscan_lru_isolate.

[Minchan Kim]

On Thu, Dec 15, 2011 at 12:14 AM, Tao Ma <tm@tao.ma> wrote:
> From: Tao Ma <boyu.mt@taobao.com>
>
> In trace_mm_vmscan_lru_isolate, we don't output 'file'
> information to the trace event and it is a bit inconvenient for the
> user to get the real information(like pasted below).
> mm_vmscan_lru_isolate: isolate_mode=2 order=0 nr_requested=32 nr_scanned=32
> nr_taken=32 contig_taken=0 contig_dirty=0 contig_failed=0
>
> 'active' can be gotten by analyzing mode(Thanks go to Minchan and Mel),
> So this patch adds 'file' to the trace event and it now looks like:
> mm_vmscan_lru_isolate: isolate_mode=2 order=0 nr_requested=32 nr_scanned=32
> nr_taken=32 contig_taken=0 contig_dirty=0 contig_failed=0 file=0
>
> Cc: Mel Gorman <mel@csn.ul.ie>
> Cc: Minchan Kim <minchan.kim@gmail.com>
> Cc: Rik van Riel <riel@redhat.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Acked-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Signed-off-by: Tao Ma <boyu.mt@taobao.com>

Andrew pointed out that   trace_mm_vmscan_memcg_isolate is out , Otherwise,
Reviewed-by: Minchan Kim <minchan@kernel.org>


-- 
Kind regards,
Minchan Kim

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>