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

Groups > linux.kernel > #1547996 > unrolled thread

[PATCH 0/7] vm, vmscan: enahance vmscan tracepoints

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/7] vm, vmscan: enahance vmscan tracepoints Michal Hocko <mhocko@kernel.org> - 2016-12-28 16:40 +0100
    [PATCH 7/7] mm, vmscan: add mm_vmscan_inactive_list_is_low tracepoint Michal Hocko <mhocko@kernel.org> - 2016-12-28 16:40 +0100
      Re: [PATCH 7/7] mm, vmscan: add mm_vmscan_inactive_list_is_low tracepoint "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2016-12-29 09:30 +0100
    [PATCH 1/7] mm, vmscan: remove unused mm_vmscan_memcg_isolate Michal Hocko <mhocko@kernel.org> - 2016-12-28 16:40 +0100
      Re: [PATCH 1/7] mm, vmscan: remove unused mm_vmscan_memcg_isolate "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2016-12-29 08:50 +0100
    [PATCH 5/7] mm, vmscan: extract shrink_page_list reclaim counters into a struct Michal Hocko <mhocko@kernel.org> - 2016-12-28 16:40 +0100
      Re: [PATCH 5/7] mm, vmscan: extract shrink_page_list reclaim counters into a struct "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2016-12-29 09:10 +0100
    [PATCH 6/7] mm, vmscan: enhance mm_vmscan_lru_shrink_inactive tracepoint Michal Hocko <mhocko@kernel.org> - 2016-12-28 16:40 +0100
      Re: [PATCH 6/7] mm, vmscan: enhance mm_vmscan_lru_shrink_inactive tracepoint "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2016-12-29 09:20 +0100
    [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate tracepoint Michal Hocko <mhocko@kernel.org> - 2016-12-28 16:40 +0100
      Re: [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate  tracepoint Nikolay Borisov <n.borisov.lkml@gmail.com> - 2016-12-28 17:00 +0100
        Re: [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate  tracepoint Michal Hocko <mhocko@kernel.org> - 2016-12-28 17:10 +0100
          Re: [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate  tracepoint Nikolay Borisov <n.borisov.lkml@gmail.com> - 2016-12-28 17:50 +0100
            Re: [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate  tracepoint Michal Hocko <mhocko@kernel.org> - 2016-12-28 18:00 +0100
      Re: [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate  tracepoint Minchan Kim <minchan@kernel.org> - 2016-12-29 07:10 +0100
        Re: [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate  tracepoint Michal Hocko <mhocko@kernel.org> - 2016-12-29 09:00 +0100
          Re: [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate  tracepoint Minchan Kim <minchan@kernel.org> - 2016-12-30 03:00 +0100
            Re: [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate  tracepoint Michal Hocko <mhocko@kernel.org> - 2016-12-30 10:40 +0100
      Re: [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate tracepoint "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2016-12-29 09:00 +0100
      Re: [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate  tracepoint Vlastimil Babka <vbabka@suse.cz> - 2017-01-03 18:10 +0100
    [PATCH 3/7] mm, vmscan: show the number of skipped pages in mm_vmscan_lru_isolate Michal Hocko <mhocko@kernel.org> - 2016-12-28 16:40 +0100
      Re: [PATCH 3/7] mm, vmscan: show the number of skipped pages in  mm_vmscan_lru_isolate Minchan Kim <minchan@kernel.org> - 2016-12-29 07:00 +0100
      Re: [PATCH 3/7] mm, vmscan: show the number of skipped pages in mm_vmscan_lru_isolate "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2016-12-29 09:00 +0100
      Re: [PATCH 3/7] mm, vmscan: show the number of skipped pages in  mm_vmscan_lru_isolate Vlastimil Babka <vbabka@suse.cz> - 2017-01-03 18:30 +0100
    [PATCH 2/7] mm, vmscan: add active list aging tracepoint Michal Hocko <mhocko@kernel.org> - 2016-12-28 16:40 +0100
      Re: [PATCH 2/7] mm, vmscan: add active list aging tracepoint Minchan Kim <minchan@kernel.org> - 2016-12-29 06:40 +0100
        Re: [PATCH 2/7] mm, vmscan: add active list aging tracepoint Michal Hocko <mhocko@kernel.org> - 2016-12-29 09:00 +0100
          Re: [PATCH 2/7] mm, vmscan: add active list aging tracepoint Minchan Kim <minchan@kernel.org> - 2016-12-30 02:50 +0100
            Re: [PATCH 2/7] mm, vmscan: add active list aging tracepoint Michal Hocko <mhocko@kernel.org> - 2016-12-30 10:30 +0100
              Re: [PATCH 2/7] mm, vmscan: add active list aging tracepoint "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2016-12-30 10:50 +0100
              Re: [PATCH 2/7] mm, vmscan: add active list aging tracepoint Minchan Kim <minchan@kernel.org> - 2016-12-30 17:10 +0100
                Re: [PATCH 2/7] mm, vmscan: add active list aging tracepoint Michal Hocko <mhocko@kernel.org> - 2016-12-30 17:40 +0100
                  Re: [PATCH 2/7] mm, vmscan: add active list aging tracepoint Michal Hocko <mhocko@kernel.org> - 2016-12-30 18:40 +0100
                  Re: [PATCH 2/7] mm, vmscan: add active list aging tracepoint Minchan Kim <minchan@kernel.org> - 2017-01-03 06:10 +0100
                    Re: [PATCH 2/7] mm, vmscan: add active list aging tracepoint Michal Hocko <mhocko@kernel.org> - 2017-01-03 09:30 +0100
      Re: [PATCH 2/7] mm, vmscan: add active list aging tracepoint "Hillf Danton" <hillf.zj@alibaba-inc.com> - 2016-12-29 09:00 +0100
    Re: [PATCH 0/7] vm, vmscan: enahance vmscan tracepoints Mel Gorman <mgorman@suse.de> - 2016-12-30 10:20 +0100
      Re: [PATCH 0/7] vm, vmscan: enahance vmscan tracepoints Michal Hocko <mhocko@kernel.org> - 2016-12-30 10:40 +0100
        Re: [PATCH 0/7] vm, vmscan: enahance vmscan tracepoints Mel Gorman <mgorman@suse.de> - 2016-12-30 11:30 +0100

Page 1 of 2  [1] 2  Next page →

#1547996 — [PATCH 0/7] vm, vmscan: enahance vmscan tracepoints

Hi,
while debugging [1] I've realized that there is some room for
improvements in the tracepoints set we offer currently. I had hard times
to make any conclusion from the existing ones. The resulting problem
turned out to be active list aging [2] and we are missing at least two
tracepoints to debug such a problem.

Some existing tracepoints could export more information to see _why_ the
reclaim progress cannot be made not only _how much_ we could reclaim.
The later could be seen quite reasonably from the vmstat counters
already. It can be argued that we are showing too many implementation
details in those tracepoints but I consider them way too lowlevel
already to be usable by any kernel independent userspace. I would be
_really_ surprised if anything but debugging tools have used them.

Any feedback is highly appreciated.

[1] http://lkml.kernel.org/r/20161215225702.GA27944@boerne.fritz.box
[2] http://lkml.kernel.org/r/20161223105157.GB23109@dhcp22.suse.cz

[toc] | [next] | [standalone]

#1547997 — [PATCH 7/7] mm, vmscan: add mm_vmscan_inactive_list_is_low tracepoint

From: Michal Hocko <mhocko@suse.com>

Currently we have tracepoints for both active and inactive LRU lists
reclaim but we do not have any which would tell us why we we decided to
age the active list. Without that it is quite hard to diagnose
active/inactive lists balancing. Add mm_vmscan_inactive_list_is_low
tracepoint to tell us this information.

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 include/trace/events/vmscan.h | 40 ++++++++++++++++++++++++++++++++++++++++
 mm/vmscan.c                   | 23 ++++++++++++++---------
 2 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
index d27606f27af7..02c038c570a9 100644
--- a/include/trace/events/vmscan.h
+++ b/include/trace/events/vmscan.h
@@ -15,6 +15,7 @@
 #define RECLAIM_WB_MIXED	0x0010u
 #define RECLAIM_WB_SYNC		0x0004u /* Unused, all reclaim async */
 #define RECLAIM_WB_ASYNC	0x0008u
+#define RECLAIM_WB_LRU		(RECLAIM_WB_ANON|RECLAIM_WB_FILE)
 
 #define show_reclaim_flags(flags)				\
 	(flags) ? __print_flags(flags, "|",			\
@@ -436,6 +437,45 @@ TRACE_EVENT(mm_vmscan_lru_shrink_active,
 		show_reclaim_flags(__entry->reclaim_flags))
 );
 
+TRACE_EVENT(mm_vmscan_inactive_list_is_low,
+
+	TP_PROTO(int nid, int reclaim_idx,
+		unsigned long total_inactive, unsigned long inactive,
+		unsigned long total_active, unsigned long active,
+		unsigned long ratio, int file),
+
+	TP_ARGS(nid, reclaim_idx, total_inactive, inactive, total_active, active, ratio, file),
+
+	TP_STRUCT__entry(
+		__field(int, nid)
+		__field(int, reclaim_idx)
+		__field(unsigned long, total_inactive)
+		__field(unsigned long, inactive)
+		__field(unsigned long, total_active)
+		__field(unsigned long, active)
+		__field(unsigned long, ratio)
+		__field(int, reclaim_flags)
+	),
+
+	TP_fast_assign(
+		__entry->nid = nid;
+		__entry->reclaim_idx = reclaim_idx;
+		__entry->total_inactive = total_inactive;
+		__entry->inactive = inactive;
+		__entry->total_active = total_active;
+		__entry->active = active;
+		__entry->ratio = ratio;
+		__entry->reclaim_flags = trace_shrink_flags(file) & RECLAIM_WB_LRU;
+	),
+
+	TP_printk("nid=%d reclaim_idx=%d total_inactive=%ld inactive=%ld total_active=%ld active=%ld ratio=%ld flags=%s",
+		__entry->nid,
+		__entry->reclaim_idx,
+		__entry->total_inactive, __entry->inactive,
+		__entry->total_active, __entry->active,
+		__entry->ratio,
+		show_reclaim_flags(__entry->reclaim_flags))
+);
 #endif /* _TRACE_VMSCAN_H */
 
 /* This part must be outside protection */
diff --git a/mm/vmscan.c b/mm/vmscan.c
index a701bdd6334a..8021401213e0 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2041,11 +2041,11 @@ static void shrink_active_list(unsigned long nr_to_scan,
  *   10TB     320        32GB
  */
 static bool inactive_list_is_low(struct lruvec *lruvec, bool file,
-						struct scan_control *sc)
+						struct scan_control *sc, bool trace)
 {
 	unsigned long inactive_ratio;
-	unsigned long inactive;
-	unsigned long active;
+	unsigned long total_inactive, inactive;
+	unsigned long total_active, active;
 	unsigned long gb;
 	struct pglist_data *pgdat = lruvec_pgdat(lruvec);
 	int zid;
@@ -2057,8 +2057,8 @@ static bool inactive_list_is_low(struct lruvec *lruvec, bool file,
 	if (!file && !total_swap_pages)
 		return false;
 
-	inactive = lruvec_lru_size(lruvec, file * LRU_FILE);
-	active = lruvec_lru_size(lruvec, file * LRU_FILE + LRU_ACTIVE);
+	total_inactive = inactive = lruvec_lru_size(lruvec, file * LRU_FILE);
+	total_active = active = lruvec_lru_size(lruvec, file * LRU_FILE + LRU_ACTIVE);
 
 	/*
 	 * For zone-constrained allocations, it is necessary to check if
@@ -2087,6 +2087,11 @@ static bool inactive_list_is_low(struct lruvec *lruvec, bool file,
 	else
 		inactive_ratio = 1;
 
+	if (trace)
+		trace_mm_vmscan_inactive_list_is_low(pgdat->node_id,
+				sc->reclaim_idx,
+				total_inactive, inactive,
+				total_active, active, inactive_ratio, file);
 	return inactive * inactive_ratio < active;
 }
 
@@ -2094,7 +2099,7 @@ static unsigned long shrink_list(enum lru_list lru, unsigned long nr_to_scan,
 				 struct lruvec *lruvec, struct scan_control *sc)
 {
 	if (is_active_lru(lru)) {
-		if (inactive_list_is_low(lruvec, is_file_lru(lru), sc))
+		if (inactive_list_is_low(lruvec, is_file_lru(lru), sc, true))
 			shrink_active_list(nr_to_scan, lruvec, sc, lru);
 		return 0;
 	}
@@ -2225,7 +2230,7 @@ static void get_scan_count(struct lruvec *lruvec, struct mem_cgroup *memcg,
 	 * lruvec even if it has plenty of old anonymous pages unless the
 	 * system is under heavy pressure.
 	 */
-	if (!inactive_list_is_low(lruvec, true, sc) &&
+	if (!inactive_list_is_low(lruvec, true, sc, false) &&
 	    lruvec_lru_size(lruvec, LRU_INACTIVE_FILE) >> sc->priority) {
 		scan_balance = SCAN_FILE;
 		goto out;
@@ -2450,7 +2455,7 @@ static void shrink_node_memcg(struct pglist_data *pgdat, struct mem_cgroup *memc
 	 * Even if we did not try to evict anon pages at all, we want to
 	 * rebalance the anon lru active/inactive ratio.
 	 */
-	if (inactive_list_is_low(lruvec, false, sc))
+	if (inactive_list_is_low(lruvec, false, sc, true))
 		shrink_active_list(SWAP_CLUSTER_MAX, lruvec,
 				   sc, LRU_ACTIVE_ANON);
 }
@@ -3100,7 +3105,7 @@ static void age_active_anon(struct pglist_data *pgdat,
 	do {
 		struct lruvec *lruvec = mem_cgroup_lruvec(pgdat, memcg);
 
-		if (inactive_list_is_low(lruvec, false, sc))
+		if (inactive_list_is_low(lruvec, false, sc, true))
 			shrink_active_list(SWAP_CLUSTER_MAX, lruvec,
 					   sc, LRU_ACTIVE_ANON);
 
-- 
2.10.2

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

#1548234 — Re: [PATCH 7/7] mm, vmscan: add mm_vmscan_inactive_list_is_low tracepoint

On Wednesday, December 28, 2016 11:31 PM Michal Hocko wrote: 
> From: Michal Hocko <mhocko@suse.com>
> 
> Currently we have tracepoints for both active and inactive LRU lists
> reclaim but we do not have any which would tell us why we we decided to
> age the active list. Without that it is quite hard to diagnose
> active/inactive lists balancing. Add mm_vmscan_inactive_list_is_low
> tracepoint to tell us this information.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
Acked-by: Hillf Danton <hillf.zj@alibaba-inc.com>

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

#1547998 — [PATCH 1/7] mm, vmscan: remove unused mm_vmscan_memcg_isolate

From: Michal Hocko <mhocko@suse.com>

the trace point is not used since 925b7673cce3 ("mm: make per-memcg LRU
lists exclusive") so it can be removed.

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 include/trace/events/vmscan.h | 31 +------------------------------
 1 file changed, 1 insertion(+), 30 deletions(-)

diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
index c88fd0934e7e..39bad8921ca1 100644
--- a/include/trace/events/vmscan.h
+++ b/include/trace/events/vmscan.h
@@ -269,8 +269,7 @@ TRACE_EVENT(mm_shrink_slab_end,
 		__entry->retval)
 );
 
-DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
-
+TRACE_EVENT(mm_vmscan_lru_isolate,
 	TP_PROTO(int classzone_idx,
 		int order,
 		unsigned long nr_requested,
@@ -311,34 +310,6 @@ DECLARE_EVENT_CLASS(mm_vmscan_lru_isolate_template,
 		__entry->file)
 );
 
-DEFINE_EVENT(mm_vmscan_lru_isolate_template, mm_vmscan_lru_isolate,
-
-	TP_PROTO(int classzone_idx,
-		int order,
-		unsigned long nr_requested,
-		unsigned long nr_scanned,
-		unsigned long nr_taken,
-		isolate_mode_t isolate_mode,
-		int file),
-
-	TP_ARGS(classzone_idx, order, nr_requested, nr_scanned, nr_taken, isolate_mode, file)
-
-);
-
-DEFINE_EVENT(mm_vmscan_lru_isolate_template, mm_vmscan_memcg_isolate,
-
-	TP_PROTO(int classzone_idx,
-		int order,
-		unsigned long nr_requested,
-		unsigned long nr_scanned,
-		unsigned long nr_taken,
-		isolate_mode_t isolate_mode,
-		int file),
-
-	TP_ARGS(classzone_idx, order, nr_requested, nr_scanned, nr_taken, isolate_mode, file)
-
-);
-
 TRACE_EVENT(mm_vmscan_writepage,
 
 	TP_PROTO(struct page *page),
-- 
2.10.2

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

#1548216 — Re: [PATCH 1/7] mm, vmscan: remove unused mm_vmscan_memcg_isolate

On Wednesday, December 28, 2016 11:30 PM Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
> 
> the trace point is not used since 925b7673cce3 ("mm: make per-memcg LRU
> lists exclusive") so it can be removed.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
Acked-by: Hillf Danton <hillf.zj@alibaba-inc.com> 

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

#1547999 — [PATCH 5/7] mm, vmscan: extract shrink_page_list reclaim counters into a struct

From: Michal Hocko <mhocko@suse.com>

shrink_page_list returns quite some counters back to its caller. Extract
the existing 5 into struct reclaim_stat because this makes the code
easier to follow and also allows further counters to be returned.

While we are at it, make all of them unsigned rather than unsigned long
as we do not really need full 64b for them (we never scan more than
SWAP_CLUSTER_MAX pages at once). This should reduce some stack space.

This patch shouldn't introduce any functional change.

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 mm/vmscan.c | 61 ++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 30 insertions(+), 31 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 3f0774f30a42..f6f2d828968c 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -902,6 +902,14 @@ static void page_check_dirty_writeback(struct page *page,
 		mapping->a_ops->is_dirty_writeback(page, dirty, writeback);
 }
 
+struct reclaim_stat {
+	unsigned nr_dirty;
+	unsigned nr_unqueued_dirty;
+	unsigned nr_congested;
+	unsigned nr_writeback;
+	unsigned nr_immediate;
+};
+
 /*
  * shrink_page_list() returns the number of reclaimed pages
  */
@@ -909,22 +917,18 @@ static unsigned long shrink_page_list(struct list_head *page_list,
 				      struct pglist_data *pgdat,
 				      struct scan_control *sc,
 				      enum ttu_flags ttu_flags,
-				      unsigned long *ret_nr_dirty,
-				      unsigned long *ret_nr_unqueued_dirty,
-				      unsigned long *ret_nr_congested,
-				      unsigned long *ret_nr_writeback,
-				      unsigned long *ret_nr_immediate,
+				      struct reclaim_stat *stat,
 				      bool force_reclaim)
 {
 	LIST_HEAD(ret_pages);
 	LIST_HEAD(free_pages);
 	int pgactivate = 0;
-	unsigned long nr_unqueued_dirty = 0;
-	unsigned long nr_dirty = 0;
-	unsigned long nr_congested = 0;
-	unsigned long nr_reclaimed = 0;
-	unsigned long nr_writeback = 0;
-	unsigned long nr_immediate = 0;
+	unsigned nr_unqueued_dirty = 0;
+	unsigned nr_dirty = 0;
+	unsigned nr_congested = 0;
+	unsigned nr_reclaimed = 0;
+	unsigned nr_writeback = 0;
+	unsigned nr_immediate = 0;
 
 	cond_resched();
 
@@ -1266,11 +1270,13 @@ static unsigned long shrink_page_list(struct list_head *page_list,
 	list_splice(&ret_pages, page_list);
 	count_vm_events(PGACTIVATE, pgactivate);
 
-	*ret_nr_dirty += nr_dirty;
-	*ret_nr_congested += nr_congested;
-	*ret_nr_unqueued_dirty += nr_unqueued_dirty;
-	*ret_nr_writeback += nr_writeback;
-	*ret_nr_immediate += nr_immediate;
+	if (stat) {
+		stat->nr_dirty = nr_dirty;
+		stat->nr_congested = nr_congested;
+		stat->nr_unqueued_dirty = nr_unqueued_dirty;
+		stat->nr_writeback = nr_writeback;
+		stat->nr_immediate = nr_immediate;
+	}
 	return nr_reclaimed;
 }
 
@@ -1282,7 +1288,7 @@ unsigned long reclaim_clean_pages_from_list(struct zone *zone,
 		.priority = DEF_PRIORITY,
 		.may_unmap = 1,
 	};
-	unsigned long ret, dummy1, dummy2, dummy3, dummy4, dummy5;
+	unsigned long ret;
 	struct page *page, *next;
 	LIST_HEAD(clean_pages);
 
@@ -1295,8 +1301,7 @@ unsigned long reclaim_clean_pages_from_list(struct zone *zone,
 	}
 
 	ret = shrink_page_list(&clean_pages, zone->zone_pgdat, &sc,
-			TTU_UNMAP|TTU_IGNORE_ACCESS,
-			&dummy1, &dummy2, &dummy3, &dummy4, &dummy5, true);
+			TTU_UNMAP|TTU_IGNORE_ACCESS, NULL, true);
 	list_splice(&clean_pages, page_list);
 	mod_node_page_state(zone->zone_pgdat, NR_ISOLATED_FILE, -ret);
 	return ret;
@@ -1696,11 +1701,7 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,
 	unsigned long nr_scanned;
 	unsigned long nr_reclaimed = 0;
 	unsigned long nr_taken;
-	unsigned long nr_dirty = 0;
-	unsigned long nr_congested = 0;
-	unsigned long nr_unqueued_dirty = 0;
-	unsigned long nr_writeback = 0;
-	unsigned long nr_immediate = 0;
+	struct reclaim_stat stat = {};
 	isolate_mode_t isolate_mode = 0;
 	int file = is_file_lru(lru);
 	struct pglist_data *pgdat = lruvec_pgdat(lruvec);
@@ -1745,9 +1746,7 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,
 		return 0;
 
 	nr_reclaimed = shrink_page_list(&page_list, pgdat, sc, TTU_UNMAP,
-				&nr_dirty, &nr_unqueued_dirty, &nr_congested,
-				&nr_writeback, &nr_immediate,
-				false);
+				&stat, false);
 
 	spin_lock_irq(&pgdat->lru_lock);
 
@@ -1781,7 +1780,7 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,
 	 * of pages under pages flagged for immediate reclaim and stall if any
 	 * are encountered in the nr_immediate check below.
 	 */
-	if (nr_writeback && nr_writeback == nr_taken)
+	if (stat.nr_writeback && stat.nr_writeback == nr_taken)
 		set_bit(PGDAT_WRITEBACK, &pgdat->flags);
 
 	/*
@@ -1793,7 +1792,7 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,
 		 * Tag a zone as congested if all the dirty pages scanned were
 		 * backed by a congested BDI and wait_iff_congested will stall.
 		 */
-		if (nr_dirty && nr_dirty == nr_congested)
+		if (stat.nr_dirty && stat.nr_dirty == stat.nr_congested)
 			set_bit(PGDAT_CONGESTED, &pgdat->flags);
 
 		/*
@@ -1802,7 +1801,7 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,
 		 * the pgdat PGDAT_DIRTY and kswapd will start writing pages from
 		 * reclaim context.
 		 */
-		if (nr_unqueued_dirty == nr_taken)
+		if (stat.nr_unqueued_dirty == nr_taken)
 			set_bit(PGDAT_DIRTY, &pgdat->flags);
 
 		/*
@@ -1811,7 +1810,7 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,
 		 * that pages are cycling through the LRU faster than
 		 * they are written so also forcibly stall.
 		 */
-		if (nr_immediate && current_may_throttle())
+		if (stat.nr_immediate && current_may_throttle())
 			congestion_wait(BLK_RW_ASYNC, HZ/10);
 	}
 
-- 
2.10.2

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

#1548229 — Re: [PATCH 5/7] mm, vmscan: extract shrink_page_list reclaim counters into a struct

On Wednesday, December 28, 2016 11:31 PM Michal Hocko wrote: 
> From: Michal Hocko <mhocko@suse.com>
> 
> shrink_page_list returns quite some counters back to its caller. Extract
> the existing 5 into struct reclaim_stat because this makes the code
> easier to follow and also allows further counters to be returned.
> 
> While we are at it, make all of them unsigned rather than unsigned long
> as we do not really need full 64b for them (we never scan more than
> SWAP_CLUSTER_MAX pages at once). This should reduce some stack space.
> 
> This patch shouldn't introduce any functional change.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
Acked-by: Hillf Danton <hillf.zj@alibaba-inc.com> 

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

#1548000 — [PATCH 6/7] mm, vmscan: enhance mm_vmscan_lru_shrink_inactive tracepoint

From: Michal Hocko <mhocko@suse.com>

mm_vmscan_lru_shrink_inactive will currently report the number of
scanned and reclaimed pages. This doesn't give us an idea how the
reclaim went except for the overall effectiveness though. Export
and show other counters which will tell us why we couldn't reclaim
some pages.
	- nr_dirty, nr_writeback, nr_congested and nr_immediate tells
	  us how many pages are blocked due to IO
	- nr_activate tells us how many pages were moved to the active
	  list
	- nr_ref_keep reports how many pages are kept on the LRU due
	  to references (mostly for the file pages which are about to
	  go for another round through the inactive list)
	- nr_unmap_fail - how many pages failed to unmap

All these are rather low level so they might change in future but the
tracepoint is already implementation specific so no tools should be
depending on its stability.

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 include/trace/events/vmscan.h | 29 ++++++++++++++++++++++++++---
 mm/vmscan.c                   | 14 ++++++++++++++
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
index cc0b4c456c78..d27606f27af7 100644
--- a/include/trace/events/vmscan.h
+++ b/include/trace/events/vmscan.h
@@ -348,14 +348,27 @@ TRACE_EVENT(mm_vmscan_lru_shrink_inactive,
 
 	TP_PROTO(int nid,
 		unsigned long nr_scanned, unsigned long nr_reclaimed,
+		unsigned long nr_dirty, unsigned long nr_writeback,
+		unsigned long nr_congested, unsigned long nr_immediate,
+		unsigned long nr_activate, unsigned long nr_ref_keep,
+		unsigned long nr_unmap_fail,
 		int priority, int file),
 
-	TP_ARGS(nid, nr_scanned, nr_reclaimed, priority, file),
+	TP_ARGS(nid, nr_scanned, nr_reclaimed, nr_dirty, nr_writeback,
+		nr_congested, nr_immediate, nr_activate, nr_ref_keep,
+		nr_unmap_fail, priority, file),
 
 	TP_STRUCT__entry(
 		__field(int, nid)
 		__field(unsigned long, nr_scanned)
 		__field(unsigned long, nr_reclaimed)
+		__field(unsigned long, nr_dirty)
+		__field(unsigned long, nr_writeback)
+		__field(unsigned long, nr_congested)
+		__field(unsigned long, nr_immediate)
+		__field(unsigned long, nr_activate)
+		__field(unsigned long, nr_ref_keep)
+		__field(unsigned long, nr_unmap_fail)
 		__field(int, priority)
 		__field(int, reclaim_flags)
 	),
@@ -364,14 +377,24 @@ TRACE_EVENT(mm_vmscan_lru_shrink_inactive,
 		__entry->nid = nid;
 		__entry->nr_scanned = nr_scanned;
 		__entry->nr_reclaimed = nr_reclaimed;
+		__entry->nr_dirty = nr_dirty;
+		__entry->nr_writeback = nr_writeback;
+		__entry->nr_congested = nr_congested;
+		__entry->nr_immediate = nr_immediate;
+		__entry->nr_activate = nr_activate;
+		__entry->nr_ref_keep = nr_ref_keep;
+		__entry->nr_unmap_fail = nr_unmap_fail;
 		__entry->priority = priority;
 		__entry->reclaim_flags = trace_shrink_flags(file);
 	),
 
-	TP_printk("nid=%d nr_scanned=%ld nr_reclaimed=%ld priority=%d flags=%s",
+	TP_printk("nid=%d nr_scanned=%ld nr_reclaimed=%ld nr_dirty=%ld nr_writeback=%ld nr_congested=%ld nr_immediate=%ld nr_activate=%ld nr_ref_keep=%ld nr_unmap_fail=%ld priority=%d flags=%s",
 		__entry->nid,
 		__entry->nr_scanned, __entry->nr_reclaimed,
-		__entry->priority,
+		__entry->nr_dirty, __entry->nr_writeback,
+		__entry->nr_congested, __entry->nr_immediate,
+		__entry->nr_activate, __entry->nr_ref_keep,
+		__entry->nr_unmap_fail, __entry->priority,
 		show_reclaim_flags(__entry->reclaim_flags))
 );
 
diff --git a/mm/vmscan.c b/mm/vmscan.c
index f6f2d828968c..a701bdd6334a 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -908,6 +908,9 @@ struct reclaim_stat {
 	unsigned nr_congested;
 	unsigned nr_writeback;
 	unsigned nr_immediate;
+	unsigned nr_activate;
+	unsigned nr_ref_keep;
+	unsigned nr_unmap_fail;
 };
 
 /*
@@ -929,6 +932,8 @@ static unsigned long shrink_page_list(struct list_head *page_list,
 	unsigned nr_reclaimed = 0;
 	unsigned nr_writeback = 0;
 	unsigned nr_immediate = 0;
+	unsigned nr_ref_keep = 0;
+	unsigned nr_unmap_fail = 0;
 
 	cond_resched();
 
@@ -1067,6 +1072,7 @@ static unsigned long shrink_page_list(struct list_head *page_list,
 		case PAGEREF_ACTIVATE:
 			goto activate_locked;
 		case PAGEREF_KEEP:
+			nr_ref_keep++;
 			goto keep_locked;
 		case PAGEREF_RECLAIM:
 		case PAGEREF_RECLAIM_CLEAN:
@@ -1104,6 +1110,7 @@ static unsigned long shrink_page_list(struct list_head *page_list,
 				(ttu_flags | TTU_BATCH_FLUSH | TTU_LZFREE) :
 				(ttu_flags | TTU_BATCH_FLUSH))) {
 			case SWAP_FAIL:
+				nr_unmap_fail++;
 				goto activate_locked;
 			case SWAP_AGAIN:
 				goto keep_locked;
@@ -1276,6 +1283,9 @@ static unsigned long shrink_page_list(struct list_head *page_list,
 		stat->nr_unqueued_dirty = nr_unqueued_dirty;
 		stat->nr_writeback = nr_writeback;
 		stat->nr_immediate = nr_immediate;
+		stat->nr_activate = pgactivate;
+		stat->nr_ref_keep = nr_ref_keep;
+		stat->nr_unmap_fail = nr_unmap_fail;
 	}
 	return nr_reclaimed;
 }
@@ -1825,6 +1835,10 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,
 
 	trace_mm_vmscan_lru_shrink_inactive(pgdat->node_id,
 			nr_scanned, nr_reclaimed,
+			stat.nr_dirty,  stat.nr_writeback,
+			stat.nr_congested, stat.nr_immediate,
+			stat.nr_activate, stat.nr_ref_keep,
+			stat.nr_unmap_fail,
 			sc->priority, file);
 	return nr_reclaimed;
 }
-- 
2.10.2

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

#1548232 — Re: [PATCH 6/7] mm, vmscan: enhance mm_vmscan_lru_shrink_inactive tracepoint

On Wednesday, December 28, 2016 11:31 PM Michal Hocko wrote: 
> From: Michal Hocko <mhocko@suse.com>
> 
> mm_vmscan_lru_shrink_inactive will currently report the number of
> scanned and reclaimed pages. This doesn't give us an idea how the
> reclaim went except for the overall effectiveness though. Export
> and show other counters which will tell us why we couldn't reclaim
> some pages.
> 	- nr_dirty, nr_writeback, nr_congested and nr_immediate tells
> 	  us how many pages are blocked due to IO
> 	- nr_activate tells us how many pages were moved to the active
> 	  list
> 	- nr_ref_keep reports how many pages are kept on the LRU due
> 	  to references (mostly for the file pages which are about to
> 	  go for another round through the inactive list)
> 	- nr_unmap_fail - how many pages failed to unmap
> 
> All these are rather low level so they might change in future but the
> tracepoint is already implementation specific so no tools should be
> depending on its stability.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
Acked-by: Hillf Danton <hillf.zj@alibaba-inc.com>

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

#1548001 — [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate tracepoint

From: Michal Hocko <mhocko@suse.com>

mm_vmscan_lru_isolate currently prints only whether the LRU we isolate
from is file or anonymous but we do not know which LRU this is. It is
useful to know whether the list is file or anonymous as well. Change
the tracepoint to show symbolic names of the lru rather.

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 include/trace/events/vmscan.h | 20 ++++++++++++++------
 mm/vmscan.c                   |  2 +-
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
index 6af4dae46db2..cc0b4c456c78 100644
--- a/include/trace/events/vmscan.h
+++ b/include/trace/events/vmscan.h
@@ -36,6 +36,14 @@
 		(RECLAIM_WB_ASYNC) \
 	)
 
+#define show_lru_name(lru) \
+	__print_symbolic(lru, \
+			{LRU_INACTIVE_ANON, "LRU_INACTIVE_ANON"}, \
+			{LRU_ACTIVE_ANON, "LRU_ACTIVE_ANON"}, \
+			{LRU_INACTIVE_FILE, "LRU_INACTIVE_FILE"}, \
+			{LRU_ACTIVE_FILE, "LRU_ACTIVE_FILE"}, \
+			{LRU_UNEVICTABLE, "LRU_UNEVICTABLE"})
+
 TRACE_EVENT(mm_vmscan_kswapd_sleep,
 
 	TP_PROTO(int nid),
@@ -277,9 +285,9 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
 		unsigned long nr_skipped,
 		unsigned long nr_taken,
 		isolate_mode_t isolate_mode,
-		int file),
+		int lru),
 
-	TP_ARGS(classzone_idx, order, nr_requested, nr_scanned, nr_skipped, nr_taken, isolate_mode, file),
+	TP_ARGS(classzone_idx, order, nr_requested, nr_scanned, nr_skipped, nr_taken, isolate_mode, lru),
 
 	TP_STRUCT__entry(
 		__field(int, classzone_idx)
@@ -289,7 +297,7 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
 		__field(unsigned long, nr_skipped)
 		__field(unsigned long, nr_taken)
 		__field(isolate_mode_t, isolate_mode)
-		__field(int, file)
+		__field(int, lru)
 	),
 
 	TP_fast_assign(
@@ -300,10 +308,10 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
 		__entry->nr_skipped = nr_skipped;
 		__entry->nr_taken = nr_taken;
 		__entry->isolate_mode = isolate_mode;
-		__entry->file = file;
+		__entry->lru = lru;
 	),
 
-	TP_printk("isolate_mode=%d classzone=%d order=%d nr_requested=%lu nr_scanned=%lu nr_skipped=%lu nr_taken=%lu file=%d",
+	TP_printk("isolate_mode=%d classzone=%d order=%d nr_requested=%lu nr_scanned=%lu nr_skipped=%lu nr_taken=%lu lru=%s",
 		__entry->isolate_mode,
 		__entry->classzone_idx,
 		__entry->order,
@@ -311,7 +319,7 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
 		__entry->nr_scanned,
 		__entry->nr_skipped,
 		__entry->nr_taken,
-		__entry->file)
+		show_lru_name(__entry->lru))
 );
 
 TRACE_EVENT(mm_vmscan_writepage,
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 4f7c0d66d629..3f0774f30a42 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1500,7 +1500,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
 	}
 	*nr_scanned = scan + total_skipped;
 	trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan, scan,
-				    skipped, nr_taken, mode, is_file_lru(lru));
+				    skipped, nr_taken, mode, lru);
 	update_lru_sizes(lruvec, lru, nr_zone_taken, nr_taken);
 	return nr_taken;
 }
-- 
2.10.2

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

#1548010 — Re: [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate tracepoint

On 28.12.2016 17:30, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
> 
> mm_vmscan_lru_isolate currently prints only whether the LRU we isolate
> from is file or anonymous but we do not know which LRU this is. It is
> useful to know whether the list is file or anonymous as well. Change
Maybe you wanted to say whether the list is ACTIVE/INACTIVE ?

> the tracepoint to show symbolic names of the lru rather.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
>  include/trace/events/vmscan.h | 20 ++++++++++++++------
>  mm/vmscan.c                   |  2 +-
>  2 files changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
> index 6af4dae46db2..cc0b4c456c78 100644
> --- a/include/trace/events/vmscan.h
> +++ b/include/trace/events/vmscan.h
> @@ -36,6 +36,14 @@
>  		(RECLAIM_WB_ASYNC) \
>  	)
>  
> +#define show_lru_name(lru) \
> +	__print_symbolic(lru, \
> +			{LRU_INACTIVE_ANON, "LRU_INACTIVE_ANON"}, \
> +			{LRU_ACTIVE_ANON, "LRU_ACTIVE_ANON"}, \
> +			{LRU_INACTIVE_FILE, "LRU_INACTIVE_FILE"}, \
> +			{LRU_ACTIVE_FILE, "LRU_ACTIVE_FILE"}, \
> +			{LRU_UNEVICTABLE, "LRU_UNEVICTABLE"})
> +
>  TRACE_EVENT(mm_vmscan_kswapd_sleep,
>  
>  	TP_PROTO(int nid),
> @@ -277,9 +285,9 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
>  		unsigned long nr_skipped,
>  		unsigned long nr_taken,
>  		isolate_mode_t isolate_mode,
> -		int file),
> +		int lru),
>  
> -	TP_ARGS(classzone_idx, order, nr_requested, nr_scanned, nr_skipped, nr_taken, isolate_mode, file),
> +	TP_ARGS(classzone_idx, order, nr_requested, nr_scanned, nr_skipped, nr_taken, isolate_mode, lru),
>  
>  	TP_STRUCT__entry(
>  		__field(int, classzone_idx)
> @@ -289,7 +297,7 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
>  		__field(unsigned long, nr_skipped)
>  		__field(unsigned long, nr_taken)
>  		__field(isolate_mode_t, isolate_mode)
> -		__field(int, file)
> +		__field(int, lru)
>  	),
>  
>  	TP_fast_assign(
> @@ -300,10 +308,10 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
>  		__entry->nr_skipped = nr_skipped;
>  		__entry->nr_taken = nr_taken;
>  		__entry->isolate_mode = isolate_mode;
> -		__entry->file = file;
> +		__entry->lru = lru;
>  	),
>  
> -	TP_printk("isolate_mode=%d classzone=%d order=%d nr_requested=%lu nr_scanned=%lu nr_skipped=%lu nr_taken=%lu file=%d",
> +	TP_printk("isolate_mode=%d classzone=%d order=%d nr_requested=%lu nr_scanned=%lu nr_skipped=%lu nr_taken=%lu lru=%s",
>  		__entry->isolate_mode,
>  		__entry->classzone_idx,
>  		__entry->order,
> @@ -311,7 +319,7 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
>  		__entry->nr_scanned,
>  		__entry->nr_skipped,
>  		__entry->nr_taken,
> -		__entry->file)
> +		show_lru_name(__entry->lru))
>  );
>  
>  TRACE_EVENT(mm_vmscan_writepage,
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 4f7c0d66d629..3f0774f30a42 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1500,7 +1500,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>  	}
>  	*nr_scanned = scan + total_skipped;
>  	trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan, scan,
> -				    skipped, nr_taken, mode, is_file_lru(lru));
> +				    skipped, nr_taken, mode, lru);
>  	update_lru_sizes(lruvec, lru, nr_zone_taken, nr_taken);
>  	return nr_taken;
>  }
> 

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

#1548013 — Re: [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate tracepoint

On Wed 28-12-16 17:50:31, Nikolay Borisov wrote:
> 
> 
> On 28.12.2016 17:30, Michal Hocko wrote:
> > From: Michal Hocko <mhocko@suse.com>
> > 
> > mm_vmscan_lru_isolate currently prints only whether the LRU we isolate
> > from is file or anonymous but we do not know which LRU this is. It is
> > useful to know whether the list is file or anonymous as well. Change
>
> Maybe you wanted to say whether the list is ACTIVE/INACTIVE ?

You are right. I will update the wording to:
"
mm_vmscan_lru_isolate currently prints only whether the LRU we isolate
from is file or anonymous but we do not know which LRU this is. It is
useful to know whether the list is active or inactive as well as we
use the same function to isolate pages for both of them. Change
the tracepoint to show symbolic names of the lru rather.
"

Does it sound better?

Thanks!
-- 
Michal Hocko
SUSE Labs

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

#1548020 — Re: [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate tracepoint

On 28.12.2016 18:00, Michal Hocko wrote:
> On Wed 28-12-16 17:50:31, Nikolay Borisov wrote:
>>
>>
>> On 28.12.2016 17:30, Michal Hocko wrote:
>>> From: Michal Hocko <mhocko@suse.com>
>>>
>>> mm_vmscan_lru_isolate currently prints only whether the LRU we isolate
>>> from is file or anonymous but we do not know which LRU this is. It is
>>> useful to know whether the list is file or anonymous as well. Change
>>
>> Maybe you wanted to say whether the list is ACTIVE/INACTIVE ?
> 
> You are right. I will update the wording to:
> "
> mm_vmscan_lru_isolate currently prints only whether the LRU we isolate
> from is file or anonymous but we do not know which LRU this is. It is
> useful to know whether the list is active or inactive as well as we
> use the same function to isolate pages for both of them. Change
> the tracepoint to show symbolic names of the lru rather.
> "
> 
> Does it sound better?

It's better. Just one more nit about the " as well as we
use the same function to isolate pages for both of them"

I think this can be reworded better. The way I understand is - it's
better to know whether it's active/inactive since we are using the same
function to do both, correct? If so then then perhaps the following is a
bit more clear:

"
It is useful to know whether the list is active or inactive, since we
are using the same function to isolate pages from both of them and it's
hard to distinguish otherwise.
"

But as I said - it's a minor nit.


> 
> Thanks!
> 

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

#1548024 — Re: [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate tracepoint

On Wed 28-12-16 18:40:16, Nikolay Borisov wrote:
[...]
> "
> It is useful to know whether the list is active or inactive, since we
> are using the same function to isolate pages from both of them and it's
> hard to distinguish otherwise.
> "

OK, updated. Thanks!
-- 
Michal Hocko
SUSE Labs

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

#1548193 — Re: [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate tracepoint

On Wed, Dec 28, 2016 at 04:30:29PM +0100, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
> 
> mm_vmscan_lru_isolate currently prints only whether the LRU we isolate
> from is file or anonymous but we do not know which LRU this is. It is
> useful to know whether the list is file or anonymous as well. Change
> the tracepoint to show symbolic names of the lru rather.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>

Not exactly same with this but idea is almost same.
I used almost same tracepoint to investigate agging(i.e., deactivating) problem
in 32b kernel with node-lru.
It was enough. Namely, I didn't need tracepoint in shrink_active_list like your
first patch.
Your first patch is more straightforwad and information. But as you introduced
this patch, I want to ask in here.
Isn't it enough with this patch without your first one to find a such problem?

Thanks.

> ---
>  include/trace/events/vmscan.h | 20 ++++++++++++++------
>  mm/vmscan.c                   |  2 +-
>  2 files changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
> index 6af4dae46db2..cc0b4c456c78 100644
> --- a/include/trace/events/vmscan.h
> +++ b/include/trace/events/vmscan.h
> @@ -36,6 +36,14 @@
>  		(RECLAIM_WB_ASYNC) \
>  	)
>  
> +#define show_lru_name(lru) \
> +	__print_symbolic(lru, \
> +			{LRU_INACTIVE_ANON, "LRU_INACTIVE_ANON"}, \
> +			{LRU_ACTIVE_ANON, "LRU_ACTIVE_ANON"}, \
> +			{LRU_INACTIVE_FILE, "LRU_INACTIVE_FILE"}, \
> +			{LRU_ACTIVE_FILE, "LRU_ACTIVE_FILE"}, \
> +			{LRU_UNEVICTABLE, "LRU_UNEVICTABLE"})
> +
>  TRACE_EVENT(mm_vmscan_kswapd_sleep,
>  
>  	TP_PROTO(int nid),
> @@ -277,9 +285,9 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
>  		unsigned long nr_skipped,
>  		unsigned long nr_taken,
>  		isolate_mode_t isolate_mode,
> -		int file),
> +		int lru),
>  
> -	TP_ARGS(classzone_idx, order, nr_requested, nr_scanned, nr_skipped, nr_taken, isolate_mode, file),
> +	TP_ARGS(classzone_idx, order, nr_requested, nr_scanned, nr_skipped, nr_taken, isolate_mode, lru),
>  
>  	TP_STRUCT__entry(
>  		__field(int, classzone_idx)
> @@ -289,7 +297,7 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
>  		__field(unsigned long, nr_skipped)
>  		__field(unsigned long, nr_taken)
>  		__field(isolate_mode_t, isolate_mode)
> -		__field(int, file)
> +		__field(int, lru)
>  	),
>  
>  	TP_fast_assign(
> @@ -300,10 +308,10 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
>  		__entry->nr_skipped = nr_skipped;
>  		__entry->nr_taken = nr_taken;
>  		__entry->isolate_mode = isolate_mode;
> -		__entry->file = file;
> +		__entry->lru = lru;
>  	),
>  
> -	TP_printk("isolate_mode=%d classzone=%d order=%d nr_requested=%lu nr_scanned=%lu nr_skipped=%lu nr_taken=%lu file=%d",
> +	TP_printk("isolate_mode=%d classzone=%d order=%d nr_requested=%lu nr_scanned=%lu nr_skipped=%lu nr_taken=%lu lru=%s",
>  		__entry->isolate_mode,
>  		__entry->classzone_idx,
>  		__entry->order,
> @@ -311,7 +319,7 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
>  		__entry->nr_scanned,
>  		__entry->nr_skipped,
>  		__entry->nr_taken,
> -		__entry->file)
> +		show_lru_name(__entry->lru))
>  );
>  
>  TRACE_EVENT(mm_vmscan_writepage,
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 4f7c0d66d629..3f0774f30a42 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1500,7 +1500,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>  	}
>  	*nr_scanned = scan + total_skipped;
>  	trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan, scan,
> -				    skipped, nr_taken, mode, is_file_lru(lru));
> +				    skipped, nr_taken, mode, lru);
>  	update_lru_sizes(lruvec, lru, nr_zone_taken, nr_taken);
>  	return nr_taken;
>  }
> -- 
> 2.10.2
> 
> --
> 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/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

#1548223 — Re: [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate tracepoint

On Thu 29-12-16 15:02:04, Minchan Kim wrote:
> On Wed, Dec 28, 2016 at 04:30:29PM +0100, Michal Hocko wrote:
> > From: Michal Hocko <mhocko@suse.com>
> > 
> > mm_vmscan_lru_isolate currently prints only whether the LRU we isolate
> > from is file or anonymous but we do not know which LRU this is. It is
> > useful to know whether the list is file or anonymous as well. Change
> > the tracepoint to show symbolic names of the lru rather.
> > 
> > Signed-off-by: Michal Hocko <mhocko@suse.com>
> 
> Not exactly same with this but idea is almost same.
> I used almost same tracepoint to investigate agging(i.e., deactivating) problem
> in 32b kernel with node-lru.
> It was enough. Namely, I didn't need tracepoint in shrink_active_list like your
> first patch.
> Your first patch is more straightforwad and information. But as you introduced
> this patch, I want to ask in here.
> Isn't it enough with this patch without your first one to find a such problem?

I assume this should be a reply to
http://lkml.kernel.org/r/20161228153032.10821-8-mhocko@kernel.org, right?
And you are right that for the particular problem it was enough to have
a tracepoint inside inactive_list_is_low and shrink_active_list one
wasn't really needed. On the other hand aging issues are really hard to
debug as well and so I think that both are useful. The first one tell us
_why_ we do aging while the later _how_ we do that.
-- 
Michal Hocko
SUSE Labs

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

#1548427 — Re: [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate tracepoint

On Thu, Dec 29, 2016 at 08:56:49AM +0100, Michal Hocko wrote:
> On Thu 29-12-16 15:02:04, Minchan Kim wrote:
> > On Wed, Dec 28, 2016 at 04:30:29PM +0100, Michal Hocko wrote:
> > > From: Michal Hocko <mhocko@suse.com>
> > > 
> > > mm_vmscan_lru_isolate currently prints only whether the LRU we isolate
> > > from is file or anonymous but we do not know which LRU this is. It is
> > > useful to know whether the list is file or anonymous as well. Change
> > > the tracepoint to show symbolic names of the lru rather.
> > > 
> > > Signed-off-by: Michal Hocko <mhocko@suse.com>
> > 
> > Not exactly same with this but idea is almost same.
> > I used almost same tracepoint to investigate agging(i.e., deactivating) problem
> > in 32b kernel with node-lru.
> > It was enough. Namely, I didn't need tracepoint in shrink_active_list like your
> > first patch.
> > Your first patch is more straightforwad and information. But as you introduced
> > this patch, I want to ask in here.
> > Isn't it enough with this patch without your first one to find a such problem?
> 
> I assume this should be a reply to
> http://lkml.kernel.org/r/20161228153032.10821-8-mhocko@kernel.org, right?

I don't know my browser says "No such Message-ID known"

> And you are right that for the particular problem it was enough to have
> a tracepoint inside inactive_list_is_low and shrink_active_list one
> wasn't really needed. On the other hand aging issues are really hard to

What kinds of aging issue? What's the problem? How such tracepoint can help?
Please describe.

> debug as well and so I think that both are useful. The first one tell us
> _why_ we do aging while the later _how_ we do that.

Solve reported problem first you already knew. It would be no doubt
to merge and then send other patches about "it might be useful" with
useful scenario.

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

#1548511 — Re: [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate tracepoint

On Fri 30-12-16 10:56:25, Minchan Kim wrote:
> On Thu, Dec 29, 2016 at 08:56:49AM +0100, Michal Hocko wrote:
> > On Thu 29-12-16 15:02:04, Minchan Kim wrote:
> > > On Wed, Dec 28, 2016 at 04:30:29PM +0100, Michal Hocko wrote:
> > > > From: Michal Hocko <mhocko@suse.com>
> > > > 
> > > > mm_vmscan_lru_isolate currently prints only whether the LRU we isolate
> > > > from is file or anonymous but we do not know which LRU this is. It is
> > > > useful to know whether the list is file or anonymous as well. Change
> > > > the tracepoint to show symbolic names of the lru rather.
> > > > 
> > > > Signed-off-by: Michal Hocko <mhocko@suse.com>
> > > 
> > > Not exactly same with this but idea is almost same.
> > > I used almost same tracepoint to investigate agging(i.e., deactivating) problem
> > > in 32b kernel with node-lru.
> > > It was enough. Namely, I didn't need tracepoint in shrink_active_list like your
> > > first patch.
> > > Your first patch is more straightforwad and information. But as you introduced
> > > this patch, I want to ask in here.
> > > Isn't it enough with this patch without your first one to find a such problem?
> > 
> > I assume this should be a reply to
> > http://lkml.kernel.org/r/20161228153032.10821-8-mhocko@kernel.org, right?
> 
> I don't know my browser says "No such Message-ID known"

Hmm, not sure why it didn't get archived at lkml.kernel.org.
I meant https://lkml.org/lkml/2016/12/28/167
 
> > And you are right that for the particular problem it was enough to have
> > a tracepoint inside inactive_list_is_low and shrink_active_list one
> > wasn't really needed. On the other hand aging issues are really hard to
> 
> What kinds of aging issue? What's the problem? How such tracepoint can help?
> Please describe.

If you do not see that active list is shrunk then you do not know why it
is not shrunk. It might be a active/inactive ratio or just a plan bug
like the 32b issue me and you were debugging.

> > debug as well and so I think that both are useful. The first one tell us
> > _why_ we do aging while the later _how_ we do that.
> 
> Solve reported problem first you already knew. It would be no doubt
> to merge and then send other patches about "it might be useful" with
> useful scenario.

I am not sure I understand. The point of tracepoints is to be
pro-actively helpful not only to add something that has been useful in
one-off cases. A particular debugging session might be really helpful to
tell us what we are missing and this was the case here to a large part.
Once I was looking there I just wanted to save the pain of adding more
debugging information in future and allow people to debug their issue
without forcing them to recompile the kernel. I believe this is one of
the strong usecases for tracepoints in the first place.

-- 
Michal Hocko
SUSE Labs

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

#1548220 — Re: [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate tracepoint

On Wednesday, December 28, 2016 11:30 PM Michal Hocko wrote: 
> From: Michal Hocko <mhocko@suse.com>
> 
> mm_vmscan_lru_isolate currently prints only whether the LRU we isolate
> from is file or anonymous but we do not know which LRU this is. It is
> useful to know whether the list is file or anonymous as well. Change
> the tracepoint to show symbolic names of the lru rather.
> 
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
Acked-by: Hillf Danton <hillf.zj@alibaba-inc.com> 

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

#1549965 — Re: [PATCH 4/7] mm, vmscan: show LRU name in mm_vmscan_lru_isolate tracepoint

On 12/28/2016 04:30 PM, Michal Hocko wrote:
> From: Michal Hocko <mhocko@suse.com>
>
> mm_vmscan_lru_isolate currently prints only whether the LRU we isolate
> from is file or anonymous but we do not know which LRU this is. It is
> useful to know whether the list is file or anonymous as well. Change
> the tracepoint to show symbolic names of the lru rather.
>
> Signed-off-by: Michal Hocko <mhocko@suse.com>
> ---
>  include/trace/events/vmscan.h | 20 ++++++++++++++------
>  mm/vmscan.c                   |  2 +-
>  2 files changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
> index 6af4dae46db2..cc0b4c456c78 100644
> --- a/include/trace/events/vmscan.h
> +++ b/include/trace/events/vmscan.h
> @@ -36,6 +36,14 @@
>  		(RECLAIM_WB_ASYNC) \
>  	)
>
> +#define show_lru_name(lru) \
> +	__print_symbolic(lru, \
> +			{LRU_INACTIVE_ANON, "LRU_INACTIVE_ANON"}, \
> +			{LRU_ACTIVE_ANON, "LRU_ACTIVE_ANON"}, \
> +			{LRU_INACTIVE_FILE, "LRU_INACTIVE_FILE"}, \
> +			{LRU_ACTIVE_FILE, "LRU_ACTIVE_FILE"}, \
> +			{LRU_UNEVICTABLE, "LRU_UNEVICTABLE"})
> +

Does this work with external tools such as trace-cmd, i.e. does it export the 
correct format file? I wouldn't expect it to be that easy to avoid the 
EM()/EMe() dance :)

Also can we make the symbolic names lower_case and without the LRU_ prefix? I 
think it's more consistent with other mm tracepoints, shorter and nicer.

>  TRACE_EVENT(mm_vmscan_kswapd_sleep,
>
>  	TP_PROTO(int nid),
> @@ -277,9 +285,9 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
>  		unsigned long nr_skipped,
>  		unsigned long nr_taken,
>  		isolate_mode_t isolate_mode,
> -		int file),
> +		int lru),
>
> -	TP_ARGS(classzone_idx, order, nr_requested, nr_scanned, nr_skipped, nr_taken, isolate_mode, file),
> +	TP_ARGS(classzone_idx, order, nr_requested, nr_scanned, nr_skipped, nr_taken, isolate_mode, lru),
>
>  	TP_STRUCT__entry(
>  		__field(int, classzone_idx)
> @@ -289,7 +297,7 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
>  		__field(unsigned long, nr_skipped)
>  		__field(unsigned long, nr_taken)
>  		__field(isolate_mode_t, isolate_mode)
> -		__field(int, file)
> +		__field(int, lru)
>  	),
>
>  	TP_fast_assign(
> @@ -300,10 +308,10 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
>  		__entry->nr_skipped = nr_skipped;
>  		__entry->nr_taken = nr_taken;
>  		__entry->isolate_mode = isolate_mode;
> -		__entry->file = file;
> +		__entry->lru = lru;
>  	),
>
> -	TP_printk("isolate_mode=%d classzone=%d order=%d nr_requested=%lu nr_scanned=%lu nr_skipped=%lu nr_taken=%lu file=%d",
> +	TP_printk("isolate_mode=%d classzone=%d order=%d nr_requested=%lu nr_scanned=%lu nr_skipped=%lu nr_taken=%lu lru=%s",
>  		__entry->isolate_mode,
>  		__entry->classzone_idx,
>  		__entry->order,
> @@ -311,7 +319,7 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
>  		__entry->nr_scanned,
>  		__entry->nr_skipped,
>  		__entry->nr_taken,
> -		__entry->file)
> +		show_lru_name(__entry->lru))
>  );
>
>  TRACE_EVENT(mm_vmscan_writepage,
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 4f7c0d66d629..3f0774f30a42 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1500,7 +1500,7 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,
>  	}
>  	*nr_scanned = scan + total_skipped;
>  	trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan, scan,
> -				    skipped, nr_taken, mode, is_file_lru(lru));
> +				    skipped, nr_taken, mode, lru);
>  	update_lru_sizes(lruvec, lru, nr_zone_taken, nr_taken);
>  	return nr_taken;
>  }
>

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

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web