diff for duplicates of <20170104050722.GA17166@bbox> diff --git a/a/1.txt b/N1/1.txt index 030109a..5961cf4 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -108,3 +108,305 @@ Let's furhter it more. We can factor out logics to account isolation of LRU from shrink_[in]active_list which is more clean, I think. + +>From 1053968d526427ecad96b682aa586701c4ecfc84 Mon Sep 17 00:00:00 2001 +From: Minchan Kim <minchan@kernel.org> +Date: Wed, 4 Jan 2017 10:04:36 +0900 +Subject: [PATCH] factor out LRU isolation accounting. + +Not-yet-signed-off-by: Minchan Kim <minchan@kernel.org> +--- + include/trace/events/vmscan.h | 14 +++++---- + mm/vmscan.c | 68 ++++++++++++++++++------------------------- + 2 files changed, 37 insertions(+), 45 deletions(-) + +diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h +index 79b3cd9c7048..5fc3a94a14cd 100644 +--- a/include/trace/events/vmscan.h ++++ b/include/trace/events/vmscan.h +@@ -364,14 +364,15 @@ TRACE_EVENT(mm_vmscan_writepage, + TRACE_EVENT(mm_vmscan_lru_shrink_inactive, + + TP_PROTO(int nid, +- unsigned long nr_scanned, unsigned long nr_reclaimed, ++ unsigned long nr_taken, ++ unsigned long nr_reclaimed, + int priority, int file), + +- TP_ARGS(nid, nr_scanned, nr_reclaimed, priority, file), ++ TP_ARGS(nid, nr_taken, nr_reclaimed, priority, file), + + TP_STRUCT__entry( + __field(int, nid) +- __field(unsigned long, nr_scanned) ++ __field(unsigned long, nr_taken) + __field(unsigned long, nr_reclaimed) + __field(int, priority) + __field(int, reclaim_flags) +@@ -379,15 +380,16 @@ TRACE_EVENT(mm_vmscan_lru_shrink_inactive, + + TP_fast_assign( + __entry->nid = nid; +- __entry->nr_scanned = nr_scanned; ++ __entry->nr_taken = nr_taken; + __entry->nr_reclaimed = nr_reclaimed; + __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_taken=%ld nr_reclaimed=%ld priority=%d flags=%s", + __entry->nid, +- __entry->nr_scanned, __entry->nr_reclaimed, ++ __entry->nr_taken, ++ __entry->nr_reclaimed, + __entry->priority, + show_reclaim_flags(__entry->reclaim_flags)) + ); +diff --git a/mm/vmscan.c b/mm/vmscan.c +index 37ccd4e0b349..74f55f39f963 100644 +--- a/mm/vmscan.c ++++ b/mm/vmscan.c +@@ -1454,16 +1454,16 @@ static __always_inline void update_lru_sizes(struct lruvec *lruvec, + * @nr_to_scan: The number of pages to look through on the list. + * @lruvec: The LRU vector to pull pages from. + * @dst: The temp list to put pages on to. +- * @nr_scanned: The number of pages that were scanned. + * @sc: The scan_control struct for this reclaim session + * @mode: One of the LRU isolation modes + * @lru: LRU list id for isolating + * + * returns how many pages were moved onto *@dst. + */ +-static unsigned long isolate_lru_pages(unsigned long nr_to_scan, ++static unsigned long isolate_lru_pages(struct pglist_data *pgdat, ++ unsigned long nr_to_scan, + struct lruvec *lruvec, struct list_head *dst, +- unsigned long *nr_scanned, struct scan_control *sc, ++ struct scan_control *sc, + isolate_mode_t mode, enum lru_list lru) + { + struct list_head *src = &lruvec->lists[lru]; +@@ -1471,8 +1471,11 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan, + unsigned long nr_zone_taken[MAX_NR_ZONES] = { 0 }; + unsigned long nr_skipped[MAX_NR_ZONES] = { 0, }; + unsigned long scan, nr_pages; ++ struct zone_reclaim_stat *reclaim_stat = &lruvec->reclaim_stat; + LIST_HEAD(pages_skipped); ++ int file = is_file_lru(lru); + ++ spin_lock_irq(&pgdat->lru_lock); + for (scan = 0; scan < nr_to_scan && nr_taken < nr_to_scan && + !list_empty(src);) { + struct page *page; +@@ -1540,10 +1543,25 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan, + + list_splice(&pages_skipped, src); + } +- *nr_scanned = scan; + trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan, scan, + nr_taken, mode, is_file_lru(lru)); + update_lru_sizes(lruvec, lru, nr_zone_taken, nr_taken); ++ ++ __mod_node_page_state(pgdat, NR_ISOLATED_ANON + file, nr_taken); ++ reclaim_stat->recent_scanned[file] += nr_taken; ++ ++ if (global_reclaim(sc)) ++ __mod_node_page_state(pgdat, NR_PAGES_SCANNED, scan); ++ if (is_active_lru(lru)) { ++ __count_vm_events(PGREFILL, scan); ++ } else { ++ if (current_is_kswapd()) ++ __count_vm_events(PGSCAN_KSWAPD, scan); ++ else ++ __count_vm_events(PGSCAN_DIRECT, scan); ++ } ++ spin_unlock_irq(&pgdat->lru_lock); ++ + return nr_taken; + } + +@@ -1735,7 +1753,6 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec, + struct scan_control *sc, enum lru_list lru) + { + LIST_HEAD(page_list); +- unsigned long nr_scanned; + unsigned long nr_reclaimed = 0; + unsigned long nr_taken; + unsigned long nr_dirty = 0; +@@ -1746,7 +1763,6 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec, + isolate_mode_t isolate_mode = 0; + int file = is_file_lru(lru); + struct pglist_data *pgdat = lruvec_pgdat(lruvec); +- struct zone_reclaim_stat *reclaim_stat = &lruvec->reclaim_stat; + + if (!inactive_reclaimable_pages(lruvec, sc, lru)) + return 0; +@@ -1766,23 +1782,8 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec, + if (!sc->may_writepage) + isolate_mode |= ISOLATE_CLEAN; + +- spin_lock_irq(&pgdat->lru_lock); +- +- nr_taken = isolate_lru_pages(nr_to_scan, lruvec, &page_list, +- &nr_scanned, sc, isolate_mode, lru); +- +- __mod_node_page_state(pgdat, NR_ISOLATED_ANON + file, nr_taken); +- reclaim_stat->recent_scanned[file] += nr_taken; +- +- if (global_reclaim(sc)) { +- __mod_node_page_state(pgdat, NR_PAGES_SCANNED, nr_scanned); +- if (current_is_kswapd()) +- __count_vm_events(PGSCAN_KSWAPD, nr_scanned); +- else +- __count_vm_events(PGSCAN_DIRECT, nr_scanned); +- } +- spin_unlock_irq(&pgdat->lru_lock); +- ++ nr_taken = isolate_lru_pages(pgdat, nr_to_scan, lruvec, &page_list, ++ sc, isolate_mode, lru); + if (nr_taken == 0) + return 0; + +@@ -1866,7 +1867,8 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec, + wait_iff_congested(pgdat, BLK_RW_ASYNC, HZ/10); + + trace_mm_vmscan_lru_shrink_inactive(pgdat->node_id, +- nr_scanned, nr_reclaimed, ++ nr_taken, ++ nr_reclaimed, + sc->priority, file); + return nr_reclaimed; + } +@@ -1943,18 +1945,17 @@ static void shrink_active_list(unsigned long nr_to_scan, + enum lru_list lru) + { + unsigned long nr_taken; +- unsigned long nr_scanned; + unsigned long vm_flags; + LIST_HEAD(l_hold); /* The pages which were snipped off */ + LIST_HEAD(l_active); + LIST_HEAD(l_inactive); + struct page *page; +- struct zone_reclaim_stat *reclaim_stat = &lruvec->reclaim_stat; + unsigned nr_deactivate, nr_activate; + unsigned nr_rotated = 0; + isolate_mode_t isolate_mode = 0; + int file = is_file_lru(lru); + struct pglist_data *pgdat = lruvec_pgdat(lruvec); ++ struct zone_reclaim_stat *reclaim_stat = &lruvec->reclaim_stat; + + lru_add_drain(); + +@@ -1963,19 +1964,8 @@ static void shrink_active_list(unsigned long nr_to_scan, + if (!sc->may_writepage) + isolate_mode |= ISOLATE_CLEAN; + +- spin_lock_irq(&pgdat->lru_lock); +- +- nr_taken = isolate_lru_pages(nr_to_scan, lruvec, &l_hold, +- &nr_scanned, sc, isolate_mode, lru); +- +- __mod_node_page_state(pgdat, NR_ISOLATED_ANON + file, nr_taken); +- reclaim_stat->recent_scanned[file] += nr_taken; +- +- if (global_reclaim(sc)) +- __mod_node_page_state(pgdat, NR_PAGES_SCANNED, nr_scanned); +- __count_vm_events(PGREFILL, nr_scanned); +- +- spin_unlock_irq(&pgdat->lru_lock); ++ nr_taken = isolate_lru_pages(pgdat, nr_to_scan, lruvec, &l_hold, ++ sc, isolate_mode, lru); + + while (!list_empty(&l_hold)) { + cond_resched(); +-- +2.7.4 + +With this, +./scripts/bloat-o-meter vmlinux.old vmlinux.new.new +add/remove: 1/1 grow/shrink: 0/9 up/down: 1394/-1636 (-242) +function old new delta +isolate_lru_pages - 1394 +1394 +print_fmt_mm_vmscan_lru_shrink_inactive 359 355 -4 +vermagic 64 58 -6 +perf_trace_mm_vmscan_lru_shrink_active 264 256 -8 +trace_raw_output_mm_vmscan_lru_shrink_active 203 193 -10 +trace_event_raw_event_mm_vmscan_lru_shrink_active 241 225 -16 +print_fmt_mm_vmscan_lru_shrink_active 458 426 -32 +trace_event_define_fields_mm_vmscan_lru_shrink_active 384 336 -48 +shrink_inactive_list 1430 1271 -159 +shrink_active_list 1265 1082 -183 +isolate_lru_pages.isra 1170 - -1170 +Total: Before=26268743, After=26268501, chg -0.00% + +We can save 242 bytes. + +If we consider binary size, 424 bytes save. + +#> ls -l vmlinux.old vmlinux.new.new +194092840 vmlinux.old +194092416 vmlinux.new.new + +> with other tracepoints but that can be helpful because you do not have +> all the tracepoints enabled all the time. So unless you see this +> particular thing as a road block I would rather keep it. + +I didn't know how long this thread becomes lenghy. To me, it was no worth +to discuss. I did best effot to explain my stand with valid points, I think +and don't want to go infinite loop. If you don't agree still, separate +the patch. One includes only necessary things with removing nr_scanned, which +I am happy to ack it. Based upon it, add one more patch you want adding +nr_scanned with your claim. I will reply that thread with my claim and +let's keep an eye on it that whether maintainer will take it or not. +If maintainer will take it, it's good indication which will represent +we can add more extra tracepoint easily with "might be helpful with someone +although it's redunant" so do not prevent others who want to do +in the future. + +> +> > > The inactive counterpart does that for quite some time already. So why +> > +> > It couldn't be a reason. If it was duplicated in there, it would be +> > better to fix it rather than adding more duplciated work to match both +> > sides. +> +> I really do not see this as a bad thing. +> +> > > exactly does that matter? Don't take me wrong but isn't this more on a +> > > nit picking side than necessary? Or do I just misunderstand your +> > > concenrs? It is not like we are providing a stable user API as the +> > +> > My concern is that I don't see what we can get benefit from those +> > duplicated work. If it doesn't give benefit to us, I don't want to add. +> > I hope you think another reasonable reasons. +> > +> > > tracepoint is clearly implementation specific and not something to be +> > > used for anything other than debugging. +> > +> > My point is we already had things "LRU isolation effectivness". Namely, +> > isolate_lru_pages. +> > +> > > +> > > > > - nr_rotated pages which tells us that we are hitting referenced +> > > > > pages which are deactivated. If this is a large part of the +> > > > > reported nr_deactivated pages then the active list is too small +> > > > +> > > > It might be but not exactly. If your goal is to know LRU size, it can be +> > > > done in get_scan_count. I tend to agree LRU size is helpful for +> > > > performance analysis because decreased LRU size signals memory shortage +> > > > then performance drop. +> > > +> > > No, I am not really interested in the exact size but rather to allow to +> > > find whether we are aging the active list too early... +> > +> > Could you elaborate it more that how we can get active list early aging +> > with nr_rotated? +> +> If you see too many referenced pages on the active list then they have +> been used since promoted and that is an indication that they might be +> reclaimed too early. If you are debugging a performance issue and see +> this happening then it might be a good indication to look at. + +This is better than "active list is too small". I hope you change +description with this. diff --git a/a/content_digest b/N1/content_digest index de1b803..d51d02e 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -131,6 +131,308 @@ "Let's furhter it more.\n" "\n" "We can factor out logics to account isolation of LRU from shrink_[in]active_list\n" - which is more clean, I think. + "which is more clean, I think.\n" + "\n" + ">From 1053968d526427ecad96b682aa586701c4ecfc84 Mon Sep 17 00:00:00 2001\n" + "From: Minchan Kim <minchan@kernel.org>\n" + "Date: Wed, 4 Jan 2017 10:04:36 +0900\n" + "Subject: [PATCH] factor out LRU isolation accounting.\n" + "\n" + "Not-yet-signed-off-by: Minchan Kim <minchan@kernel.org>\n" + "---\n" + " include/trace/events/vmscan.h | 14 +++++----\n" + " mm/vmscan.c | 68 ++++++++++++++++++-------------------------\n" + " 2 files changed, 37 insertions(+), 45 deletions(-)\n" + "\n" + "diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h\n" + "index 79b3cd9c7048..5fc3a94a14cd 100644\n" + "--- a/include/trace/events/vmscan.h\n" + "+++ b/include/trace/events/vmscan.h\n" + "@@ -364,14 +364,15 @@ TRACE_EVENT(mm_vmscan_writepage,\n" + " TRACE_EVENT(mm_vmscan_lru_shrink_inactive,\n" + " \n" + " \tTP_PROTO(int nid,\n" + "-\t\tunsigned long nr_scanned, unsigned long nr_reclaimed,\n" + "+\t\tunsigned long nr_taken,\n" + "+\t\tunsigned long nr_reclaimed,\n" + " \t\tint priority, int file),\n" + " \n" + "-\tTP_ARGS(nid, nr_scanned, nr_reclaimed, priority, file),\n" + "+\tTP_ARGS(nid, nr_taken, nr_reclaimed, priority, file),\n" + " \n" + " \tTP_STRUCT__entry(\n" + " \t\t__field(int, nid)\n" + "-\t\t__field(unsigned long, nr_scanned)\n" + "+\t\t__field(unsigned long, nr_taken)\n" + " \t\t__field(unsigned long, nr_reclaimed)\n" + " \t\t__field(int, priority)\n" + " \t\t__field(int, reclaim_flags)\n" + "@@ -379,15 +380,16 @@ TRACE_EVENT(mm_vmscan_lru_shrink_inactive,\n" + " \n" + " \tTP_fast_assign(\n" + " \t\t__entry->nid = nid;\n" + "-\t\t__entry->nr_scanned = nr_scanned;\n" + "+\t\t__entry->nr_taken = nr_taken;\n" + " \t\t__entry->nr_reclaimed = nr_reclaimed;\n" + " \t\t__entry->priority = priority;\n" + " \t\t__entry->reclaim_flags = trace_shrink_flags(file);\n" + " \t),\n" + " \n" + "-\tTP_printk(\"nid=%d nr_scanned=%ld nr_reclaimed=%ld priority=%d flags=%s\",\n" + "+\tTP_printk(\"nid=%d nr_taken=%ld nr_reclaimed=%ld priority=%d flags=%s\",\n" + " \t\t__entry->nid,\n" + "-\t\t__entry->nr_scanned, __entry->nr_reclaimed,\n" + "+\t\t__entry->nr_taken,\n" + "+\t\t__entry->nr_reclaimed,\n" + " \t\t__entry->priority,\n" + " \t\tshow_reclaim_flags(__entry->reclaim_flags))\n" + " );\n" + "diff --git a/mm/vmscan.c b/mm/vmscan.c\n" + "index 37ccd4e0b349..74f55f39f963 100644\n" + "--- a/mm/vmscan.c\n" + "+++ b/mm/vmscan.c\n" + "@@ -1454,16 +1454,16 @@ static __always_inline void update_lru_sizes(struct lruvec *lruvec,\n" + " * @nr_to_scan:\tThe number of pages to look through on the list.\n" + " * @lruvec:\tThe LRU vector to pull pages from.\n" + " * @dst:\tThe temp list to put pages on to.\n" + "- * @nr_scanned:\tThe number of pages that were scanned.\n" + " * @sc:\t\tThe scan_control struct for this reclaim session\n" + " * @mode:\tOne of the LRU isolation modes\n" + " * @lru:\tLRU list id for isolating\n" + " *\n" + " * returns how many pages were moved onto *@dst.\n" + " */\n" + "-static unsigned long isolate_lru_pages(unsigned long nr_to_scan,\n" + "+static unsigned long isolate_lru_pages(struct pglist_data *pgdat,\n" + "+\t\tunsigned long nr_to_scan,\n" + " \t\tstruct lruvec *lruvec, struct list_head *dst,\n" + "-\t\tunsigned long *nr_scanned, struct scan_control *sc,\n" + "+\t\tstruct scan_control *sc,\n" + " \t\tisolate_mode_t mode, enum lru_list lru)\n" + " {\n" + " \tstruct list_head *src = &lruvec->lists[lru];\n" + "@@ -1471,8 +1471,11 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,\n" + " \tunsigned long nr_zone_taken[MAX_NR_ZONES] = { 0 };\n" + " \tunsigned long nr_skipped[MAX_NR_ZONES] = { 0, };\n" + " \tunsigned long scan, nr_pages;\n" + "+\tstruct zone_reclaim_stat *reclaim_stat = &lruvec->reclaim_stat;\n" + " \tLIST_HEAD(pages_skipped);\n" + "+\tint file = is_file_lru(lru);\n" + " \n" + "+\tspin_lock_irq(&pgdat->lru_lock);\n" + " \tfor (scan = 0; scan < nr_to_scan && nr_taken < nr_to_scan &&\n" + " \t\t\t\t\t!list_empty(src);) {\n" + " \t\tstruct page *page;\n" + "@@ -1540,10 +1543,25 @@ static unsigned long isolate_lru_pages(unsigned long nr_to_scan,\n" + " \n" + " \t\tlist_splice(&pages_skipped, src);\n" + " \t}\n" + "-\t*nr_scanned = scan;\n" + " \ttrace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan, scan,\n" + " \t\t\t\t nr_taken, mode, is_file_lru(lru));\n" + " \tupdate_lru_sizes(lruvec, lru, nr_zone_taken, nr_taken);\n" + "+\n" + "+\t__mod_node_page_state(pgdat, NR_ISOLATED_ANON + file, nr_taken);\n" + "+\treclaim_stat->recent_scanned[file] += nr_taken;\n" + "+\n" + "+\tif (global_reclaim(sc))\n" + "+\t\t__mod_node_page_state(pgdat, NR_PAGES_SCANNED, scan);\n" + "+\tif (is_active_lru(lru)) {\n" + "+\t\t__count_vm_events(PGREFILL, scan);\n" + "+\t} else {\n" + "+\t\tif (current_is_kswapd())\n" + "+\t\t\t__count_vm_events(PGSCAN_KSWAPD, scan);\n" + "+\t\telse\n" + "+\t\t\t__count_vm_events(PGSCAN_DIRECT, scan);\n" + "+\t}\n" + "+\tspin_unlock_irq(&pgdat->lru_lock);\n" + "+\n" + " \treturn nr_taken;\n" + " }\n" + " \n" + "@@ -1735,7 +1753,6 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,\n" + " \t\t struct scan_control *sc, enum lru_list lru)\n" + " {\n" + " \tLIST_HEAD(page_list);\n" + "-\tunsigned long nr_scanned;\n" + " \tunsigned long nr_reclaimed = 0;\n" + " \tunsigned long nr_taken;\n" + " \tunsigned long nr_dirty = 0;\n" + "@@ -1746,7 +1763,6 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,\n" + " \tisolate_mode_t isolate_mode = 0;\n" + " \tint file = is_file_lru(lru);\n" + " \tstruct pglist_data *pgdat = lruvec_pgdat(lruvec);\n" + "-\tstruct zone_reclaim_stat *reclaim_stat = &lruvec->reclaim_stat;\n" + " \n" + " \tif (!inactive_reclaimable_pages(lruvec, sc, lru))\n" + " \t\treturn 0;\n" + "@@ -1766,23 +1782,8 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,\n" + " \tif (!sc->may_writepage)\n" + " \t\tisolate_mode |= ISOLATE_CLEAN;\n" + " \n" + "-\tspin_lock_irq(&pgdat->lru_lock);\n" + "-\n" + "-\tnr_taken = isolate_lru_pages(nr_to_scan, lruvec, &page_list,\n" + "-\t\t\t\t &nr_scanned, sc, isolate_mode, lru);\n" + "-\n" + "-\t__mod_node_page_state(pgdat, NR_ISOLATED_ANON + file, nr_taken);\n" + "-\treclaim_stat->recent_scanned[file] += nr_taken;\n" + "-\n" + "-\tif (global_reclaim(sc)) {\n" + "-\t\t__mod_node_page_state(pgdat, NR_PAGES_SCANNED, nr_scanned);\n" + "-\t\tif (current_is_kswapd())\n" + "-\t\t\t__count_vm_events(PGSCAN_KSWAPD, nr_scanned);\n" + "-\t\telse\n" + "-\t\t\t__count_vm_events(PGSCAN_DIRECT, nr_scanned);\n" + "-\t}\n" + "-\tspin_unlock_irq(&pgdat->lru_lock);\n" + "-\n" + "+\tnr_taken = isolate_lru_pages(pgdat, nr_to_scan, lruvec, &page_list,\n" + "+\t\t\t\t\tsc, isolate_mode, lru);\n" + " \tif (nr_taken == 0)\n" + " \t\treturn 0;\n" + " \n" + "@@ -1866,7 +1867,8 @@ shrink_inactive_list(unsigned long nr_to_scan, struct lruvec *lruvec,\n" + " \t\twait_iff_congested(pgdat, BLK_RW_ASYNC, HZ/10);\n" + " \n" + " \ttrace_mm_vmscan_lru_shrink_inactive(pgdat->node_id,\n" + "-\t\t\tnr_scanned, nr_reclaimed,\n" + "+\t\t\tnr_taken,\n" + "+\t\t\tnr_reclaimed,\n" + " \t\t\tsc->priority, file);\n" + " \treturn nr_reclaimed;\n" + " }\n" + "@@ -1943,18 +1945,17 @@ static void shrink_active_list(unsigned long nr_to_scan,\n" + " \t\t\t enum lru_list lru)\n" + " {\n" + " \tunsigned long nr_taken;\n" + "-\tunsigned long nr_scanned;\n" + " \tunsigned long vm_flags;\n" + " \tLIST_HEAD(l_hold);\t/* The pages which were snipped off */\n" + " \tLIST_HEAD(l_active);\n" + " \tLIST_HEAD(l_inactive);\n" + " \tstruct page *page;\n" + "-\tstruct zone_reclaim_stat *reclaim_stat = &lruvec->reclaim_stat;\n" + " \tunsigned nr_deactivate, nr_activate;\n" + " \tunsigned nr_rotated = 0;\n" + " \tisolate_mode_t isolate_mode = 0;\n" + " \tint file = is_file_lru(lru);\n" + " \tstruct pglist_data *pgdat = lruvec_pgdat(lruvec);\n" + "+\tstruct zone_reclaim_stat *reclaim_stat = &lruvec->reclaim_stat;\n" + " \n" + " \tlru_add_drain();\n" + " \n" + "@@ -1963,19 +1964,8 @@ static void shrink_active_list(unsigned long nr_to_scan,\n" + " \tif (!sc->may_writepage)\n" + " \t\tisolate_mode |= ISOLATE_CLEAN;\n" + " \n" + "-\tspin_lock_irq(&pgdat->lru_lock);\n" + "-\n" + "-\tnr_taken = isolate_lru_pages(nr_to_scan, lruvec, &l_hold,\n" + "-\t\t\t\t &nr_scanned, sc, isolate_mode, lru);\n" + "-\n" + "-\t__mod_node_page_state(pgdat, NR_ISOLATED_ANON + file, nr_taken);\n" + "-\treclaim_stat->recent_scanned[file] += nr_taken;\n" + "-\n" + "-\tif (global_reclaim(sc))\n" + "-\t\t__mod_node_page_state(pgdat, NR_PAGES_SCANNED, nr_scanned);\n" + "-\t__count_vm_events(PGREFILL, nr_scanned);\n" + "-\n" + "-\tspin_unlock_irq(&pgdat->lru_lock);\n" + "+\tnr_taken = isolate_lru_pages(pgdat, nr_to_scan, lruvec, &l_hold,\n" + "+\t\t\t\t sc, isolate_mode, lru);\n" + " \n" + " \twhile (!list_empty(&l_hold)) {\n" + " \t\tcond_resched();\n" + "-- \n" + "2.7.4\n" + "\n" + "With this,\n" + "./scripts/bloat-o-meter vmlinux.old vmlinux.new.new\n" + "add/remove: 1/1 grow/shrink: 0/9 up/down: 1394/-1636 (-242)\n" + "function old new delta\n" + "isolate_lru_pages - 1394 +1394\n" + "print_fmt_mm_vmscan_lru_shrink_inactive 359 355 -4\n" + "vermagic 64 58 -6\n" + "perf_trace_mm_vmscan_lru_shrink_active 264 256 -8\n" + "trace_raw_output_mm_vmscan_lru_shrink_active 203 193 -10\n" + "trace_event_raw_event_mm_vmscan_lru_shrink_active 241 225 -16\n" + "print_fmt_mm_vmscan_lru_shrink_active 458 426 -32\n" + "trace_event_define_fields_mm_vmscan_lru_shrink_active 384 336 -48\n" + "shrink_inactive_list 1430 1271 -159\n" + "shrink_active_list 1265 1082 -183\n" + "isolate_lru_pages.isra 1170 - -1170\n" + "Total: Before=26268743, After=26268501, chg -0.00%\n" + "\n" + "We can save 242 bytes.\n" + "\n" + "If we consider binary size, 424 bytes save.\n" + "\n" + "#> ls -l vmlinux.old vmlinux.new.new\n" + "194092840 vmlinux.old\n" + "194092416 vmlinux.new.new\n" + "\n" + "> with other tracepoints but that can be helpful because you do not have\n" + "> all the tracepoints enabled all the time. So unless you see this\n" + "> particular thing as a road block I would rather keep it.\n" + "\n" + "I didn't know how long this thread becomes lenghy. To me, it was no worth\n" + "to discuss. I did best effot to explain my stand with valid points, I think\n" + "and don't want to go infinite loop. If you don't agree still, separate\n" + "the patch. One includes only necessary things with removing nr_scanned, which\n" + "I am happy to ack it. Based upon it, add one more patch you want adding\n" + "nr_scanned with your claim. I will reply that thread with my claim and\n" + "let's keep an eye on it that whether maintainer will take it or not.\n" + "If maintainer will take it, it's good indication which will represent\n" + "we can add more extra tracepoint easily with \"might be helpful with someone\n" + "although it's redunant\" so do not prevent others who want to do\n" + "in the future.\n" + "\n" + "> \n" + "> > > The inactive counterpart does that for quite some time already. So why\n" + "> > \n" + "> > It couldn't be a reason. If it was duplicated in there, it would be\n" + "> > better to fix it rather than adding more duplciated work to match both\n" + "> > sides.\n" + "> \n" + "> I really do not see this as a bad thing.\n" + "> \n" + "> > > exactly does that matter? Don't take me wrong but isn't this more on a\n" + "> > > nit picking side than necessary? Or do I just misunderstand your\n" + "> > > concenrs? It is not like we are providing a stable user API as the\n" + "> > \n" + "> > My concern is that I don't see what we can get benefit from those\n" + "> > duplicated work. If it doesn't give benefit to us, I don't want to add.\n" + "> > I hope you think another reasonable reasons.\n" + "> > \n" + "> > > tracepoint is clearly implementation specific and not something to be\n" + "> > > used for anything other than debugging.\n" + "> > \n" + "> > My point is we already had things \"LRU isolation effectivness\". Namely,\n" + "> > isolate_lru_pages.\n" + "> > \n" + "> > > \n" + "> > > > > \t- nr_rotated pages which tells us that we are hitting referenced\n" + "> > > > > \t pages which are deactivated. If this is a large part of the\n" + "> > > > > \t reported nr_deactivated pages then the active list is too small\n" + "> > > > \n" + "> > > > It might be but not exactly. If your goal is to know LRU size, it can be\n" + "> > > > done in get_scan_count. I tend to agree LRU size is helpful for\n" + "> > > > performance analysis because decreased LRU size signals memory shortage\n" + "> > > > then performance drop.\n" + "> > > \n" + "> > > No, I am not really interested in the exact size but rather to allow to\n" + "> > > find whether we are aging the active list too early...\n" + "> > \n" + "> > Could you elaborate it more that how we can get active list early aging\n" + "> > with nr_rotated?\n" + "> \n" + "> If you see too many referenced pages on the active list then they have\n" + "> been used since promoted and that is an indication that they might be\n" + "> reclaimed too early. If you are debugging a performance issue and see\n" + "> this happening then it might be a good indication to look at.\n" + "\n" + "This is better than \"active list is too small\". I hope you change\n" + description with this. -6fbec2748bc83874e7741b03f19586544442d6110cd43df107c4d4a5f7cf9f65 +69d43bdc1029a13e01a252be5e5dc91022c6bd9aa8178ac8a3ea88abee1e3afb
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.