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

From: Minchan Kim
Date: Sun Dec 11 2011 - 19:59:20 EST


Hi Tao,

On Sun, Dec 11, 2011 at 11:46 PM, Tao Ma <tm@xxxxxx> wrote:
> From: Tao Ma <boyu.mt@xxxxxxxxxx>
>
> 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@xxxxxxxxx>
> Cc: Rik van Riel <riel@xxxxxxxxxx>
> Cc: Johannes Weiner <hannes@xxxxxxxxxxx>
> Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
> Cc: KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx>
> Cc: Andrea Arcangeli <aarcange@xxxxxxxxxx>
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Tao Ma <boyu.mt@xxxxxxxxxx>
> ---
> Â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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/